8343344: Windows attach logic failed to handle a failed open on a pipe

Reviewed-by: kevinw, cjplummer
This commit is contained in:
Alex Menkov 2024-11-12 20:24:25 +00:00
parent 63eb4853f6
commit a4e2c20849
2 changed files with 27 additions and 13 deletions

View File

@ -84,7 +84,8 @@ public:
0, // default attributes 0, // default attributes
nullptr); // no template file nullptr); // no template file
if (_hPipe == INVALID_HANDLE_VALUE) { if (_hPipe == INVALID_HANDLE_VALUE) {
log_error(attach)("could not open (%d) pipe %s", GetLastError(), pipe); log_error(attach)("could not open %s (%d) pipe %s",
(write_only ? "write-only" : "read-write"), GetLastError(), pipe);
return false; return false;
} }
return true; return true;
@ -106,7 +107,11 @@ public:
(DWORD)size, (DWORD)size,
&nread, &nread,
nullptr); // not overlapped nullptr); // not overlapped
return fSuccess ? (int)nread : -1; if (!fSuccess) {
log_error(attach)("pipe read error (%d)", GetLastError());
return -1;
}
return (int)nread;
} }
// ReplyWriter // ReplyWriter
@ -118,7 +123,11 @@ public:
(DWORD)size, (DWORD)size,
&written, &written,
nullptr); // not overlapped nullptr); // not overlapped
return fSuccess ? (int)written : -1; if (!fSuccess) {
log_error(attach)("pipe write error (%d)", GetLastError());
return -1;
}
return (int)written;
} }
void flush() override { void flush() override {
@ -138,12 +147,12 @@ private:
public: public:
// for v1 pipe must be write-only // for v1 pipe must be write-only
void open_pipe(const char* pipe_name, bool write_only) { bool open_pipe(const char* pipe_name, bool write_only) {
_pipe.open(pipe_name, write_only); return _pipe.open(pipe_name, write_only);
} }
bool read_request() { bool read_request() {
return AttachOperation::read_request(&_pipe); return AttachOperation::read_request(&_pipe);
} }
public: public:
@ -390,13 +399,17 @@ Win32AttachOperation* Win32AttachListener::dequeue() {
for (int i = 0; i < AttachOperation::arg_count_max; i++) { for (int i = 0; i < AttachOperation::arg_count_max; i++) {
op->append_arg(request->arg(i)); op->append_arg(request->arg(i));
} }
op->open_pipe(request->pipe(), true/*write-only*/); if (!op->open_pipe(request->pipe(), true/*write-only*/)) {
// error is already logged
delete op;
op = nullptr;
}
break; break;
case ATTACH_API_V2: case ATTACH_API_V2:
op = new Win32AttachOperation(); op = new Win32AttachOperation();
op->open_pipe(request->pipe(), false/*write-only*/); if (!op->open_pipe(request->pipe(), false/*write-only*/)
if (!op->read_request()) { || !op->read_request()) {
log_error(attach)("AttachListener::dequeue, reading request ERROR"); // error is already logged
delete op; delete op;
op = nullptr; op = nullptr;
} }

View File

@ -628,15 +628,16 @@ bool AttachOperation::read_request(RequestReader* reader) {
buffer_size = (name_length_max + 1) + arg_count_max * (arg_length_max + 1); buffer_size = (name_length_max + 1) + arg_count_max * (arg_length_max + 1);
min_str_count = 1 /*name*/ + arg_count_max; min_str_count = 1 /*name*/ + arg_count_max;
break; break;
case ATTACH_API_V2: // <ver>0<size>0<cmd>0<arg>0<arg>0<arg>0 case ATTACH_API_V2: // <ver>0<size>0<cmd>0(<arg>0)* (any number of arguments)
if (AttachListener::get_supported_version() < 2) { if (AttachListener::get_supported_version() < 2) {
log_error(attach)("Failed to read request: v2 is unsupported ot disabled"); log_error(attach)("Failed to read request: v2 is unsupported or disabled");
return false; return false;
} }
// read size of the data // read size of the data
buffer_size = reader->read_uint(); buffer_size = reader->read_uint();
if (buffer_size < 0) { if (buffer_size < 0) {
log_error(attach)("Failed to read request: negative request size (%d)", buffer_size);
return false; return false;
} }
log_debug(attach)("v2 request, data size = %d", buffer_size); log_debug(attach)("v2 request, data size = %d", buffer_size);
@ -646,7 +647,7 @@ bool AttachOperation::read_request(RequestReader* reader) {
log_error(attach)("Failed to read request: too big"); log_error(attach)("Failed to read request: too big");
return false; return false;
} }
// Must contain exact 'buffer_size' bytes. // Must contain exactly 'buffer_size' bytes.
min_read_size = buffer_size; min_read_size = buffer_size;
break; break;
default: default: