commit b1ad1a1d0266a20bb0dac15e65abe7b65a74e8a0
Author: Nick Mathewson <nickm(a)torproject.org>
Date: Sat Jun 23 15:30:01 2012 -0400
Resolve crash caused by format_helper_exit_status changes in #5557
Because the string output was no longer equal in length to
HEX_ERRNO_SIZE, the write() call would add some extra spaces and
maybe a NUL, and the NUL would trigger an assert in
get_string_from_pipe.
Fixes bug 6225; bug not in any released version of Tor.
---
src/common/util.c | 29 ++++++++++++++++++++---------
src/common/util.h | 4 ++--
src/test/test_util.c | 16 +++++++++++-----
3 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/src/common/util.c b/src/common/util.c
index cb8ff85..8cb013e 100644
--- a/src/common/util.c
+++ b/src/common/util.c
@@ -3264,8 +3264,11 @@ format_hex_number_for_helper_exit_status(unsigned int x, char *buf,
* in the processs of starting the child process did the failure occur (see
* CHILD_STATE_* macros for definition), and SAVED_ERRNO is the value of
* errno when the failure occurred.
+ *
+ * On success return the number of characters added to hex_errno, not counting
+ * the terminating NUL; return -1 on error.
*/
-void
+int
format_helper_exit_status(unsigned char child_state, int saved_errno,
char *hex_errno)
{
@@ -3273,6 +3276,7 @@ format_helper_exit_status(unsigned char child_state, int saved_errno,
int written, left;
char *cur;
size_t i;
+ int res = -1;
/* Fill hex_errno with spaces, and a trailing newline (memset may
not be signal handler safe, so we can't use it) */
@@ -3343,6 +3347,8 @@ format_helper_exit_status(unsigned char child_state, int saved_errno,
*cur++ = '\n';
*cur++ = '\0';
+ res = cur - hex_errno - 1;
+
goto done;
err:
@@ -3353,7 +3359,7 @@ format_helper_exit_status(unsigned char child_state, int saved_errno,
*hex_errno = '\0';
done:
- return;
+ return res;
}
/* Maximum number of file descriptors, if we cannot get it via sysconf() */
@@ -3695,15 +3701,20 @@ tor_spawn_background(const char *const filename, const char **argv,
child_state = CHILD_STATE_FAILEXEC;
error:
- /* XXX: are we leaking fds from the pipe? */
+ {
+ /* XXX: are we leaking fds from the pipe? */
+ int n;
- format_helper_exit_status(child_state, errno, hex_errno);
+ n = format_helper_exit_status(child_state, errno, hex_errno);
- /* Write the error message. GCC requires that we check the return
- value, but there is nothing we can do if it fails */
- /* TODO: Don't use STDOUT, use a pipe set up just for this purpose */
- nbytes = write(STDOUT_FILENO, error_message, error_message_length);
- nbytes = write(STDOUT_FILENO, hex_errno, sizeof(hex_errno));
+ if (n >= 0) {
+ /* Write the error message. GCC requires that we check the return
+ value, but there is nothing we can do if it fails */
+ /* TODO: Don't use STDOUT, use a pipe set up just for this purpose */
+ nbytes = write(STDOUT_FILENO, error_message, error_message_length);
+ nbytes = write(STDOUT_FILENO, hex_errno, n);
+ }
+ }
(void) nbytes;
diff --git a/src/common/util.h b/src/common/util.h
index 6b7c6fb..a2ab0cc 100644
--- a/src/common/util.h
+++ b/src/common/util.h
@@ -473,8 +473,8 @@ void tor_process_handle_destroy(process_handle_t *process_handle,
int format_hex_number_for_helper_exit_status(unsigned int x, char *buf,
int max_len);
-void format_helper_exit_status(unsigned char child_state,
- int saved_errno, char *hex_errno);
+int format_helper_exit_status(unsigned char child_state,
+ int saved_errno, char *hex_errno);
/* Space for hex values of child state, a slash, saved_errno (with
leading minus) and newline (no null) */
diff --git a/src/test/test_util.c b/src/test/test_util.c
index d71d280..fab9595 100644
--- a/src/test/test_util.c
+++ b/src/test/test_util.c
@@ -2138,28 +2138,34 @@ test_util_exit_status(void *ptr)
{
/* Leave an extra byte for a \0 so we can do string comparison */
char hex_errno[HEX_ERRNO_SIZE + 1];
+ int n;
(void)ptr;
clear_hex_errno(hex_errno);
- format_helper_exit_status(0, 0, hex_errno);
+ n = format_helper_exit_status(0, 0, hex_errno);
test_streq("0/0\n", hex_errno);
+ test_eq(n, strlen(hex_errno));
clear_hex_errno(hex_errno);
- format_helper_exit_status(0, 0x7FFFFFFF, hex_errno);
+ n = format_helper_exit_status(0, 0x7FFFFFFF, hex_errno);
test_streq("0/7FFFFFFF\n", hex_errno);
+ test_eq(n, strlen(hex_errno));
clear_hex_errno(hex_errno);
- format_helper_exit_status(0xFF, -0x80000000, hex_errno);
+ n = format_helper_exit_status(0xFF, -0x80000000, hex_errno);
test_streq("FF/-80000000\n", hex_errno);
+ test_eq(n, strlen(hex_errno));
clear_hex_errno(hex_errno);
- format_helper_exit_status(0x7F, 0, hex_errno);
+ n = format_helper_exit_status(0x7F, 0, hex_errno);
test_streq("7F/0\n", hex_errno);
+ test_eq(n, strlen(hex_errno));
clear_hex_errno(hex_errno);
- format_helper_exit_status(0x08, -0x242, hex_errno);
+ n = format_helper_exit_status(0x08, -0x242, hex_errno);
test_streq("8/-242\n", hex_errno);
+ test_eq(n, strlen(hex_errno));
done:
;