This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/3] posix: Remove dynamic memory allocation from execl{e,p}

Adhemerval Zanella wrote:
+  int argc;
+  va_list ap;
+  va_start (ap, arg);
+  for (argc = 1; va_arg (ap, const char *); argc++)
+    continue;

With my "no arbitrary limits" hat on, I noticed that this has undefined behavior if more than INT_MAX arguments are passed to execl. The existing code is no saint in this area (it messes up badly if more than UINT_MAX args are passed), but the new code should not make things worse, and we might as well fix the UINT_MAX bug while we're at it.

Attached please find a contrived test case illustrating the bug on x86-64. This test succeeds on x86-64 now (in that the program prints "execl: Cannot allocate memory" and exits with status 0) but could crash with the proposed patch. Perhaps you can add this to the glibc test cases while you're at it.

/* This program assumes x86-64.  Use 'ulimit -s 30000000' before running.  */

#include <stdio.h>
#include <unistd.h>

main (void)
  /* Number of extra arguments to pass to execl, counting
     the trailing NULL.  Must be a multiple of 2.  */
  long n = 2147483648;

  char x[] = "";

  /* Push the the trailing NULL onto the stack.  */
  asm ("pushq $0");

  /* Push the the trailing arguments onto the stack.  */
  for (long i = 1; i < n; i++)
    asm ("pushq %0" :: "r" (x));

  /* Call execl, without a trailing NULL.  GCC may warn
     about this.  */
  execl ("/bin/true", "true", x, x, x, x, x, x, x, x);

  /* Pop the added arguments off the stack.  */
  asm ("addq %0, %%rsp" :: "r" (n * sizeof (char *)));

  perror ("execl");
  _exit (0);

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]