diff mbox series

stdio: Do not ignore posix_spawn error on popen (BZ #29016)

Message ID 20221206164503.2534492-1-adhemerval.zanella@linaro.org
State New
Headers show
Series stdio: Do not ignore posix_spawn error on popen (BZ #29016) | expand

Commit Message

Adhemerval Zanella Netto Dec. 6, 2022, 4:45 p.m. UTC 
To correctly return error in case of default shell is not present.

Checked on x86_64-linux-gnu.
---
 libio/iopopen.c           | 38 ++++++++++++++++++++++----------------
 stdio-common/Makefile     |  3 +++
 stdio-common/tst-popen3.c | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 16 deletions(-)
 create mode 100644 stdio-common/tst-popen3.c

Comments

Andreas Schwab Dec. 6, 2022, 4:53 p.m. UTC | #1 
On Dez 06 2022, Adhemerval Zanella wrote:

> To correctly return error in case of default shell is not present.

Ok.

> diff --git a/libio/iopopen.c b/libio/iopopen.c
> index 06778cf110..e4be6c9779 100644
> --- a/libio/iopopen.c
> +++ b/libio/iopopen.c
> @@ -66,11 +66,12 @@ unlock (void *not_used)
>     be close (by transversing the proc_file_chain list) and the insertion of a
>     new one after a successful posix_spawn this function should be called
>     with proc_file_chain_lock acquired.  */
> -static bool
> +static int
>  spawn_process (posix_spawn_file_actions_t *fa, FILE *fp, const char *command,
>  	       int do_cloexec, int pipe_fds[2], int parent_end, int child_end,
>  	       int child_pipe_fd)
>  {
> +  int err = 0;

I don't think this needs to be initialized here.

> diff --git a/stdio-common/tst-popen3.c b/stdio-common/tst-popen3.c
> new file mode 100644
> index 0000000000..add5e97c09
> --- /dev/null
> +++ b/stdio-common/tst-popen3.c
> @@ -0,0 +1,38 @@
> +/* Check if popen return a correct error code if the default shell does not

s/return/returns/

> +   exists (BZ#29016).

s/exists/exist/
diff mbox series

Patch

diff --git a/libio/iopopen.c b/libio/iopopen.c
index 06778cf110..e4be6c9779 100644
--- a/libio/iopopen.c
+++ b/libio/iopopen.c
@@ -66,11 +66,12 @@  unlock (void *not_used)
    be close (by transversing the proc_file_chain list) and the insertion of a
    new one after a successful posix_spawn this function should be called
    with proc_file_chain_lock acquired.  */
-static bool
+static int
 spawn_process (posix_spawn_file_actions_t *fa, FILE *fp, const char *command,
 	       int do_cloexec, int pipe_fds[2], int parent_end, int child_end,
 	       int child_pipe_fd)
 {
+  int err = 0;
 
   for (struct _IO_proc_file *p = proc_file_chain; p; p = p->next)
     {
@@ -79,15 +80,19 @@  spawn_process (posix_spawn_file_actions_t *fa, FILE *fp, const char *command,
       /* If any stream from previous popen() calls has fileno
 	 child_pipe_fd, it has been already closed by the adddup2 action
 	 above.  */
-      if (fd != child_pipe_fd
-	  && __posix_spawn_file_actions_addclose (fa, fd) != 0)
-	return false;
+      if (fd != child_pipe_fd)
+	{
+	  err = __posix_spawn_file_actions_addclose (fa, fd);
+	  if (err != 0)
+	    return err;
+	}
     }
 
-  if (__posix_spawn (&((_IO_proc_file *) fp)->pid, _PATH_BSHELL, fa, 0,
-		     (char *const[]){ (char*) "sh", (char*) "-c",
-		     (char *) command, NULL }, __environ) != 0)
-    return false;
+  err = __posix_spawn (&((_IO_proc_file *) fp)->pid, _PATH_BSHELL, fa, 0,
+		       (char *const[]){ (char*) "sh", (char*) "-c",
+		       (char *) command, NULL }, __environ);
+  if (err != 0)
+    return err;
 
   __close_nocancel (pipe_fds[child_end]);
 
@@ -101,7 +106,7 @@  spawn_process (posix_spawn_file_actions_t *fa, FILE *fp, const char *command,
   ((_IO_proc_file *) fp)->next = proc_file_chain;
   proc_file_chain = (_IO_proc_file *) fp;
 
-  return true;
+  return 0;
 }
 
 FILE *
@@ -112,7 +117,7 @@  _IO_new_proc_open (FILE *fp, const char *command, const char *mode)
   int parent_end, child_end;
   int pipe_fds[2];
   int child_pipe_fd;
-  bool spawn_ok;
+  int err;
 
   int do_read = 0;
   int do_write = 0;
@@ -185,16 +190,17 @@  _IO_new_proc_open (FILE *fp, const char *command, const char *mode)
       pipe_fds[child_end] = tmp;
     }
 
-  if (__posix_spawn_file_actions_adddup2 (&fa, pipe_fds[child_end],
-      child_pipe_fd) != 0)
+  err = __posix_spawn_file_actions_adddup2 (&fa, pipe_fds[child_end],
+					    child_pipe_fd);
+  if (err != 0)
     goto spawn_failure;
 
 #ifdef _IO_MTSAFE_IO
   _IO_cleanup_region_start_noarg (unlock);
   _IO_lock_lock (proc_file_chain_lock);
 #endif
-  spawn_ok = spawn_process (&fa, fp, command, do_cloexec, pipe_fds,
-			    parent_end, child_end, child_pipe_fd);
+  err = spawn_process (&fa, fp, command, do_cloexec, pipe_fds, parent_end,
+		       child_end, child_pipe_fd);
 #ifdef _IO_MTSAFE_IO
   _IO_lock_unlock (proc_file_chain_lock);
   _IO_cleanup_region_end (0);
@@ -202,12 +208,12 @@  _IO_new_proc_open (FILE *fp, const char *command, const char *mode)
 
   __posix_spawn_file_actions_destroy (&fa);
 
-  if (!spawn_ok)
+  if (err != 0)
     {
+      __set_errno (err);
     spawn_failure:
       __close_nocancel (pipe_fds[child_end]);
       __close_nocancel (pipe_fds[parent_end]);
-      __set_errno (ENOMEM);
       return NULL;
     }
 
diff --git a/stdio-common/Makefile b/stdio-common/Makefile
index fe57dbdf56..9a9a37ae08 100644
--- a/stdio-common/Makefile
+++ b/stdio-common/Makefile
@@ -215,6 +215,9 @@  tests := \
   xbug \
   # tests
 
+tests-container += \
+  tst-popen3
+
 generated += \
   errlist-data-aux-shared.S \
   errlist-data-aux.S \
diff --git a/stdio-common/tst-popen3.c b/stdio-common/tst-popen3.c
new file mode 100644
index 0000000000..add5e97c09
--- /dev/null
+++ b/stdio-common/tst-popen3.c
@@ -0,0 +1,38 @@ 
+/* Check if popen return a correct error code if the default shell does not
+   exists (BZ#29016).
+
+   Copyright (C) 2022 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#include <paths.h>
+#include <stdio.h>
+#include <errno.h>
+#include <support/check.h>
+#include <support/xunistd.h>
+
+int
+do_test (void)
+{
+  xunlink (_PATH_BSHELL);
+
+  FILE *f = popen ("/non-existent", "r");
+  TEST_VERIFY (f == NULL);
+  TEST_COMPARE (errno, ENOENT);
+  return 0;
+}
+
+#include <support/test-driver.c>