Commit b1ad1a1d authored by Nick Mathewson's avatar Nick Mathewson 🤹
Browse files

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.
parent d0243e82

src/common/util.c

+20 −9
Original line number Diff line number Diff line
@@ -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? */

      int n;



    format_helper_exit_status(child_state, errno, hex_errno);

      n = format_helper_exit_status(child_state, errno, 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, sizeof(hex_errno));

        nbytes = write(STDOUT_FILENO, hex_errno, n);

      }

    }



    (void) nbytes;

src/common/util.h

+2 −2
Original line number Diff line number Diff line
@@ -473,7 +473,7 @@ 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 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

src/test/test_util.c

+11 −5
Original line number Diff line number Diff line
@@ -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:

  ;