diff mbox

[COMMITTED] posix: Fix posix_spawn invalid memory access

Message ID 1458508785-31315-1-git-send-email-adhemerval.zanella@linaro.com
State New
Headers show

Commit Message

Adhemerval Zanella Netto March 20, 2016, 9:19 p.m. UTC 
Current Linux posix_spawn spawn do not test if the pid argument is
valid before trying to update it for success case.  This patch fixes
it.

Tested on x86_64 and i686.

	* sysdeps/unix/sysv/linux/spawni.c (__spawnix): Fix invalid memory
	access where posix_spawn success and pid argument is null.
	* posix/tst-spawn.c (do_test): Add posix_spawn null pid argument for
	success case.
---
 ChangeLog                        | 7 +++++++
 posix/tst-spawn.c                | 4 ++++
 sysdeps/unix/sysv/linux/spawni.c | 2 +-
 3 files changed, 12 insertions(+), 1 deletion(-)

Comments

Florian Weimer March 21, 2016, 1:54 p.m. UTC | #1 
* Adhemerval Zanella:

> -  if (!ec && new_pid)
> +  if (!ec && pid)

I think the general convention is not to perform implicit NULL checks,
so it should be “pid != NULL”.
Andreas Schwab March 21, 2016, 2:10 p.m. UTC | #2 
Florian Weimer <fw@deneb.enyo.de> writes:

> * Adhemerval Zanella:
>
>> -  if (!ec && new_pid)
>> +  if (!ec && pid)
>
> I think the general convention is not to perform implicit NULL checks,
> so it should be “pid != NULL”.

For the same reason "!ec" should be changed to "ec == 0".

Andreas.
diff mbox

Patch

diff --git a/ChangeLog b/ChangeLog
index ceee215..6bd5a11 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@ 
+2016-03-20  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
+
+	* sysdeps/unix/sysv/linux/spawni.c (__spawnix): Fix invalid memory
+	access where posix_spawn success and pid argument is null.
+	* posix/tst-spawn.c (do_test): Add posix_spawn null pid argument for
+	success case.
+
 2016-03-20  Samuel Thibault  <samuel.thibault@ens-lyon.org>:
 
 	* sysdeps/mach/hurd/i386/c++-types.data: New file.
diff --git a/posix/tst-spawn.c b/posix/tst-spawn.c
index 68f4357..c046098 100644
--- a/posix/tst-spawn.c
+++ b/posix/tst-spawn.c
@@ -257,6 +257,10 @@  do_test (int argc, char *argv[])
    if (posix_spawn (&pid, argv[1], &actions, NULL, spargv, environ) != 0)
      error (EXIT_FAILURE, errno, "posix_spawn");
 
+   /* Same test but with a NULL pid argument.  */
+   if (posix_spawn (NULL, argv[1], &actions, NULL, spargv, environ) != 0)
+     error (EXIT_FAILURE, errno, "posix_spawn");
+
    /* Cleanup.  */
    if (posix_spawn_file_actions_destroy (&actions) != 0)
      error (EXIT_FAILURE, errno, "posix_spawn_file_actions_destroy");
diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
index 454462b..cb80cea 100644
--- a/sysdeps/unix/sysv/linux/spawni.c
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -381,7 +381,7 @@  __spawnix (pid_t * pid, const char *file,
 
   close_not_cancel (args.pipe[0]);
 
-  if (!ec && new_pid)
+  if (!ec && pid)
     *pid = new_pid;
 
   __sigprocmask (SIG_SETMASK, &args.oldmask, 0);