commit cfa9ee5fe708539965f46a31b5d2abe4950179af Author: Steven Murdoch Steven.Murdoch@cl.cam.ac.uk Date: Thu Sep 1 13:09:38 2011 +0100

Fix double-closing a stdio stream

After a stream reached eof, we fclose it, but then test_util_spawn_background_partial_read() reads from it again, which causes an error and thus another fclose(). Some platforms are fine with this, others (e.g. debian-sid-i386) trigger a double-free() error. The actual code used by Tor (log_from_pipe() and tor_check_port_forwarding()) handle this case correctly. --- src/common/util.c | 18 +++++++++++++----- src/common/util.h | 3 ++- src/test/test_util.c | 18 ++++++++++++------ 3 files changed, 27 insertions(+), 12 deletions(-)

diff --git a/src/common/util.c b/src/common/util.c index 7cd9dd8..db6b00f 100644 --- a/src/common/util.c +++ b/src/common/util.c @@ -3583,14 +3583,19 @@ tor_read_all_handle(HANDLE h, char *buf, size_t count, * <b>process</b> is NULL, the function will return immediately if there is * nothing more to read. Otherwise data will be read until end of file, or * <b>count</b> bytes are read. Returns the number of bytes read, or -1 on - * error. */ + * error. Sets <b>eof</b> to true if <b>eof</b> is not NULL and the end of the + * file has been reached. */ ssize_t tor_read_all_handle(FILE *h, char *buf, size_t count, - const process_handle_t *process) + const process_handle_t *process, + int *eof) { size_t numread = 0; char *retval;

+ if (eof) + *eof = 0; + if (count > SIZE_T_CEILING || count > SSIZE_T_MAX) return -1;

@@ -3599,7 +3604,10 @@ tor_read_all_handle(FILE *h, char *buf, size_t count, retval = fgets(buf+numread, (int)(count-numread), h); if (NULL == retval) { if (feof(h)) { + log_debug(LD_GENERAL, "fgets() reached end of file"); fclose(h); + if (eof) + *eof = 1; break; } else { if (EAGAIN == errno) { @@ -3620,7 +3628,7 @@ tor_read_all_handle(FILE *h, char *buf, size_t count, numread += strlen(retval); }

- log_debug(LD_GENERAL, "fgets read %d bytes from handle", (int)numread); + log_debug(LD_GENERAL, "fgets() read %d bytes from handle", (int)numread); return (ssize_t)numread; } #endif @@ -3635,7 +3643,7 @@ tor_read_all_from_process_stdout(const process_handle_t *process_handle, process_handle); #else return tor_read_all_handle(process_handle->stdout_handle, buf, count, - process_handle); + process_handle, NULL); #endif }

@@ -3649,7 +3657,7 @@ tor_read_all_from_process_stderr(const process_handle_t *process_handle, process_handle); #else return tor_read_all_handle(process_handle->stderr_handle, buf, count, - process_handle); + process_handle, NULL); #endif }

diff --git a/src/common/util.h b/src/common/util.h index 6211267..c8cce39 100644 --- a/src/common/util.h +++ b/src/common/util.h @@ -391,7 +391,8 @@ ssize_t tor_read_all_handle(HANDLE h, char *buf, size_t count, const process_handle_t *process); #else ssize_t tor_read_all_handle(FILE *h, char *buf, size_t count, - const process_handle_t *process); + const process_handle_t *process, + int *eof); #endif ssize_t tor_read_all_from_process_stdout( const process_handle_t *process_handle, char *buf, size_t count); diff --git a/src/test/test_util.c b/src/test/test_util.c index 06c1be3..7faa0ae 100644 --- a/src/test/test_util.c +++ b/src/test/test_util.c @@ -1484,7 +1484,7 @@ test_util_spawn_background_partial_read(void *ptr) const int expected_status = PROCESS_STATUS_RUNNING;

int retval, exit_code; - ssize_t pos; + ssize_t pos = -1; process_handle_t process_handle; char stdout_buf[100], stderr_buf[100]; #ifdef MS_WINDOWS @@ -1499,6 +1499,7 @@ test_util_spawn_background_partial_read(void *ptr) "DONE\n", NULL }; const char *expected_err = "ERR\n"; + int eof = 0; #endif int expected_out_ctr; (void)ptr; @@ -1517,8 +1518,10 @@ test_util_spawn_background_partial_read(void *ptr) pos = tor_read_all_handle(process_handle.stdout_pipe, stdout_buf, sizeof(stdout_buf) - 1, NULL); #else + /* Check that we didn't read the end of file last time */ + tt_assert(!eof); pos = tor_read_all_handle(process_handle.stdout_handle, stdout_buf, - sizeof(stdout_buf) - 1, NULL); + sizeof(stdout_buf) - 1, NULL, &eof); #endif log_info(LD_GENERAL, "tor_read_all_handle() returned %d", (int)pos);

@@ -1526,7 +1529,7 @@ test_util_spawn_background_partial_read(void *ptr) if (0 == pos) continue;

- tt_int_op(pos, >=, 0); + tt_int_op(pos, >, 0); stdout_buf[pos] = '\0'; tt_str_op(stdout_buf, ==, expected_out[expected_out_ctr]); tt_int_op(pos, ==, strlen(expected_out[expected_out_ctr])); @@ -1539,10 +1542,13 @@ test_util_spawn_background_partial_read(void *ptr) sizeof(stdout_buf) - 1, &process_handle); #else - pos = tor_read_all_handle(process_handle.stdout_handle, stdout_buf, - sizeof(stdout_buf) - 1, - &process_handle); + if (!eof) + pos = tor_read_all_handle(process_handle.stdout_handle, stdout_buf, + sizeof(stdout_buf) - 1, + &process_handle, &eof); + tt_assert(eof) #endif + tt_int_op(pos, ==, 0);

/* Check it terminated correctly */ retval = tor_get_exit_code(process_handle, 1, &exit_code);