Commit 5a77c648 authored by Steven Murdoch's avatar Steven Murdoch
Browse files

Fix issues in nickm's review of format_helper_exit_status for bug #1903 

- Responsibility of clearing hex_errno is no longer with caller
- More conservative bounds checking
- Length requirement of hex_errno documented
- Output format documented
parent 80b515b8

src/common/util.c

+33 −11
Original line number Diff line number Diff line
@@ -2879,17 +2879,34 @@ load_windows_system_library(const TCHAR *library_name) 
}

#endif



/** Format child_state and saved_errno as a hex string placed in hex_errno.

 * Called between fork and _exit, so must be signal-handler safe */

/** Format <b>child_state</b> and <b>saved_errno</b> as a hex string placed in

 * <b>hex_errno</b>.  Called between fork and _exit, so must be signal-handler

 * safe.

 *

 * <b>hex_errno</b> must have at least HEX_ERRNO_SIZE bytes available.

 *

 * The format of <b>hex_errno</b> is: "CHILD_STATE/ERRNO\n", left-padded

 * with spaces. Note that there is no trailing \0. CHILD_STATE indicates where

 * 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.

 */



void

format_helper_exit_status(unsigned char child_state, int saved_errno,

                          char *hex_errno)

{

  /* Convert errno to be unsigned for hex conversion */

  unsigned int unsigned_errno;

  char *cur;

  size_t i;



  /* Fill hex_errno with spaces, and a trailing newline (memset may

     not be signal handler safe, so we can't use it) */

  for (i = 0; i < (HEX_ERRNO_SIZE - 1); i++)

    hex_errno[i] = ' ';

  hex_errno[HEX_ERRNO_SIZE - 1] = '\n';



  /* If errno is negative, negate it */

  /* Convert errno to be unsigned for hex conversion */

  if (saved_errno < 0) {

    unsigned_errno = (unsigned int) -saved_errno;

  } else {
@@ -2899,18 +2916,27 @@ format_helper_exit_status(unsigned char child_state, int saved_errno, 
  /* Convert errno to hex (start before \n) */

  cur = hex_errno + HEX_ERRNO_SIZE - 2;



  /* Check for overflow on first iteration of the loop */

  if (cur < hex_errno)

    return;



  do {

    *cur-- = "0123456789ABCDEF"[unsigned_errno % 16];

    unsigned_errno /= 16;

  } while (unsigned_errno != 0 && cur >= hex_errno);



  /* Add on the minus side if errno was negative */

  if (saved_errno < 0)

  /* Prepend the minus sign if errno was negative */

  if (saved_errno < 0 && cur >= hex_errno)

    *cur-- = '-';



  /* Leave a gap */

  if (cur >= hex_errno)

    *cur-- = '/';



  /* Check for overflow on first iteration of the loop */

  if (cur < hex_errno)

    return;



  /* Convert child_state to hex */

  do {

    *cur-- = "0123456789ABCDEF"[child_state % 16];
@@ -2970,10 +2996,6 @@ tor_spawn_background(const char *const filename, int *stdout_read, 
     and we are not allowed to use unsafe functions between fork and exec */

  error_message_length = strlen(error_message);



  /* Fill hex_errno with spaces, and a trailing newline */

  memset(hex_errno, ' ', sizeof(hex_errno) - 1);

  hex_errno[sizeof(hex_errno) - 1] = '\n';



  child_state = CHILD_STATE_PIPE;



  /* Set up pipe for redirecting stdout and stderr of child */

src/test/test_util.c

+2 −3
Original line number Diff line number Diff line
@@ -1212,14 +1212,13 @@ test_util_load_win_lib(void *ptr) 
static void

clear_hex_errno(char *hex_errno)

{

  memset(hex_errno, ' ', HEX_ERRNO_SIZE - 2);

  hex_errno[HEX_ERRNO_SIZE - 1] = '\n';

  hex_errno[HEX_ERRNO_SIZE] = '\0';

  memset(hex_errno, '\0', HEX_ERRNO_SIZE + 1);

}



static void

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];



  (void)ptr;