diff --git a/src/hotspot/os/posix/attachListener_posix.cpp b/src/hotspot/os/posix/attachListener_posix.cpp index 569c317caa7..fea2075296a 100644 --- a/src/hotspot/os/posix/attachListener_posix.cpp +++ b/src/hotspot/os/posix/attachListener_posix.cpp @@ -76,17 +76,7 @@ class PosixAttachListener: AllStatic { static bool _atexit_registered; - // reads a request from the given connected socket - static PosixAttachOperation* read_request(int s); - public: - enum { - ATTACH_PROTOCOL_VER = 1 // protocol version - }; - enum { - ATTACH_ERROR_BADVERSION = 101 // error codes - }; - static void set_path(char* path) { if (path == nullptr) { _path[0] = '\0'; @@ -107,25 +97,61 @@ class PosixAttachListener: AllStatic { static bool has_path() { return _has_path; } static int listener() { return _listener; } - // write the given buffer to a socket - static int write_fully(int s, char* buf, size_t len); - static PosixAttachOperation* dequeue(); }; +class SocketChannel : public AttachOperation::RequestReader, public AttachOperation::ReplyWriter { +private: + int _socket; +public: + SocketChannel(int socket) : _socket(socket) {} + ~SocketChannel() { + close(); + } + + bool opened() const { + return _socket != -1; + } + + void close() { + if (opened()) { + ::close(_socket); + _socket = -1; + } + } + + // RequestReader + int read(void* buffer, int size) override { + ssize_t n; + RESTARTABLE(::read(_socket, buffer, (size_t)size), n); + return checked_cast(n); + } + + // ReplyWriter + int write(const void* buffer, int size) override { + ssize_t n; + RESTARTABLE(::write(_socket, buffer, size), n); + return checked_cast(n); + } + // called after writing all data + void flush() override { + ::shutdown(_socket, SHUT_RDWR); + } +}; + class PosixAttachOperation: public AttachOperation { private: // the connection to the client - int _socket; + SocketChannel _socket_channel; public: - void complete(jint res, bufferedStream* st); + void complete(jint res, bufferedStream* st) override; - void set_socket(int s) { _socket = s; } - int socket() const { return _socket; } + PosixAttachOperation(int socket) : AttachOperation(), _socket_channel(socket) { + } - PosixAttachOperation(char* name) : AttachOperation(name) { - set_socket(-1); + bool read_request() { + return AttachOperation::read_request(&_socket_channel, &_socket_channel); } }; @@ -135,35 +161,6 @@ bool PosixAttachListener::_has_path; volatile int PosixAttachListener::_listener = -1; bool PosixAttachListener::_atexit_registered = false; -// Supporting class to help split a buffer into individual components -class ArgumentIterator : public StackObj { - private: - char* _pos; - char* _end; - public: - ArgumentIterator(char* arg_buffer, size_t arg_size) { - _pos = arg_buffer; - _end = _pos + arg_size - 1; - } - char* next() { - if (*_pos == '\0') { - // advance the iterator if possible (null arguments) - if (_pos < _end) { - _pos += 1; - } - return nullptr; - } - char* res = _pos; - char* next_pos = strchr(_pos, '\0'); - if (next_pos < _end) { - next_pos++; - } - _pos = next_pos; - return res; - } -}; - - // atexit hook to stop listener and unlink the file that it is // bound too. extern "C" { @@ -249,103 +246,6 @@ int PosixAttachListener::init() { return 0; } -// Given a socket that is connected to a peer we read the request and -// create an AttachOperation. As the socket is blocking there is potential -// for a denial-of-service if the peer does not response. However this happens -// after the peer credentials have been checked and in the worst case it just -// means that the attach listener thread is blocked. -// -PosixAttachOperation* PosixAttachListener::read_request(int s) { - char ver_str[8]; - os::snprintf_checked(ver_str, sizeof(ver_str), "%d", ATTACH_PROTOCOL_VER); - - // The request is a sequence of strings so we first figure out the - // expected count and the maximum possible length of the request. - // The request is: - // 00000 - // where is the protocol version (1), is the command - // name ("load", "datadump", ...), and is an argument - int expected_str_count = 2 + AttachOperation::arg_count_max; - const size_t max_len = (sizeof(ver_str) + 1) + (AttachOperation::name_length_max + 1) + - AttachOperation::arg_count_max*(AttachOperation::arg_length_max + 1); - - char buf[max_len]; - int str_count = 0; - - // Read until all (expected) strings have been read, the buffer is - // full, or EOF. - - size_t off = 0; - size_t left = max_len; - - do { - ssize_t n; - RESTARTABLE(read(s, buf+off, left), n); - assert(n <= checked_cast(left), "buffer was too small, impossible!"); - buf[max_len - 1] = '\0'; - if (n == -1) { - return nullptr; // reset by peer or other error - } - if (n == 0) { - break; - } - for (ssize_t i=0; i so check it now to - // check for protocol mismatch - if (str_count == 1) { - if ((strlen(buf) != strlen(ver_str)) || - (atoi(buf) != ATTACH_PROTOCOL_VER)) { - char msg[32]; - os::snprintf_checked(msg, sizeof(msg), "%d\n", ATTACH_ERROR_BADVERSION); - write_fully(s, msg, strlen(msg)); - return nullptr; - } - } - } - } - off += n; - left -= n; - } while (left > 0 && str_count < expected_str_count); - - if (str_count != expected_str_count) { - return nullptr; // incomplete request - } - - // parse request - - ArgumentIterator args(buf, (max_len)-left); - - // version already checked - char* v = args.next(); - - char* name = args.next(); - if (name == nullptr || strlen(name) > AttachOperation::name_length_max) { - return nullptr; - } - - PosixAttachOperation* op = new PosixAttachOperation(name); - - for (int i=0; iset_arg(i, nullptr); - } else { - if (strlen(arg) > AttachOperation::arg_length_max) { - delete op; - return nullptr; - } - op->set_arg(i, arg); - } - } - - op->set_socket(s); - return op; -} - // Dequeue an operation // // In the Linux and BSD implementations, there is only a single operation and @@ -400,9 +300,9 @@ PosixAttachOperation* PosixAttachListener::dequeue() { #endif // peer credential look okay so we read the request - PosixAttachOperation* op = read_request(s); - if (op == nullptr) { - ::close(s); + PosixAttachOperation* op = new PosixAttachOperation(s); + if (!op->read_request()) { + delete op; continue; } else { return op; @@ -410,21 +310,6 @@ PosixAttachOperation* PosixAttachListener::dequeue() { } } -// write the given buffer to the socket -int PosixAttachListener::write_fully(int s, char* buf, size_t len) { - do { - ssize_t n = ::write(s, buf, len); - if (n == -1) { - if (errno != EINTR) return -1; - } else { - buf += n; - len -= n; - } - } - while (len > 0); - return 0; -} - // Complete an operation by sending the operation result and any result // output to the client. At this time the socket is in blocking mode so // potentially we can block if there is a lot of data and the client is @@ -437,19 +322,7 @@ void PosixAttachOperation::complete(jint result, bufferedStream* st) { JavaThread* thread = JavaThread::current(); ThreadBlockInVM tbivm(thread); - // write operation result - char msg[32]; - os::snprintf_checked(msg, sizeof(msg), "%d\n", result); - int rc = PosixAttachListener::write_fully(this->socket(), msg, strlen(msg)); - - // write any result data - if (rc == 0) { - PosixAttachListener::write_fully(this->socket(), (char*) st->base(), st->size()); - ::shutdown(this->socket(), 2); - } - - // done - ::close(this->socket()); + write_reply(&_socket_channel, result, st); delete this; } @@ -490,6 +363,8 @@ void AttachListener::vm_start() { } int AttachListener::pd_init() { + AttachListener::set_supported_version(ATTACH_API_V2); + JavaThread* thread = JavaThread::current(); ThreadBlockInVM tbivm(thread); diff --git a/src/hotspot/os/windows/attachListener_windows.cpp b/src/hotspot/os/windows/attachListener_windows.cpp index fa45c98ced2..8423fe42b75 100644 --- a/src/hotspot/os/windows/attachListener_windows.cpp +++ b/src/hotspot/os/windows/attachListener_windows.cpp @@ -152,7 +152,7 @@ public: } bool read_request() { - return AttachOperation::read_request(&_pipe); + return AttachOperation::read_request(&_pipe, &_pipe); } public: diff --git a/src/hotspot/share/services/attachListener.cpp b/src/hotspot/share/services/attachListener.cpp index 8cf5c31b0b3..0d67268ec38 100644 --- a/src/hotspot/share/services/attachListener.cpp +++ b/src/hotspot/share/services/attachListener.cpp @@ -525,7 +525,7 @@ AttachAPIVersion AttachListener::get_supported_version() { } -int AttachOperation::RequestReader::read_uint() { +int AttachOperation::RequestReader::read_uint(bool may_be_empty) { const int MAX_VALUE = INT_MAX / 20; char ch; int value = 0; @@ -534,7 +534,9 @@ int AttachOperation::RequestReader::read_uint() { if (n != 1) { // IO errors (n < 0) are logged by read(). if (n == 0) { // EOF - log_error(attach)("Failed to read int value: EOF"); + if (!may_be_empty || value != 0) { // value != 0 means this is not the 1st read + log_error(attach)("Failed to read int value: EOF"); + } } return -1; } @@ -615,8 +617,11 @@ bool AttachOperation::read_request_data(AttachOperation::RequestReader* reader, return true; } -bool AttachOperation::read_request(RequestReader* reader) { - uint ver = reader->read_uint(); +bool AttachOperation::read_request(RequestReader* reader, ReplyWriter* error_writer) { + int ver = reader->read_uint(true); // do not log error if this is "empty" connection + if (ver < 0) { + return false; + } int buffer_size = 0; // Read conditions: int min_str_count = 0; // expected number of strings in the request @@ -631,6 +636,7 @@ bool AttachOperation::read_request(RequestReader* reader) { case ATTACH_API_V2: // 000(0)* (any number of arguments) if (AttachListener::get_supported_version() < 2) { log_error(attach)("Failed to read request: v2 is unsupported or disabled"); + write_reply(error_writer, ATTACH_ERROR_BADVERSION, "v2 is unsupported or disabled"); return false; } @@ -652,10 +658,26 @@ bool AttachOperation::read_request(RequestReader* reader) { break; default: log_error(attach)("Failed to read request: unknown version (%d)", ver); + write_reply(error_writer, ATTACH_ERROR_BADVERSION, "unknown version"); return false; } - return read_request_data(reader, buffer_size, min_str_count, min_read_size); + bool result = read_request_data(reader, buffer_size, min_str_count, min_read_size); + if (result && ver == ATTACH_API_V1) { + // We know the whole request does not exceed buffer_size, + // for v1 also name/arguments should not exceed name_length_max/arg_length_max. + if (strlen(name()) > AttachOperation::name_length_max) { + log_error(attach)("Failed to read request: operation name is too long"); + return false; + } + for (int i = 0; i < arg_count(); i++) { + if (strlen(arg(i)) > AttachOperation::arg_length_max) { + log_error(attach)("Failed to read request: operation argument is too long"); + return false; + } + } + } + return result; } bool AttachOperation::ReplyWriter::write_fully(const void* buffer, int size) { @@ -671,16 +693,23 @@ bool AttachOperation::ReplyWriter::write_fully(const void* buffer, int size) { return true; } -bool AttachOperation::write_reply(ReplyWriter* writer, jint result, bufferedStream* result_stream) { - char msg[32]; - os::snprintf_checked(msg, sizeof(msg), "%d\n", result); - if (!writer->write_fully(msg, (int)strlen(msg))) { +bool AttachOperation::write_reply(ReplyWriter * writer, jint result, const char* message, int message_len) { + if (message_len < 0) { + message_len = (int)strlen(message); + } + char buf[32]; + os::snprintf_checked(buf, sizeof(buf), "%d\n", result); + if (!writer->write_fully(buf, (int)strlen(buf))) { return false; } - if (!writer->write_fully(result_stream->base(), (int)result_stream->size())) { + if (!writer->write_fully(message, message_len)) { return false; } writer->flush(); return true; } +bool AttachOperation::write_reply(ReplyWriter* writer, jint result, bufferedStream* result_stream) { + return write_reply(writer, result, result_stream->base(), (int)result_stream->size()); +} + diff --git a/src/hotspot/share/services/attachListener.hpp b/src/hotspot/share/services/attachListener.hpp index 093fa410d12..c98491b6ec6 100644 --- a/src/hotspot/share/services/attachListener.hpp +++ b/src/hotspot/share/services/attachListener.hpp @@ -70,8 +70,8 @@ Version 2 (since jdk24): attach operations may have any number of arguments of a If the target VM does not support version 2, client uses version 1 to enqueue operations. */ enum AttachAPIVersion: int { - ATTACH_API_V1 = 1, - ATTACH_API_V2 = 2 + ATTACH_API_V1 = 1, + ATTACH_API_V2 = 2 }; class AttachListenerThread : public JavaThread { @@ -164,6 +164,10 @@ public: arg_length_max = 1024, // maximum length of argument arg_count_max = 3 // maximum number of arguments }; + // error codes (reported as status to clients) + enum { + ATTACH_ERROR_BADVERSION = 101 + }; // name of special operation that can be enqueued when all // clients detach @@ -231,6 +235,8 @@ public: // complete operation by sending result code and any result data to the client virtual void complete(jint result, bufferedStream* result_stream) = 0; + class ReplyWriter; // forward declaration + // Helper classes/methods for platform-specific implementations. class RequestReader { public: @@ -239,11 +245,16 @@ public: virtual int read(void* buffer, int size) = 0; // Reads unsigned value, returns -1 on error. - int read_uint(); + // + // Attach client can make sanity connect/disconnect. + // In that case we get "premature EOF" error. + // If may_be_empty is true, the error is not logged. + int read_uint(bool may_be_empty = false); }; // Reads standard operation request (v1 or v2). - bool read_request(RequestReader* reader); + // Some errors known by clients are reported to error_writer. + bool read_request(RequestReader* reader, ReplyWriter* error_writer); class ReplyWriter { public: @@ -256,6 +267,7 @@ public: }; // Writes standard operation reply (to be called from 'complete' method). + bool write_reply(ReplyWriter* writer, jint result, const char* message, int message_len = -1); bool write_reply(ReplyWriter* writer, jint result, bufferedStream* result_stream); private: diff --git a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java index 69cd4dcdb99..52cfd97d7c2 100644 --- a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java +++ b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java @@ -56,6 +56,7 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { private static final Path ROOT_TMP = Path.of("root/tmp"); String socket_path; + private int ver = VERSION_1; // updated in ctor depending on detectVersion result /** * Attaches to the target VM @@ -122,14 +123,18 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { // bogus process checkPermissions(socket_path); - // Check that we can connect to the process - // - this ensures we throw the permission denied error now rather than - // later when we attempt to enqueue a command. - int s = socket(); - try { - connect(s, socket_path); - } finally { - close(s); + if (isAPIv2Enabled()) { + ver = detectVersion(); + } else { + // Check that we can connect to the process + // - this ensures we throw the permission denied error now rather than + // later when we attempt to enqueue a command. + int s = socket(); + try { + connect(s, socket_path); + } finally { + close(s); + } } } @@ -144,14 +149,10 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { } } - // protocol version - private static final String PROTOCOL_VERSION = "1"; - /** * Execute the given command in the target VM. */ InputStream execute(String cmd, Object ... args) throws AgentLoadException, IOException { - assert args.length <= 3; // includes null checkNulls(args); // did we detach? @@ -175,18 +176,9 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { IOException ioe = null; // connected - write request - // try { - writeString(s, PROTOCOL_VERSION); - writeString(s, cmd); - - for (int i = 0; i < 3; i++) { - if (i < args.length && args[i] != null) { - writeString(s, (String)args[i]); - } else { - writeString(s, ""); - } - } + SocketOutputStream writer = new SocketOutputStream(s); + writeCommand(writer, ver, cmd, args); } catch (IOException x) { ioe = x; } @@ -202,6 +194,17 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { return sis; } + private static class SocketOutputStream implements AttachOutputStream { + private int fd; + public SocketOutputStream(int fd) { + this.fd = fd; + } + @Override + public void write(byte[] buffer, int offset, int length) throws IOException { + VirtualMachineImpl.write(fd, buffer, offset, length); + } + } + /* * InputStream for the socket connection to get target VM */ @@ -273,20 +276,6 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { return (Files.isWritable(procPidRoot) ? procPidRoot : TMPDIR).toString(); } - /* - * Write/sends the given to the target VM. String is transmitted in - * UTF-8 encoding. - */ - private void writeString(int fd, String s) throws IOException { - if (s.length() > 0) { - byte[] b = s.getBytes(UTF_8); - VirtualMachineImpl.write(fd, b, 0, b.length); - } - byte b[] = new byte[1]; - b[0] = 0; - write(fd, b, 0, 1); - } - // Return the inner most namespaced PID if there is one, // otherwise return the original PID. private long getNamespacePid(long pid) throws AttachNotSupportedException, IOException {