nihilist/tor
1
0
Fork 0
mirror of https://gitlab.torproject.org/tpo/core/tor.git synced 2024-11-10 21:23:58 +01:00
Code Issues Packages Projects Releases Wiki Activity

Handle errors even after success from ReadFileEx() and WriteFileEx().

Browse Source 
This patch adds some additional error checking after calls to
ReadFileEx() and WriteFileEx(). I have not managed to get this code to
reach the branch where `error_code` is NOT `ERROR_SUCCESS`, but MSDN
says one should check for this condition so we do so just to be safe.

See: https://bugs.torproject.org/28179
This commit is contained in:
Alexander Færøy 2018-12-20 12:57:20 +01:00
parent 44586a89ef
commit c6e041e3d8
1 changed files with 50 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
src/lib/process/process_win32.c
View File

@ -329,6 +329,7 @@ process_win32_write(struct process_t *process, buf_t *buffer)
process_win32_t *win32_process = process_get_win32_process(process);
BOOL ret = FALSE;
DWORD error_code = 0;
const size_t buffer_size = buf_datalen(buffer);
/* Windows is still writing our buffer. */
@ -350,6 +351,12 @@ process_win32_write(struct process_t *process, buf_t *buffer)
/* Read data from the process_t buffer into our intermediate buffer. */
buf_get_bytes(buffer, win32_process->stdin_handle.buffer, write_size);
/* Because of the slightly weird API for WriteFileEx() we must set this to 0
* before we call WriteFileEx() because WriteFileEx() does not reset the last
* error itself when it's succesful. See comment below after the call to
* GetLastError(). */
SetLastError(0);
/* Schedule our write. */
ret = WriteFileEx(win32_process->stdin_handle.pipe,
win32_process->stdin_handle.buffer,
@ -363,6 +370,24 @@ process_win32_write(struct process_t *process, buf_t *buffer)
return 0;
}
/* Here be dragons: According to MSDN's documentation for WriteFileEx() we
* should check GetLastError() after a call to WriteFileEx() even though the
* `ret` return value was successful. If everything is good, GetLastError()
* returns `ERROR_SUCCESS` and nothing happens.
*
* XXX(ahf): I have not managed to trigger this code while stress-testing
* this code. */
error_code = GetLastError();
if (error_code != ERROR_SUCCESS) {
/* LCOV_EXCL_START */
log_warn(LD_PROCESS, "WriteFileEx() failed after returning success: %s",
format_win32_error(error_code));
win32_process->stdin_handle.reached_eof = true;
return 0;
/* LCOV_EXCL_STOP */
}
/* This cast should be safe since our buffer can maximum be BUFFER_SIZE
* large. */
return (int)write_size;
@ -864,6 +889,7 @@ process_win32_read_from_handle(process_win32_handle_t *handle,
BOOL ret = FALSE;
int bytes_available = 0;
DWORD error_code = 0;
/* We already have a request to read data that isn't complete yet. */
if (BUG(handle->busy))
@ -887,6 +913,12 @@ process_win32_read_from_handle(process_win32_handle_t *handle,
memset(handle->buffer, 0, sizeof(handle->buffer));
}
/* Because of the slightly weird API for ReadFileEx() we must set this to 0
* before we call ReadFileEx() because ReadFileEx() does not reset the last
* error itself when it's succesful. See comment below after the call to
* GetLastError(). */
SetLastError(0);
/* Ask the Windows kernel to read data from our pipe into our buffer and call
* the callback function when it is done. */
ret = ReadFileEx(handle->pipe,
@ -901,6 +933,24 @@ process_win32_read_from_handle(process_win32_handle_t *handle,
return bytes_available;
}
/* Here be dragons: According to MSDN's documentation for ReadFileEx() we
* should check GetLastError() after a call to ReadFileEx() even though the
* `ret` return value was successful. If everything is good, GetLastError()
* returns `ERROR_SUCCESS` and nothing happens.
*
* XXX(ahf): I have not managed to trigger this code while stress-testing
* this code. */
error_code = GetLastError();
if (error_code != ERROR_SUCCESS) {
/* LCOV_EXCL_START */
log_warn(LD_PROCESS, "ReadFileEx() failed after returning success: %s",
format_win32_error(error_code));
handle->reached_eof = true;
return bytes_available;
/* LCOV_EXCL_STOP */
}
/* We mark our handle as having a pending I/O request. */
handle->busy = true;