public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH 2/2] posix: New Linux posix_spawn{p} implementation
  2016-01-27 12:32 [PATCH v2 1/2] posix: execvpe cleanup Adhemerval Zanella
@ 2016-01-27 12:32 ` Adhemerval Zanella
  2016-01-27 16:17   ` Paul Eggert
  2016-01-27 13:34 ` [PATCH v2 1/2] posix: execvpe cleanup Andreas Schwab
  2016-01-27 16:17 ` Paul Eggert
  2 siblings, 1 reply; 10+ messages in thread
From: Adhemerval Zanella @ 2016-01-27 12:32 UTC (permalink / raw)
  To: libc-alpha

This patch implements a new posix_spawn{p} implementation for Linux.  The main
difference is it uses the clone syscall directly with CLONE_VM and CLONE_VFORK
flags and a direct allocated stack.  The new stack and start function solves
most the vfork limitation (possible parent clobber due stack spilling).  The
remaning issue are related to signal handling:

  1. That no signal handlers must run in child context, to avoid corrupt
     parent's state.
  2. Child must synchronize with parent to enforce stack deallocation and
     to possible return execv issues.

The first one is solved by blocking all signals in child, even NPTL-internal
ones (SIGCANCEL and SIGSETXID).  The second issue is done by a stack allocation
in parent and a synchronization with using a pipe or waitpid (in case or error).
The pipe has the advantage of allowing the child signal an exec error (checked
with new tst-spawn2 test).

There is an inherent race condition in pipe2 usage for architectures that do not
support the syscall directly.  In such cases the a pipe plus fctnl is used
instead and it may lead to file descriptor leak in parent (as decribed by fcntl
documentation).

The child process stack is allocate with a mmap with MAP_STACK flag using
default architecture stack size.  Although it is slower than use a stack buffer
from parent, it allows some slack for the compatibility code to run scripts
with no shebang (which may use a buffer with size depending of argument list
count).

Performance should be similar to the vfork default posix implementation and
way faster than fork path (vfork on mostly linux ports are basically
clone with CLONE_VM plus CLONE_VFORK).  The only difference is the syscalls
required for the stack allocation/deallocation.

It fixes BZ#10354, BZ#14750, and BZ#18433.

Tested on i386, x86_64, powerpc64le, and aarch64.

	[BZ #14750]
	[BZ #10354]
	[BZ #18433]
	* include/sched.h (__clone): Add hidden prototype.
	(__clone2): Likewise.
	* include/unistd.h (__dup): Likewise.
	* posix/Makefile (tests): Add tst-spawn2.
	* posix/tst-spawn2.c: New file.
	* sysdeps/posix/dup.c (__dup): Add hidden definition.
	* sysdeps/unix/sysv/linux/aarch64/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/alpha/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/arm/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/hppa/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/i386/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/ia64/clone2.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/m68k/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/microblaze/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/mips/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/nios2/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S (__clone):
	Likewise.
	* sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S (__clone):
	Likewise.
	* sysdeps/unix/sysv/linux/s390/s390-32/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/s390/s390-64/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/sh/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/sparc/sparc32/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/sparc/sparc64/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/tile/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/x86_64/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/nptl-signals.h
	(____nptl_is_internal_signal): New function.
	* sysdeps/unix/sysv/linux/spawni.c: New file.
---
 ChangeLog                                         |  34 ++
 include/sched.h                                   |   2 +
 include/unistd.h                                  |   1 +
 posix/Makefile                                    |   2 +-
 posix/tst-spawn2.c                                |  73 ++++
 sysdeps/posix/dup.c                               |   2 +-
 sysdeps/unix/sysv/linux/aarch64/clone.S           |   1 +
 sysdeps/unix/sysv/linux/alpha/clone.S             |   1 +
 sysdeps/unix/sysv/linux/arm/clone.S               |   1 +
 sysdeps/unix/sysv/linux/hppa/clone.S              |   1 +
 sysdeps/unix/sysv/linux/i386/clone.S              |   1 +
 sysdeps/unix/sysv/linux/ia64/clone2.S             |   2 +
 sysdeps/unix/sysv/linux/m68k/clone.S              |   1 +
 sysdeps/unix/sysv/linux/microblaze/clone.S        |   1 +
 sysdeps/unix/sysv/linux/mips/clone.S              |   1 +
 sysdeps/unix/sysv/linux/nios2/clone.S             |   1 +
 sysdeps/unix/sysv/linux/nptl-signals.h            |  10 +
 sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S |   1 +
 sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S |   1 +
 sysdeps/unix/sysv/linux/s390/s390-32/clone.S      |   2 +
 sysdeps/unix/sysv/linux/s390/s390-64/clone.S      |   2 +
 sysdeps/unix/sysv/linux/sh/clone.S                |   1 +
 sysdeps/unix/sysv/linux/sparc/sparc32/clone.S     |   1 +
 sysdeps/unix/sysv/linux/sparc/sparc64/clone.S     |   1 +
 sysdeps/unix/sysv/linux/spawni.c                  | 426 ++++++++++++++++++++++
 sysdeps/unix/sysv/linux/tile/clone.S              |   1 +
 sysdeps/unix/sysv/linux/x86_64/clone.S            |   1 +
 27 files changed, 570 insertions(+), 2 deletions(-)
 create mode 100644 posix/tst-spawn2.c
 create mode 100644 sysdeps/unix/sysv/linux/spawni.c

diff --git a/ChangeLog b/ChangeLog
index 892062c..b8175eb 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,39 @@
 2016-01-25  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
 
+	[BZ #14750]
+	[BZ #10354]
+	[BZ #18433]
+	* include/sched.h (__clone): Add hidden prototype.
+	(__clone2): Likewise.
+	* include/unistd.h (__dup): Likewise.
+	* posix/Makefile (tests): Add tst-spawn2.
+	* posix/tst-spawn2.c: New file.
+	* sysdeps/posix/dup.c (__dup): Add hidden definition.
+	* sysdeps/unix/sysv/linux/aarch64/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/alpha/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/arm/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/hppa/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/i386/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/ia64/clone2.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/m68k/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/microblaze/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/mips/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/nios2/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S (__clone):
+	Likewise.
+	* sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S (__clone):
+	Likewise.
+	* sysdeps/unix/sysv/linux/s390/s390-32/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/s390/s390-64/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/sh/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/sparc/sparc32/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/sparc/sparc64/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/tile/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/x86_64/clone.S (__clone): Likewise.
+	* sysdeps/unix/sysv/linux/nptl-signals.h
+	(____nptl_is_internal_signal): New function.
+	* sysdeps/unix/sysv/linux/spawni.c: New file.
+
 	* posix/execvpe.c (__execvpe): Remove dynamic allocation.
 	* posix/Makefile (tests): Add tst-execvpe{1,2,3,4,5,6}.
 	* posix/tst-execvp1.c (do_test): Use a macro to call execvp.
diff --git a/include/sched.h b/include/sched.h
index 4f59397..b4d7406 100644
--- a/include/sched.h
+++ b/include/sched.h
@@ -19,7 +19,9 @@ extern int __sched_rr_get_interval (__pid_t __pid, struct timespec *__t);
 /* These are Linux specific.  */
 extern int __clone (int (*__fn) (void *__arg), void *__child_stack,
 		    int __flags, void *__arg, ...);
+libc_hidden_proto (__clone)
 extern int __clone2 (int (*__fn) (void *__arg), void *__child_stack_base,
 		     size_t __child_stack_size, int __flags, void *__arg, ...);
+libc_hidden_proto (__clone2)
 #endif
 #endif
diff --git a/include/unistd.h b/include/unistd.h
index 5152f64..d2802b2 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -81,6 +81,7 @@ char *__canonicalize_directory_name_internal (const char *__thisdir,
 					      size_t __size) attribute_hidden;
 
 extern int __dup (int __fd);
+libc_hidden_proto (__dup)
 extern int __dup2 (int __fd, int __fd2);
 libc_hidden_proto (__dup2)
 extern int __dup3 (int __fd, int __fd2, int flags);
diff --git a/posix/Makefile b/posix/Makefile
index e251363..906817e 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -93,7 +93,7 @@ tests		:= tstgetopt testfnm runtests runptests	     \
 xtests		:= bug-ga2
 ifeq (yes,$(build-shared))
 test-srcs	:= globtest
-tests           += wordexp-test tst-exec tst-spawn
+tests           += wordexp-test tst-exec tst-spawn tst-spawn2
 endif
 tests-static	= tst-exec-static tst-spawn-static
 tests		+= $(tests-static)
diff --git a/posix/tst-spawn2.c b/posix/tst-spawn2.c
new file mode 100644
index 0000000..41bfde8
--- /dev/null
+++ b/posix/tst-spawn2.c
@@ -0,0 +1,73 @@
+/* Further tests for spawn in case of invalid binary paths.
+   Copyright (C) 2016 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <spawn.h>
+#include <error.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/wait.h>
+
+#include <stdio.h>
+
+int
+posix_spawn_test (void)
+{
+  /* Check if posix_spawn correctly returns an error and an invalid pid
+     by trying to spawn an invalid binary.  */
+
+  const char *program = "/path/to/invalid/binary";
+  char * const args[] = { 0 };
+  pid_t pid = -1;
+
+  int ret = posix_spawn (&pid, program, 0, 0, args, environ);
+  if (ret != ENOENT)
+    error (EXIT_FAILURE, errno, "posix_spawn");
+
+  /* POSIX states the value returned on pid variable in case of an error
+     is not specified.  GLIBC will update the value iff the child 
+     execution is successful.  */
+  if (pid != -1)
+    error (EXIT_FAILURE, errno, "posix_spawn returned pid != -1");
+
+  /* Check if no child is actually created.  */
+  ret = waitpid (-1, NULL, 0);
+  if (ret != -1 || errno != ECHILD)
+    error (EXIT_FAILURE, errno, "waitpid");
+
+  /* Same as before, but with posix_spawnp.  */
+  char *args2[] = { (char*) program, 0 };
+
+  ret = posix_spawnp (&pid, args2[0], 0, 0, args2, environ);
+  if (ret != ENOENT)
+    error (EXIT_FAILURE, errno, "posix_spawnp");
+  
+  if (pid != -1)
+    error (EXIT_FAILURE, errno, "posix_spawnp returned pid != -1");
+
+  ret = waitpid (-1, NULL, 0);
+  if (ret != -1 || errno != ECHILD)
+    error (EXIT_FAILURE, errno, "waitpid");
+
+  return 0;
+}
+
+#define TEST_FUNCTION  posix_spawn_test ()
+#include "../test-skeleton.c"
+
diff --git a/sysdeps/posix/dup.c b/sysdeps/posix/dup.c
index 9525e76..abf3ff5 100644
--- a/sysdeps/posix/dup.c
+++ b/sysdeps/posix/dup.c
@@ -26,5 +26,5 @@ __dup (int fd)
 {
   return fcntl (fd, F_DUPFD, 0);
 }
-
+libc_hidden_def (__dup)
 weak_alias (__dup, dup)
diff --git a/sysdeps/unix/sysv/linux/aarch64/clone.S b/sysdeps/unix/sysv/linux/aarch64/clone.S
index 596fb9c..8f3ab40 100644
--- a/sysdeps/unix/sysv/linux/aarch64/clone.S
+++ b/sysdeps/unix/sysv/linux/aarch64/clone.S
@@ -93,4 +93,5 @@ thread_start:
 	cfi_endproc
 	.size thread_start, .-thread_start
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/alpha/clone.S b/sysdeps/unix/sysv/linux/alpha/clone.S
index ec9c06d..556aa63 100644
--- a/sysdeps/unix/sysv/linux/alpha/clone.S
+++ b/sysdeps/unix/sysv/linux/alpha/clone.S
@@ -137,4 +137,5 @@ thread_start:
 	cfi_endproc
 	.end thread_start
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/arm/clone.S b/sysdeps/unix/sysv/linux/arm/clone.S
index 460a8f5..c103123 100644
--- a/sysdeps/unix/sysv/linux/arm/clone.S
+++ b/sysdeps/unix/sysv/linux/arm/clone.S
@@ -93,4 +93,5 @@ PSEUDO_END (__clone)
 
 	.fnend
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/hppa/clone.S b/sysdeps/unix/sysv/linux/hppa/clone.S
index e80fd8d..0a18137 100644
--- a/sysdeps/unix/sysv/linux/hppa/clone.S
+++ b/sysdeps/unix/sysv/linux/hppa/clone.S
@@ -175,4 +175,5 @@ ENTRY(__clone)
 
 PSEUDO_END(__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/i386/clone.S b/sysdeps/unix/sysv/linux/i386/clone.S
index 7d818c1..ef447d1 100644
--- a/sysdeps/unix/sysv/linux/i386/clone.S
+++ b/sysdeps/unix/sysv/linux/i386/clone.S
@@ -139,4 +139,5 @@ L(nomoregetpid):
 	cfi_startproc
 PSEUDO_END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/ia64/clone2.S b/sysdeps/unix/sysv/linux/ia64/clone2.S
index 9b257b3..fc046eb 100644
--- a/sysdeps/unix/sysv/linux/ia64/clone2.S
+++ b/sysdeps/unix/sysv/linux/ia64/clone2.S
@@ -96,6 +96,8 @@ ENTRY(__clone2)
 	ret			/* Not reached.		*/
 PSEUDO_END(__clone2)
 
+libc_hidden_def (__clone2)
+
 /* For now we leave __clone undefined.  This is unlikely to be a	*/
 /* problem, since at least the i386 __clone in glibc always failed	*/
 /* with a 0 sp (eventhough the kernel explicitly handled it).		*/
diff --git a/sysdeps/unix/sysv/linux/m68k/clone.S b/sysdeps/unix/sysv/linux/m68k/clone.S
index 8b40df2..33474cc 100644
--- a/sysdeps/unix/sysv/linux/m68k/clone.S
+++ b/sysdeps/unix/sysv/linux/m68k/clone.S
@@ -127,4 +127,5 @@ donepid:
 	cfi_startproc
 PSEUDO_END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/microblaze/clone.S b/sysdeps/unix/sysv/linux/microblaze/clone.S
index 035d88b..cbc95ce 100644
--- a/sysdeps/unix/sysv/linux/microblaze/clone.S
+++ b/sysdeps/unix/sysv/linux/microblaze/clone.S
@@ -69,4 +69,5 @@ L(thread_start):
 	nop
 PSEUDO_END(__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone,clone)
diff --git a/sysdeps/unix/sysv/linux/mips/clone.S b/sysdeps/unix/sysv/linux/mips/clone.S
index 755e8cc..feb8250 100644
--- a/sysdeps/unix/sysv/linux/mips/clone.S
+++ b/sysdeps/unix/sysv/linux/mips/clone.S
@@ -166,4 +166,5 @@ L(gotpid):
 
 	END(__thread_start)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/nios2/clone.S b/sysdeps/unix/sysv/linux/nios2/clone.S
index 4da5c19..e4d3970 100644
--- a/sysdeps/unix/sysv/linux/nios2/clone.S
+++ b/sysdeps/unix/sysv/linux/nios2/clone.S
@@ -104,4 +104,5 @@ thread_start:
 
 	cfi_startproc
 PSEUDO_END (__clone)
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/nptl-signals.h
index 7560a21..01f34c2 100644
--- a/sysdeps/unix/sysv/linux/nptl-signals.h
+++ b/sysdeps/unix/sysv/linux/nptl-signals.h
@@ -16,6 +16,8 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <signal.h>
+
 /* The signal used for asynchronous cancelation.  */
 #define SIGCANCEL       __SIGRTMIN
 
@@ -29,5 +31,13 @@
 /* Signal used to implement the setuid et.al. functions.  */
 #define SIGSETXID       (__SIGRTMIN + 1)
 
+
+/* Return is sig is used internally.  */
+static inline int
+__nptl_is_internal_signal (int sig)
+{
+  return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID);
+}
+
 /* Used to communicate with signal handler.  */
 extern struct xid_command *__xidcmd attribute_hidden;
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S b/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S
index 28948ea..eb973db 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S
@@ -108,4 +108,5 @@ L(badargs):
 	cfi_startproc
 END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S b/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S
index c8c6de8..959fdb7 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S
@@ -126,4 +126,5 @@ L(parent):
 
 END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/clone.S b/sysdeps/unix/sysv/linux/s390/s390-32/clone.S
index cb2afbb..f774e90 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/clone.S
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/clone.S
@@ -70,4 +70,6 @@ thread_start:
 	xc	0(4,%r15),0(%r15)
 	basr    %r14,%r1        /* jump to fn */
 	DO_CALL (exit, 1)
+
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/clone.S b/sysdeps/unix/sysv/linux/s390/s390-64/clone.S
index eddab35..928a881 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/clone.S
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/clone.S
@@ -73,4 +73,6 @@ thread_start:
 	xc	0(8,%r15),0(%r15)
 	basr	%r14,%r1	/* jump to fn */
 	DO_CALL	(exit, 1)
+
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/sh/clone.S b/sysdeps/unix/sysv/linux/sh/clone.S
index 53cc08b..a73dd7d 100644
--- a/sysdeps/unix/sysv/linux/sh/clone.S
+++ b/sysdeps/unix/sysv/linux/sh/clone.S
@@ -123,4 +123,5 @@ ENTRY(__clone)
 	.word	TID - TLS_PRE_TCB_SIZE
 PSEUDO_END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S b/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S
index 6f41f0f..68f8202 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S
@@ -100,4 +100,5 @@ __thread_start:
 
 	.size	__thread_start, .-__thread_start
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S b/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S
index e2d3914..cecffa7 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S
+++ b/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S
@@ -96,4 +96,5 @@ __thread_start:
 
 	.size	__thread_start, .-__thread_start
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
new file mode 100644
index 0000000..16f602d
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -0,0 +1,426 @@
+/* POSIX spawn interface.  Linux version.
+   Copyright (C) 2016 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <spawn.h>
+#include <fcntl.h>
+#include <paths.h>
+#include <string.h>
+#include <sys/resource.h>
+#include <sys/wait.h>
+#include <sys/param.h>
+#include <sys/mman.h>
+#include <not-cancel.h>
+#include <local-setxid.h>
+#include <shlib-compat.h>
+#include <nptl/pthreadP.h>
+#include <dl-sysdep.h>
+#include <ldsodefs.h>
+#include "spawn_int.h"
+
+/* The Linux implementation of posix_spawn{p} uses the clone syscall directly
+   with CLONE_VM and CLONE_VFORK flags and an allocated stack.  The new stack
+   and start function solves most the vfork limitation (possible parent
+   clobber due stack spilling). The remaning issue are:
+
+   1. That no signal handlers must run in child context, to avoid corrupt
+      parent's state.
+   2. The parent must ensure child's stack freeing.
+   3. Child must synchronize with parent to enforce 2. and to possible
+      return execv issues.
+
+   The first issue is solved by blocking all signals in child, even the
+   NPTL-internal ones (SIGCANCEL and SIGSETXID).  The second and third issue
+   is done by a stack allocation in parent and a synchronization with using
+   a pipe or waitpid (in case or error).  The pipe has the advantage of
+   allowing the child the signal an exec error.  */
+
+
+/* The Unix standard contains a long explanation of the way to signal
+   an error after the fork() was successful.  Since no new wait status
+   was wanted there is no way to signal an error using one of the
+   available methods.  The committee chose to signal an error by a
+   normal program exit with the exit code 127.  */
+#define SPAWN_ERROR	127
+
+/* We need to block both SIGCANCEL and SIGSETXID.  */
+#define SIGALL_SET ((sigset_t *)(const unsigned long long [2]){ -1,-1 })
+
+#ifdef __ia64__
+# define CLONE(__fn, __stack, __flags, __args) \
+  __clone2 (__fn, __stack, __flags, __args, 0, 0, 0)
+#else
+# define CLONE(__fn, __stack, __flags, __args) \
+  __clone (__fn, __stack, __flags, __args)
+#endif
+
+#if _STACK_GROWS_DOWN
+# define STACK(__stack, __stack_size) (__stack + __stack_size)
+#elif _STACK_GROWS_UP
+# define STACK(__stack, __stack_size) (__stack)
+#endif
+
+/* Issues a 'pipe2' when the architecture supports or a 'pipe' plus O_CLOCEXEC
+   otherwise.  When pipe2 syscall is not available the pipe plus fcntl have a
+   inherent race condition (pipe2 is the right solution to avoid it).  */
+static int __do_pipe2_cloexec (int pipe_fds[2])
+{
+  /* Try pipe2 even if __ASSUME_PIPE2 is not define and returning an error
+     iff the call returns ENOSYS.  */
+  int r = -1;
+#ifdef O_CLOEXEC
+# ifndef __ASSUME_PIPE2
+  if (__have_pipe2 >= 0)
+# endif
+    { 
+      r = __pipe2 (pipe_fds, O_CLOEXEC);
+# ifndef __ASSUME_PIPE2
+      if (__have_pipe2 == 0)
+        __have_pipe2 = r != -1 || errno != ENOSYS ? 1 : -1;
+
+      if (__have_pipe2 > 0)
+# endif 
+        return r;
+    }
+#endif /* O_CLOEXEC  */
+#ifndef __ASSUME_PIPE2
+# ifdef O_CLOEXEC
+  if (__have_pipe2 < 0)
+# endif
+    if (__pipe (pipe_fds) < 0)
+      return -1;
+#endif /* __ASSUME_PIPE2  */
+
+  /* Then apply FD_CLOCEXEC if it is supported in the pipe call case.  */
+#ifndef __ASSUME_PIPE2
+# ifdef O_CLOEXEC
+  if (__have_pipe2 < 0)
+# endif
+    {
+      if ((r = __fcntl (pipe_fds[0], F_SETFD, FD_CLOEXEC)) == -1
+	  || (r = __fcntl (pipe_fds[1], F_SETFD, FD_CLOEXEC)) == -1)
+        {
+	  close_not_cancel (pipe_fds[0]);
+	  close_not_cancel (pipe_fds[1]);
+	  return r;
+	}
+    }
+#endif /* __ASSUME_PIPE2  */
+
+  return r;
+}
+
+struct posix_spawn_args {
+  int p[2];
+  sigset_t oldmask;
+  const char *path;
+  int (*exec)(const char *, char *const *, char *const *);
+  const posix_spawn_file_actions_t *fa;
+  const posix_spawnattr_t *restrict attr;
+  char *const *argv, *const *envp;
+  int xflags;
+};
+
+/* Older version requires that shell script without shebang definition
+   to be called explicity using /bin/sh (_PATH_BSHELL).  */
+static void
+maybe_script_execute (struct posix_spawn_args *args)
+{
+  if (SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_15)
+      && (args->xflags & SPAWN_XFLAGS_TRY_SHELL)
+      && errno == ENOEXEC)
+    {
+      /* Count the arguments.  */
+      int argc = 0;
+      char *const *argv = args->argv;
+      while (argv[argc++]);
+
+      /* Construct an argument list for the shell.  */
+      char *new_argv[argc];
+      new_argv[0] = (char *) _PATH_BSHELL;
+      new_argv[1] = (char *) args->path;
+      while (argc > 1)
+	{
+	  new_argv[argc] = argv[argc - 1];
+	  --argc;
+	}
+
+      /* Execute the shell.  */
+      args->exec (new_argv[0], new_argv, args->envp);
+    }
+}
+
+/* Function used in the clone call to setup the signals mask, posix_spawn
+   attributes, and file actions.  It run on its own stack (provided by the
+   posix_spawn call).  */
+static int
+__spawni_child (void *arguments)
+{
+  struct posix_spawn_args *args = arguments;
+  const posix_spawnattr_t *restrict attr = args->attr;
+  const posix_spawn_file_actions_t *file_actions = args->fa;
+  int p = args->p[1];
+  int ret;
+
+  close_not_cancel (args->p[0]);
+
+  /* The child must ensure that no signal handler are enabled because it shared
+     memory with parent, so the signal disposition must be either SIG_DFL or
+     SIG_IGN.  It does by iterating over all signals and although it could 
+     possibly be more optimized (by tracking which signal potentially have a
+     signal handler), it might requires system specific solutions (since the
+     sigset_t data type can be very different on different architectures).  */
+  struct sigaction sa;
+  memset (&sa, '\0', sizeof (sa));
+
+  sigset_t hset;
+  __sigprocmask (SIG_BLOCK, 0, &hset);
+  for (int sig = 1; sig < _NSIG; ++sig)
+    {
+      if ((attr->__flags & POSIX_SPAWN_SETSIGDEF)
+          && sigismember (&attr->__sd, sig))
+	{
+	  sa.sa_handler = SIG_DFL;
+	}
+      else if (sigismember (&hset, sig))
+	{
+	   if (__nptl_is_internal_signal (sig))
+	      sa.sa_handler = SIG_IGN;
+	   else
+	     {
+		__libc_sigaction (sig, 0, &sa);
+		if (sa.sa_handler == SIG_IGN)
+		  continue;
+		sa.sa_handler = SIG_DFL;
+	     }
+	}
+      else
+	continue;
+
+      __libc_sigaction (sig, &sa, 0);
+    }
+
+#ifdef _POSIX_PRIORITY_SCHEDULING
+  /* Set the scheduling algorithm and parameters.  */
+  if ((attr->__flags & (POSIX_SPAWN_SETSCHEDPARAM | POSIX_SPAWN_SETSCHEDULER))
+      == POSIX_SPAWN_SETSCHEDPARAM)
+    {
+      if ((ret = __sched_setparam (0, &attr->__sp)) == -1)
+	goto fail;
+    }
+  else if ((attr->__flags & POSIX_SPAWN_SETSCHEDULER) != 0)
+    {
+      if ((ret = __sched_setscheduler (0, attr->__policy, &attr->__sp)) == -1)
+        goto fail;
+    }
+#endif
+
+  /* Set the process group ID.  */
+  if ((attr->__flags & POSIX_SPAWN_SETPGROUP) != 0
+      && (ret = __setpgid (0, attr->__pgrp)) != 0)
+    goto fail;
+
+  /* Set the effective user and group IDs.  */
+  if ((attr->__flags & POSIX_SPAWN_RESETIDS) != 0
+      && ((ret = local_seteuid (__getuid ())) != 0
+	  || (ret = local_setegid (__getgid ())) != 0))
+    goto fail;
+
+  /* Execute the file actions.  */
+  if (file_actions != 0)
+    {
+      int cnt;
+      struct rlimit64 fdlimit;
+      bool have_fdlimit = false;
+
+      for (cnt = 0; cnt < file_actions->__used; ++cnt)
+	{
+	  struct __spawn_action *action = &file_actions->__actions[cnt];
+
+	  /* Dup the pipe fd onto an unoccupied one to avoid any file
+	     operation to clobber it.  */
+	  if ((action->action.close_action.fd == p)
+	      || (action->action.open_action.fd == p)
+	      || (action->action.dup2_action.fd == p))
+	    {
+	      if ((ret = __dup (p)) < 0)
+		goto fail;
+	      p = ret;
+	    }
+
+	  switch (action->tag)
+	    {
+	    case spawn_do_close:
+	      if ((ret = close_not_cancel (action->action.close_action.fd)) != 0)
+		{
+		  if (!have_fdlimit)
+		    {
+		      __getrlimit64 (RLIMIT_NOFILE, &fdlimit);
+		      have_fdlimit = true;
+		    }
+
+		  /* Only signal errors for file descriptors out of range.  */
+		  if (action->action.close_action.fd < 0
+		      || action->action.close_action.fd >= fdlimit.rlim_cur)
+		    goto fail;
+		}
+	      break;
+
+	    case spawn_do_open:
+	      {
+		ret = open_not_cancel (action->action.open_action.path,
+			    action->action.open_action.oflag | O_LARGEFILE,
+			    action->action.open_action.mode);
+
+		if (ret == -1)
+		  goto fail;
+
+		int new_fd = ret;
+
+		/* Make sure the desired file descriptor is used.  */
+		if (ret != action->action.open_action.fd)
+		  {
+		    if ((ret = __dup2 (new_fd, action->action.open_action.fd))
+			!= action->action.open_action.fd)
+		      goto fail;
+
+		    if ((ret = close_not_cancel (new_fd)) != 0)
+		      goto fail;
+		  }
+	      }
+	      break;
+
+	    case spawn_do_dup2:
+	      if ((ret = __dup2 (action->action.dup2_action.fd,
+			  action->action.dup2_action.newfd))
+		  != action->action.dup2_action.newfd)
+		goto fail;
+	      break;
+	    }
+	}
+    }
+
+  /* Set the initial signal mask of the child if POSIX_SPAWN_SETSIGMASK
+     is set, otherwise restore the previous one.  */
+  __sigprocmask (SIG_SETMASK, (attr->__flags & POSIX_SPAWN_SETSIGMASK)
+    ? &attr->__ss : &args->oldmask, 0);
+
+  args->exec (args->path, args->argv, args->envp);
+
+  /* This is compatibility function required to enable posix_spawn run
+     script without shebang definition for older posix_spawn versions
+     (2.15).  */
+  maybe_script_execute (args);
+
+  ret = -errno;
+
+fail:
+  /* Since sizeof errno < PIPE_BUF, the write is atomic. */
+  ret = -ret;
+  if (ret)
+    while (write_not_cancel (p, &ret, sizeof ret) < 0);
+  exit (SPAWN_ERROR);
+}
+
+/* Spawn a new process executing PATH with the attributes describes in *ATTRP.
+   Before running the process perform the actions described in FILE-ACTIONS. */
+static int
+__spawnix (pid_t *pid, const char *file,
+  const posix_spawn_file_actions_t *file_actions,
+  const posix_spawnattr_t *attrp, char *const argv[], char *const envp[],
+  int xflags, int (*exec)(const char *, char *const *, char *const *))
+{
+  pid_t new_pid;
+  struct posix_spawn_args args;
+  int ec;
+
+  if (__do_pipe2_cloexec (args.p))
+    return errno;
+
+  /* Allocates a child stack with sufficient size to handle a large argument
+     set in both the 'maybe_script_execute' compatibility code and on the
+     execvpe (posix_spawnp) where it might allocate stack size when path
+     does not contain a slash.  */
+  const int prot = (PROT_READ | PROT_WRITE
+                    | ((GL(dl_stack_flags) & PF_X) ? PROT_EXEC : 0));
+
+  const int stack_size = ARCH_STACK_DEFAULT_SIZE;
+  void *stack = __mmap (NULL, stack_size, prot,
+                        MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+  if (__glibc_unlikely (stack == MAP_FAILED))
+    {
+      close_not_cancel (args.p[0]);
+      close_not_cancel (args.p[1]);
+      return errno;
+    }
+
+  /* Disable asynchronous cancellation.  */
+  int cs = LIBC_CANCEL_ASYNC ();
+
+  args.path = file;
+  args.exec = exec;
+  args.fa = file_actions;
+  args.attr = attrp ? attrp : &(const posix_spawnattr_t){ 0 };
+  args.argv = argv;
+  args.envp = envp;
+  args.xflags = xflags;
+
+  __sigprocmask (SIG_BLOCK, SIGALL_SET, &args.oldmask);
+
+  new_pid = CLONE (__spawni_child, STACK (stack, stack_size),
+                   CLONE_VM | CLONE_VFORK | SIGCHLD, &args);
+
+  close_not_cancel (args.p[1]);
+
+  if (new_pid > 0)
+    {
+       if (__read (args.p[0], &ec, sizeof ec) != sizeof ec)
+          ec = 0;
+       else
+          __waitpid (new_pid, NULL, 0);
+    }
+  else
+    {
+       ec = -new_pid;
+    }
+
+  __munmap (stack, stack_size);
+
+  close_not_cancel (args.p[0]);
+
+  if (!ec && new_pid)
+    *pid = new_pid;
+
+  __sigprocmask (SIG_SETMASK, &args.oldmask, 0);
+
+  LIBC_CANCEL_RESET (cs);
+
+  return ec;
+}
+
+/* Spawn a new process executing PATH with the attributes describes in *ATTRP.
+   Before running the process perform the actions described in FILE-ACTIONS. */
+int
+__spawni (pid_t *pid, const char *file,
+          const posix_spawn_file_actions_t *acts,
+          const posix_spawnattr_t *attrp, char *const argv[],
+          char *const envp[], int xflags)
+{
+  if (xflags & SPAWN_XFLAGS_USE_PATH)
+    return __spawnix (pid, file, acts, attrp, argv, envp, xflags, __execvpe);
+  return __spawnix (pid, file, acts, attrp, argv, envp, xflags, __execve);
+}
diff --git a/sysdeps/unix/sysv/linux/tile/clone.S b/sysdeps/unix/sysv/linux/tile/clone.S
index 02fe3a8..a300eb5 100644
--- a/sysdeps/unix/sysv/linux/tile/clone.S
+++ b/sysdeps/unix/sysv/linux/tile/clone.S
@@ -219,4 +219,5 @@ ENTRY (__clone)
 	}
 PSEUDO_END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/x86_64/clone.S b/sysdeps/unix/sysv/linux/x86_64/clone.S
index 382568a..1a50957 100644
--- a/sysdeps/unix/sysv/linux/x86_64/clone.S
+++ b/sysdeps/unix/sysv/linux/x86_64/clone.S
@@ -115,4 +115,5 @@ L(thread_start):
 	cfi_startproc;
 PSEUDO_END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
-- 
1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] posix: execvpe cleanup
@ 2016-01-27 12:32 Adhemerval Zanella
  2016-01-27 12:32 ` [PATCH 2/2] posix: New Linux posix_spawn{p} implementation Adhemerval Zanella
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Adhemerval Zanella @ 2016-01-27 12:32 UTC (permalink / raw)
  To: libc-alpha

This patch removes all the dynamic allocation on execvpe code and
instead use direct stack allocation.  This is QoI approach to make
it possible use in scenarios where memory is shared with parent
(vfork or clone with CLONE_VM).

For default process spawn (script file without a shebang), stack
allocation is bounded by NAME_MAX plus PATH_MAX plus 1.  Large
file arguments returns an error (ENAMETOOLONG).  This differs than
current GLIBC pratice in general, but it used to limit stack
allocation for large inputs.  Also, path in PATH environment variable
larger than PATH_MAX are ignored.

The shell direct execution exeception, where execve returns ENOEXEC,
might requires a large stack allocation due large input argument list.

Tested on i686, x86_64, powerpc64le, and aarch64.

Changes from previous version:

- Limit argument count in maybe_script_execute to avoid unbound stack
  allocation.  Current limit is NCARGS (131072 for Linux).
- Added testcases for execvpe based both on existing execvp and two
  new to test general e failed execution.

	* posix/execvpe.c (__execvpe): Remove dynamic allocation.
	* posix/Makefile (tests): Add tst-execvpe{1,2,3,4,5,6}.
	* posix/tst-execvp1.c (do_test): Use a macro to call execvp.
	* posix/tst-execvp2.c (do_test): Likewise.
	* posix/tst-execvp3.c (do_test): Likewise.
	* posix/tst-execvp4.c (do_test): Likewise.
	* posix/tst-execvpe1.c: New file.
	* posix/tst-execvpe2.c: Likewise.
	* posix/tst-execvpe3.c: Likewise.
	* posix/tst-execvpe4.c: Likewise.
	* posix/tst-execvpe5.c: Likewise.
	* posix/tst-execvpe6.c: Likewise.
---
 posix/Makefile       |   3 +
 posix/execvpe.c      | 241 ++++++++++++++++++++-------------------------------
 posix/tst-execvp1.c  |   6 +-
 posix/tst-execvp2.c  |   5 +-
 posix/tst-execvp3.c  |   5 +-
 posix/tst-execvp4.c  |   6 +-
 posix/tst-execvpe1.c |  20 +++++
 posix/tst-execvpe2.c |  20 +++++
 posix/tst-execvpe3.c |  20 +++++
 posix/tst-execvpe4.c |  20 +++++
 posix/tst-execvpe5.c | 130 +++++++++++++++++++++++++++
 posix/tst-execvpe6.c |  82 ++++++++++++++++++
 13 files changed, 424 insertions(+), 149 deletions(-)
 create mode 100644 posix/tst-execvpe1.c
 create mode 100644 posix/tst-execvpe2.c
 create mode 100644 posix/tst-execvpe3.c
 create mode 100644 posix/tst-execvpe4.c
 create mode 100644 posix/tst-execvpe5.c
 create mode 100644 posix/tst-execvpe6.c

diff --git a/posix/Makefile b/posix/Makefile
index f94e023..e251363 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -82,6 +82,8 @@ tests		:= tstgetopt testfnm runtests runptests	     \
 		   tst-execv1 tst-execv2 tst-execl1 tst-execl2 \
 		   tst-execve1 tst-execve2 tst-execle1 tst-execle2 \
 		   tst-execvp3 tst-execvp4 tst-rfc3484 tst-rfc3484-2 \
+		   tst-execvpe1 tst-execvpe2 tst-execvpe3 tst-execvpe4 \
+		   tst-execvpe5 tst-execvpe6 \
 		   tst-rfc3484-3 \
 		   tst-getaddrinfo3 tst-fnmatch2 tst-cpucount tst-cpuset \
 		   bug-getopt1 bug-getopt2 bug-getopt3 bug-getopt4 \
@@ -228,6 +230,7 @@ tstgetopt-ARGS = -a -b -cfoobar --required foobar --optional=bazbug \
 
 tst-exec-ARGS = -- $(host-test-program-cmd)
 tst-exec-static-ARGS = $(tst-exec-ARGS)
+tst-execvpe5-ARGS = -- $(host-test-program-cmd)
 tst-spawn-ARGS = -- $(host-test-program-cmd)
 tst-spawn-static-ARGS = $(tst-spawn-ARGS)
 tst-dir-ARGS = `pwd` `cd $(common-objdir)/$(subdir); pwd` `cd $(common-objdir); pwd` $(objpfx)tst-dir
diff --git a/posix/execvpe.c b/posix/execvpe.c
index 61697a7..6fe507d 100644
--- a/posix/execvpe.c
+++ b/posix/execvpe.c
@@ -15,7 +15,6 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <alloca.h>
 #include <unistd.h>
 #include <stdarg.h>
 #include <stdbool.h>
@@ -23,22 +22,37 @@
 #include <string.h>
 #include <errno.h>
 #include <paths.h>
-
+#include <confstr.h>
+#include <sys/param.h>
 
 /* The file is accessible but it is not an executable file.  Invoke
    the shell to interpret it as a script.  */
 static void
-internal_function
-scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
+maybe_script_execute (const char *path, char *const argv[], char *const envp[])
 {
+  /* Linux accepts a very large argument number (INT_MAX to fit on a signed
+     32-bit integer).  To limit stack allocation we set it to a lower
+     bound.  */
+  int argc = 0;
+  while (argv[argc++])
+    if (argc > NCARGS)
+      {
+	errno = E2BIG;
+	return;
+      }
+
   /* Construct an argument list for the shell.  */
+  char *new_argv[argc];
   new_argv[0] = (char *) _PATH_BSHELL;
-  new_argv[1] = (char *) file;
+  new_argv[1] = (char *) path;
   while (argc > 1)
     {
       new_argv[argc] = argv[argc - 1];
       --argc;
     }
+
+  /* Execute the shell.  */
+  __execve (new_argv[0], new_argv, envp);
 }
 
 
@@ -47,170 +61,107 @@ scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
 int
 __execvpe (const char *file, char *const argv[], char *const envp[])
 {
+  /* We check the simple case first. */
   if (*file == '\0')
     {
-      /* We check the simple case first. */
       __set_errno (ENOENT);
       return -1;
     }
 
-  if (strchr (file, '/') != NULL)
+  /* Don't search when it contains a slash.  */
+  if (strchr (file, '/'))
     {
-      /* Don't search when it contains a slash.  */
       __execve (file, argv, envp);
 
       if (errno == ENOEXEC)
-	{
-	  /* Count the arguments.  */
-	  int argc = 0;
-	  while (argv[argc++])
-	    ;
-	  size_t len = (argc + 1) * sizeof (char *);
-	  char **script_argv;
-	  void *ptr = NULL;
-	  if (__libc_use_alloca (len))
-	    script_argv = alloca (len);
-	  else
-	    script_argv = ptr = malloc (len);
-
-	  if (script_argv != NULL)
-	    {
-	      scripts_argv (file, argv, argc, script_argv);
-	      __execve (script_argv[0], script_argv, envp);
-
-	      free (ptr);
-	    }
-	}
+        maybe_script_execute (file, argv, envp);
+
+      return -1;
     }
-  else
+
+  const char *path = getenv ("PATH");
+  if (!path)
+    path = CS_PATH;
+  /* Although GLIBC does not enforce NAME_MAX, we set it as the maximum
+     size to avoid unbounded stack allocation.  Same applies for
+     PATH_MAX.  */
+  size_t file_len = __strnlen (file, NAME_MAX + 1);
+  if (file_len > NAME_MAX)
     {
-      size_t pathlen;
-      size_t alloclen = 0;
-      char *path = getenv ("PATH");
-      if (path == NULL)
-	{
-	  pathlen = confstr (_CS_PATH, (char *) NULL, 0);
-	  alloclen = pathlen + 1;
-	}
-      else
-	pathlen = strlen (path);
+      errno = ENAMETOOLONG;
+      return -1;
+    }
+  size_t path_len = __strnlen (path, PATH_MAX - 1) + 1;
 
-      size_t len = strlen (file) + 1;
-      alloclen += pathlen + len + 1;
+  const char *subp;
+  bool got_eacces = false;
+  for (const char *p = path; ; p = subp)
+    {
+      char buffer[path_len + file_len + 1];
 
-      char *name;
-      char *path_malloc = NULL;
-      if (__libc_use_alloca (alloclen))
-	name = alloca (alloclen);
-      else
-	{
-	  path_malloc = name = malloc (alloclen);
-	  if (name == NULL)
-	    return -1;
-	}
+      subp = __strchrnul (p, ':');
 
-      if (path == NULL)
+      /* PATH is larger than PATH_MAX and thus potentially larger than
+	 the stack allocation.  */
+      if (subp - p >= path_len)
 	{
-	  /* There is no `PATH' in the environment.
-	     The default search path is the current directory
-	     followed by the path `confstr' returns for `_CS_PATH'.  */
-	  path = name + pathlen + len + 1;
-	  path[0] = ':';
-	  (void) confstr (_CS_PATH, path + 1, pathlen);
+          /* If there is only one path, bail out.  */
+	  if (!*subp)
+	    break;
+	  /* Otherwise skip to next one.  */
+	  continue;
 	}
 
-      /* Copy the file name at the top.  */
-      name = (char *) memcpy (name + pathlen + 1, file, len);
-      /* And add the slash.  */
-      *--name = '/';
+      /* Set current path considered plus a '/'.  */
+      memcpy (buffer, p, subp - p);
+      buffer[subp - p] = '/';
+      /* And the file to execute.  */
+      memcpy (buffer + (subp - p) + (subp > p), file, file_len + 1);
+
+      __execve (buffer, argv, envp);
+
+      if (errno == ENOEXEC)
+        maybe_script_execute (buffer, argv, envp);
 
-      char **script_argv = NULL;
-      void *script_argv_malloc = NULL;
-      bool got_eacces = false;
-      char *p = path;
-      do
+      switch (errno)
 	{
-	  char *startp;
-
-	  path = p;
-	  p = __strchrnul (path, ':');
-
-	  if (p == path)
-	    /* Two adjacent colons, or a colon at the beginning or the end
-	       of `PATH' means to search the current directory.  */
-	    startp = name + 1;
-	  else
-	    startp = (char *) memcpy (name - (p - path), path, p - path);
-
-	  /* Try to execute this name.  If it works, execve will not return. */
-	  __execve (startp, argv, envp);
-
-	  if (errno == ENOEXEC)
-	    {
-	      if (script_argv == NULL)
-		{
-		  /* Count the arguments.  */
-		  int argc = 0;
-		  while (argv[argc++])
-		    ;
-		  size_t arglen = (argc + 1) * sizeof (char *);
-		  if (__libc_use_alloca (alloclen + arglen))
-		    script_argv = alloca (arglen);
-		  else
-		    script_argv = script_argv_malloc = malloc (arglen);
-		  if (script_argv == NULL)
-		    {
-		      /* A possible EACCES error is not as important as
-			 the ENOMEM.  */
-		      got_eacces = false;
-		      break;
-		    }
-		  scripts_argv (startp, argv, argc, script_argv);
-		}
-
-	      __execve (script_argv[0], script_argv, envp);
-	    }
-
-	  switch (errno)
-	    {
-	    case EACCES:
-	      /* Record the we got a `Permission denied' error.  If we end
-		 up finding no executable we can use, we want to diagnose
-		 that we did find one but were denied access.  */
-	      got_eacces = true;
-	    case ENOENT:
-	    case ESTALE:
-	    case ENOTDIR:
-	      /* Those errors indicate the file is missing or not executable
-		 by us, in which case we want to just try the next path
-		 directory.  */
-	    case ENODEV:
-	    case ETIMEDOUT:
-	      /* Some strange filesystems like AFS return even
-		 stranger error numbers.  They cannot reasonably mean
-		 anything else so ignore those, too.  */
-	      break;
-
-	    default:
-	      /* Some other error means we found an executable file, but
-		 something went wrong executing it; return the error to our
-		 caller.  */
-	      return -1;
-	    }
+	  case EACCES:
+	  /* Record the we got a 'Permission denied' error.  If we end
+             up finding no executable we can use, we want to diagnose
+             that we did find one but were denied access.  */
+	    got_eacces = true;
+	  case ENOENT:
+	  case ESTALE:
+	  case ENOTDIR:
+	  /* Those errors indicate the file is missing or not executable
+	     by us, in which case we want to just try the next path
+	     directory.  */
+	  case ENODEV:
+	  case ETIMEDOUT:
+          /* Some strange filesystems like AFS return even
+             stranger error numbers.  They cannot reasonably mean
+             anything else so ignore those, too.  */
+	    break;
+
+          default:
+	  /* Some other error means we found an executable file, but
+	     something went wrong executing it; return the error to our
+	     caller.  */
+	    return -1;
 	}
-      while (*p++ != '\0');
-
-      /* We tried every element and none of them worked.  */
-      if (got_eacces)
-	/* At least one failure was due to permissions, so report that
-	   error.  */
-	__set_errno (EACCES);
 
-      free (script_argv_malloc);
-      free (path_malloc);
+      if (*subp++ == '\0')
+	break;
     }
 
-  /* Return the error from the last attempt (probably ENOENT).  */
+  /* We tried every element and none of them worked.  */
+  if (got_eacces)
+    /* At least one failure was due to permissions, so report that
+       error.  */
+    __set_errno (EACCES);
+
   return -1;
+
 }
+
 weak_alias (__execvpe, execvpe)
diff --git a/posix/tst-execvp1.c b/posix/tst-execvp1.c
index ecc673d..fe98ce5 100644
--- a/posix/tst-execvp1.c
+++ b/posix/tst-execvp1.c
@@ -3,6 +3,10 @@
 #include <stdlib.h>
 #include <unistd.h>
 
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
+
 static int
 do_test (void)
 {
@@ -19,7 +23,7 @@ do_test (void)
 
   char *argv[] = { (char *) "does-not-exist", NULL };
   errno = 0;
-  execvp (argv[0], argv);
+  EXECVP (argv[0], argv);
 
   if (errno != ENOENT)
     {
diff --git a/posix/tst-execvp2.c b/posix/tst-execvp2.c
index 7e0f5d8..9be8733 100644
--- a/posix/tst-execvp2.c
+++ b/posix/tst-execvp2.c
@@ -14,6 +14,9 @@ static int do_test (void);
 #define TEST_FUNCTION do_test ()
 #include "../test-skeleton.c"
 
+#ifndef EXECVP
+# define EXECVP(__file, __argv)  execvp (__file, __argv)
+#endif
 
 static char *copy;
 
@@ -70,7 +73,7 @@ do_test (void)
 
   char *argv[] = { basename (copy), NULL };
   errno = 0;
-  execvp (argv[0], argv);
+  EXECVP (argv[0], argv);
 
   if (errno != EACCES)
     {
diff --git a/posix/tst-execvp3.c b/posix/tst-execvp3.c
index 5ebc879..43f7c34 100644
--- a/posix/tst-execvp3.c
+++ b/posix/tst-execvp3.c
@@ -12,6 +12,9 @@ static int do_test (void);
 
 #include "../test-skeleton.c"
 
+#ifndef EXECVP
+# define EXECVP(__file, __argv)  execvp (__file, __argv)
+#endif
 
 static char *fname;
 
@@ -35,7 +38,7 @@ do_test (void)
     }
 
   char *argv[] = { fname, NULL };
-  execvp (basename (fname), argv);
+  EXECVP (basename (fname), argv);
 
   /* If we come here, the execvp call failed.  */
   return 1;
diff --git a/posix/tst-execvp4.c b/posix/tst-execvp4.c
index 531fab2..116624f 100644
--- a/posix/tst-execvp4.c
+++ b/posix/tst-execvp4.c
@@ -5,6 +5,10 @@
 #include <unistd.h>
 #include <sys/stat.h>
 
+#ifndef EXECVP
+# define EXECVP(__file, __argv)  execvp (__file, __argv)
+#endif
+
 static int
 do_test (void)
 {
@@ -27,7 +31,7 @@ do_test (void)
 
   unsetenv ("PATH");
   char *argv[] = { buf + 9, NULL };
-  execvp (argv[0], argv);
+  EXECVP (argv[0], argv);
   return 0;
 }
 
diff --git a/posix/tst-execvpe1.c b/posix/tst-execvpe1.c
new file mode 100644
index 0000000..7b386c8
--- /dev/null
+++ b/posix/tst-execvpe1.c
@@ -0,0 +1,20 @@
+/* Check ENOENT failure for execvpe.
+   Copyright (C) 2016 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
+   <http://www.gnu.org/licenses/>.  */
+
+#define EXECVP(__file, __argv) execvpe (__file, __argv, NULL)
+#include <posix/tst-execvp1.c>
diff --git a/posix/tst-execvpe2.c b/posix/tst-execvpe2.c
new file mode 100644
index 0000000..3c99fb1
--- /dev/null
+++ b/posix/tst-execvpe2.c
@@ -0,0 +1,20 @@
+/* Check EACCES for execvpe.
+   Copyright (C) 2016 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
+   <http://www.gnu.org/licenses/>.  */
+
+#define EXECVP(__file, __argv) execvpe (__file, __argv, NULL)
+#include <posix/tst-execvp2.c>
diff --git a/posix/tst-execvpe3.c b/posix/tst-execvpe3.c
new file mode 100644
index 0000000..8380fd3
--- /dev/null
+++ b/posix/tst-execvpe3.c
@@ -0,0 +1,20 @@
+/* Check script execution without shebang for execvpe.
+   Copyright (C) 2016 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
+   <http://www.gnu.org/licenses/>.  */
+
+#define EXECVP(__file, __argv) execvpe (__file, __argv, NULL)
+#include <posix/tst-execvp3.c>
diff --git a/posix/tst-execvpe4.c b/posix/tst-execvpe4.c
new file mode 100644
index 0000000..08fdaf0
--- /dev/null
+++ b/posix/tst-execvpe4.c
@@ -0,0 +1,20 @@
+/* Check unexistent binary for execvpe.
+   Copyright (C) 2016 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
+   <http://www.gnu.org/licenses/>.  */
+
+#define EXECVP(__file, __argv) execvpe (__file, __argv, NULL)
+#include <posix/tst-execvp4.c>
diff --git a/posix/tst-execvpe5.c b/posix/tst-execvpe5.c
new file mode 100644
index 0000000..09f0fb0
--- /dev/null
+++ b/posix/tst-execvpe5.c
@@ -0,0 +1,130 @@
+/* General tests for execpve.
+   Copyright (C) 2016 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <error.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <wait.h>
+
+
+/* Nonzero if the program gets called via `exec'.  */
+static int restart;
+
+
+#define CMDLINE_OPTIONS \
+  { "restart", no_argument, &restart, 1 },
+
+/* Prototype for our test function.  */
+extern void do_prepare (int argc, char *argv[]);
+extern int do_test (int argc, char *argv[]);
+
+#include "../test-skeleton.c"
+
+#define EXECVPE_KEY    "EXECVPE_ENV"
+#define EXECVPE_VALUE  "execvpe_test"
+
+
+static int
+handle_restart (void)
+{
+  /* First check if only one variable is passed on execvpe.  */
+  int env_count = 0;
+  for (char **e = environ; *e != NULL; ++e)
+    if (++env_count == INT_MAX)
+      error (EXIT_FAILURE, 0, "environment variable number overflow");
+  if (env_count != 1)
+    error (EXIT_FAILURE, 0, "wrong number of environment variables");
+
+  /* Check if the combinarion os "EXECVPE_ENV=execvpe_test"  */
+  const char *env = getenv (EXECVPE_KEY);
+  if (env == NULL)
+    error (EXIT_FAILURE, 0, "test environment variable not found");
+
+  if (strncmp (env, EXECVPE_VALUE, sizeof (EXECVPE_VALUE)))
+    error (EXIT_FAILURE, 0, "test environment variable with wrong value");
+
+  return 0;
+}
+
+
+int
+do_test (int argc, char *argv[])
+{
+  pid_t pid;
+  int status;
+
+  /* We must have
+     - one or four parameters left if called initially
+       + path for ld.so		optional
+       + "--library-path"	optional
+       + the library path	optional
+       + the application name
+  */
+
+  if (restart)
+    {
+      if (argc != 1)
+	error (EXIT_FAILURE, 0, "wrong number of arguments (%d)", argc);
+
+      return handle_restart ();
+    }
+
+  if (argc != 2 && argc != 5)
+    error (EXIT_FAILURE, 0, "wrong number of arguments (%d)", argc);
+
+  /* We want to test the `execvpe' function.  To do this we restart the
+     program with an additional parameter.  */
+  pid = fork ();
+  if (pid == 0)
+    {
+      /* This is the child.  Construct the command line.  */
+
+      /* We cast here to char* because the test itself does not modify neither
+	 the argument nor the environment list.  */
+      char *envs[] = { (char*)(EXECVPE_KEY "=" EXECVPE_VALUE), NULL };
+      if (argc == 5)
+	{
+	  char *args[] = { argv[1], argv[2], argv[3], argv[4],
+			   (char*)"--direct", (char*)"--restart", NULL };
+	  execvpe (args[0], args, envs);
+	}
+      else
+	{
+	  char *args[] = { argv[1], argv[1],
+			   (char*)"--direct", (char*)"--restart", NULL };
+	  execvpe (args[0], args, envs);
+	}
+
+      error (EXIT_FAILURE, errno, "cannot exec");
+    }
+  else if (pid == (pid_t) -1)
+    error (EXIT_FAILURE, errno, "cannot fork");
+
+  /* Wait for the child.  */
+  if (waitpid (pid, &status, 0) != pid)
+    error (EXIT_FAILURE, errno, "wrong child");
+
+  if (WTERMSIG (status) != 0)
+    error (EXIT_FAILURE, 0, "Child terminated incorrectly");
+  status = WEXITSTATUS (status);
+
+  return status;
+}
diff --git a/posix/tst-execvpe6.c b/posix/tst-execvpe6.c
new file mode 100644
index 0000000..e099a2f
--- /dev/null
+++ b/posix/tst-execvpe6.c
@@ -0,0 +1,82 @@
+/* Check execvpe script argument handling.
+   Copyright (C) 2016 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/param.h>
+
+static char *fname;
+
+static void do_prepare (void);
+#define PREPARE(argc, argv) do_prepare ()
+static int do_test (void);
+#define TEST_FUNCTION do_test ()
+
+#include "../test-skeleton.c"
+
+static void
+do_prepare (void)
+{
+  int fd = create_temp_file ("testscript", &fname);
+  dprintf (fd, "echo foo\n");
+  fchmod (fd, 0700);
+  close (fd);
+}
+
+static int
+do_test (void)
+{
+  if  (setenv ("PATH", test_dir, 1) != 0)
+    {
+      puts ("setenv failed");
+      return 1;
+    }
+
+  /* To limit stack allocation for argument construction in case of
+     script without shebang execvpe limits total number of argument
+     to NCARGS.  */
+  const size_t max_args = NCARGS + 1;
+  char **args = malloc ((NCARGS + 1) * sizeof (char*));
+  if (args == NULL)
+    {
+      puts ("malloc failed");
+      return 1;
+    }
+
+  args[0] = fname;
+  for (int i=1; i < max_args; ++i)
+    args[i] = strdup ("a");
+  args[max_args] = NULL;
+
+  execvpe (basename (fname), args, NULL);
+
+  for (int i=1; i < max_args; ++i)
+    free (args[i]);
+  free (args);
+
+  if (errno != E2BIG)
+    {
+      printf ("errno = %d (%m), expected E2BIG\n", errno);
+      return 1;
+    }
+
+  return 0;
+}
-- 
1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] posix: execvpe cleanup
  2016-01-27 12:32 [PATCH v2 1/2] posix: execvpe cleanup Adhemerval Zanella
  2016-01-27 12:32 ` [PATCH 2/2] posix: New Linux posix_spawn{p} implementation Adhemerval Zanella
@ 2016-01-27 13:34 ` Andreas Schwab
  2016-01-27 16:17 ` Paul Eggert
  2 siblings, 0 replies; 10+ messages in thread
From: Andreas Schwab @ 2016-01-27 13:34 UTC (permalink / raw)
  To: Adhemerval Zanella; +Cc: libc-alpha

Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:

> -  if (strchr (file, '/') != NULL)
> +  /* Don't search when it contains a slash.  */
> +  if (strchr (file, '/'))

No implicit boolean conversion.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] posix: New Linux posix_spawn{p} implementation
  2016-01-27 12:32 ` [PATCH 2/2] posix: New Linux posix_spawn{p} implementation Adhemerval Zanella
@ 2016-01-27 16:17   ` Paul Eggert
  2016-01-28 17:22     ` Mike Frysinger
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Eggert @ 2016-01-27 16:17 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha

Adhemerval Zanella wrote:
> +#define SIGALL_SET ((sigset_t *)(const unsigned long long [2]){ -1,-1 })

This shouldn't assume that _SIGSET_NWORDS is 2. And since the code is 
Linux-specific, you should be able to avoid casting to sigset_t *; just write a 
constant that evaluates to sigset_t * without a cast.

> +#ifdef O_CLOEXEC
> +# ifndef __ASSUME_PIPE2
> +  if (__have_pipe2 >= 0)
> +# endif
> +    {
> +      r = __pipe2 (pipe_fds, O_CLOEXEC);
> +# ifndef __ASSUME_PIPE2
> +      if (__have_pipe2 == 0)
...

This sort of code is hard to read. Instead, declare substitutes like this after 
you do your #includes:

   #ifndef __ASSUME_PIPE2
   # define __have_pipe2 1
   #endif
   #ifndef O_CLOEXEC
   # define O_CLOEXEC 0
   #endif

and let the rest of the code just use __have_pipe2 and O_CLOEXEC, without the 
forest of #ifdefs.

> +struct posix_spawn_args {
> +  int p[2];
> +  sigset_t oldmask;
> +  const char *path;

This is a file name, right? Not a path? If so, it should not be called "path".

> +  int (*exec)(const char *, char *const *, char *const *);

Spaces between )(. Also, please check the indenting overall; it seemed a bit 
inconsistent in this file.

> +      while (argv[argc++]);
...
> +    while (write_not_cancel (p, &ret, sizeof ret) < 0);

Use "continue".

> +      /* Construct an argument list for the shell.  */
> +      char *new_argv[argc];

This can overflow the stack.

> +		  /* Only signal errors for file descriptors out of range.  */

"Signal errors only for ..."




^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] posix: execvpe cleanup
  2016-01-27 12:32 [PATCH v2 1/2] posix: execvpe cleanup Adhemerval Zanella
  2016-01-27 12:32 ` [PATCH 2/2] posix: New Linux posix_spawn{p} implementation Adhemerval Zanella
  2016-01-27 13:34 ` [PATCH v2 1/2] posix: execvpe cleanup Andreas Schwab
@ 2016-01-27 16:17 ` Paul Eggert
  2016-01-27 20:15   ` Adhemerval Zanella
  2 siblings, 1 reply; 10+ messages in thread
From: Paul Eggert @ 2016-01-27 16:17 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha

Adhemerval Zanella wrote:
> +  int argc = 0;
> +  while (argv[argc++])
> +    if (argc > NCARGS)

This won't work on platforms where NCARGS is INT_MAX.

> +  char *new_argv[argc];

This can allocate up to NCARGS * sizeof (char *) bytes on the stack, which is 
too much. I suggest allocating on the stack only if __libc_use_alloca says it's OK.

> +  size_t path_len = __strnlen (path, PATH_MAX - 1) + 1;

What about platforms that don't define PATH_MAX because there's no limit? Or 
what if PATH_MAX is larger than what __libc_use_alloca would allow?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] posix: execvpe cleanup
  2016-01-27 16:17 ` Paul Eggert
@ 2016-01-27 20:15   ` Adhemerval Zanella
  2016-01-27 22:16     ` Paul Eggert
  0 siblings, 1 reply; 10+ messages in thread
From: Adhemerval Zanella @ 2016-01-27 20:15 UTC (permalink / raw)
  To: Paul Eggert, libc-alpha



On 27-01-2016 14:17, Paul Eggert wrote:
> Adhemerval Zanella wrote:
>> +  int argc = 0;
>> +  while (argv[argc++])
>> +    if (argc > NCARGS)
> 
> This won't work on platforms where NCARGS is INT_MAX.
> 
>> +  char *new_argv[argc];
> 
> This can allocate up to NCARGS * sizeof (char *) bytes on the stack, which is too much. I suggest allocating on the stack only if __libc_use_alloca says it's OK.
> 

Right, I can change to:

--
  int argc = 0;
  do
    if ((argc+1) == NCARGS)
      { 
        errno = E2BIG;
        return;
      } 
  while (argv[argc++] != NULL);

  /* Linux accepts a very large argument number (INT_MAX to fit on a signed
     32-bit integer).  To limit stack allocation we set it to a lower
     bound.  */
  if (!__libc_alloca_cutoff (argc * sizeof (char*)))
    {
      errno = E2BIG;
      return;
    }
--

This will take care of argc overflow and stack allocation.  The only problem is
for such code path (execvpe for a shell script without shebang) total arguments
will be limited to __MAX_ALLOCA_CUTOFF / sizeof (char*), which for current
glibc settings would be 16384 for 32 bits and 8192.  I can live with it, specially
since it a functionality which IMHO we should not have provided in first place
(it is provided only on compat mode for posix_spawn{p}).


>> +  size_t path_len = __strnlen (path, PATH_MAX - 1) + 1;
> 
> What about platforms that don't define PATH_MAX because there's no limit? Or what if PATH_MAX is larger than what __libc_use_alloca would allow?

I noted glibc code usually set PATH_MAX to 1024 if it is not defined, so following
the code system/posix/getcwd.c should suffice:

#ifndef PATH_MAX
# ifdef MAXPATHLEN
#  define PATH_MAX MAXPATHLEN
# else
#  define PATH_MAX 1024
# endif
#endif

About '__libc_use_alloca' I can add a test like that:

  size_t file_len = __strnlen (file, NAME_MAX + 1);
  size_t path_len = __strnlen (path, PATH_MAX - 1) + 1;

  if ((file_len > NAME_MAX)
      || !__libc_alloca_cutoff (path_len + file_len + 1))
    {
      errno = ENAMETOOLONG;
      return -1;
    }

We current __MAX_ALLOCA_CUTOFF of 65536 I think we have plenty of room for various
directory limits configurations.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] posix: execvpe cleanup
  2016-01-27 20:15   ` Adhemerval Zanella
@ 2016-01-27 22:16     ` Paul Eggert
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Eggert @ 2016-01-27 22:16 UTC (permalink / raw)
  To: Adhemerval Zanella, libc-alpha

On 01/27/2016 12:15 PM, Adhemerval Zanella wrote:
>    int argc = 0;
>    do
>      if ((argc+1) == NCARGS)
>        {
>          errno = E2BIG;
>          return;
>        }
>    while (argv[argc++] != NULL);

Change that to something like the following (this is pseudocode):

     int argc = 0;
     int limit = min (NCARGS, allocacutoff / sizeof (char *));
     while (argv[argc++] != NULL)
       if (limit <= argc)
         {
            errno = E2BIG;
            return;
         }

That way, you won't need the following snippet:

>
>    /* Linux accepts a very large argument number (INT_MAX to fit on a signed
>       32-bit integer).  To limit stack allocation we set it to a lower
>       bound.  */
>    if (!__libc_alloca_cutoff (argc * sizeof (char*)))
>      {
>        errno = E2BIG;
>        return;
>      }

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] posix: New Linux posix_spawn{p} implementation
  2016-01-27 16:17   ` Paul Eggert
@ 2016-01-28 17:22     ` Mike Frysinger
  2016-01-28 17:45       ` Adhemerval Zanella
  0 siblings, 1 reply; 10+ messages in thread
From: Mike Frysinger @ 2016-01-28 17:22 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Adhemerval Zanella, libc-alpha

[-- Attachment #1: Type: text/plain, Size: 964 bytes --]

On 27 Jan 2016 08:17, Paul Eggert wrote:
> Adhemerval Zanella wrote:
> > +#ifdef O_CLOEXEC
> > +# ifndef __ASSUME_PIPE2
> > +  if (__have_pipe2 >= 0)
> > +# endif
> > +    {
> > +      r = __pipe2 (pipe_fds, O_CLOEXEC);
> > +# ifndef __ASSUME_PIPE2
> > +      if (__have_pipe2 == 0)
> ...
> 
> This sort of code is hard to read. Instead, declare substitutes like this after 
> you do your #includes:
> 
>    #ifndef __ASSUME_PIPE2
>    # define __have_pipe2 1
>    #endif
>    #ifndef O_CLOEXEC
>    # define O_CLOEXEC 0
>    #endif
> 
> and let the rest of the code just use __have_pipe2 and O_CLOEXEC, without the 
> forest of #ifdefs.

where does O_CLOEXEC not exist ?  can't we clean that up ?

looking at __ASSUME_O_CLOEXEC, that too is set to 1 everywhere
(linux/nacl/hurd).  rather than add more code using these, can't
we delete this old code ?  or at the very least, do not try to
use those symbols in this new code.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] posix: New Linux posix_spawn{p} implementation
  2016-01-28 17:22     ` Mike Frysinger
@ 2016-01-28 17:45       ` Adhemerval Zanella
  0 siblings, 0 replies; 10+ messages in thread
From: Adhemerval Zanella @ 2016-01-28 17:45 UTC (permalink / raw)
  To: Paul Eggert, libc-alpha

On 28-01-2016 15:22, Mike Frysinger wrote:
> On 27 Jan 2016 08:17, Paul Eggert wrote:
>> Adhemerval Zanella wrote:
>>> +#ifdef O_CLOEXEC
>>> +# ifndef __ASSUME_PIPE2
>>> +  if (__have_pipe2 >= 0)
>>> +# endif
>>> +    {
>>> +      r = __pipe2 (pipe_fds, O_CLOEXEC);
>>> +# ifndef __ASSUME_PIPE2
>>> +      if (__have_pipe2 == 0)
>> ...
>>
>> This sort of code is hard to read. Instead, declare substitutes like this after 
>> you do your #includes:
>>
>>    #ifndef __ASSUME_PIPE2
>>    # define __have_pipe2 1
>>    #endif
>>    #ifndef O_CLOEXEC
>>    # define O_CLOEXEC 0
>>    #endif
>>
>> and let the rest of the code just use __have_pipe2 and O_CLOEXEC, without the 
>> forest of #ifdefs.
> 
> where does O_CLOEXEC not exist ?  can't we clean that up ?
> 
> looking at __ASSUME_O_CLOEXEC, that too is set to 1 everywhere
> (linux/nacl/hurd).  rather than add more code using these, can't
> we delete this old code ?  or at the very least, do not try to
> use those symbols in this new code.
> -mike
> 

According to 74385da564a92fc441a7d7f3ed7a4295b74bfdbf we can now assume
O_CLOEXEC as always existent for Linux. I will change that.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 2/2] posix: New Linux posix_spawn{p} implementation
  2016-01-25 14:24 [PATCH " Adhemerval Zanella
@ 2016-01-25 14:24 ` Adhemerval Zanella
  0 siblings, 0 replies; 10+ messages in thread
From: Adhemerval Zanella @ 2016-01-25 14:24 UTC (permalink / raw)
  To: libc-alpha

This patch implements a new posix_spawn{p} implementation for Linux.  The main
difference is it uses the clone syscall directly with CLONE_VM and CLONE_VFORK
flags and a direct allocated stack.  The new stack and start function solves
most the vfork limitation (possible parent clobber due stack spilling).  The
remaning issue are related to signal handling:

  1. That no signal handlers must run in child context, to avoid corrupt
     parent's state.
  2. Child must synchronize with parent to enforce stack deallocation and
     to possible return execv issues.

The first one is solved by blocking all signals in child, even NPTL-internal
ones (SIGCANCEL and SIGSETXID).  The second issue is done by a stack allocation
in parent and a synchronization with using a pipe or waitpid (in case or error).
The pipe has the advantage of allowing the child signal an exec error (checked
with new tst-spawn2 test).

There is an inherent race condition in pipe2 usage for architectures that do not
support the syscall directly.  In such cases the a pipe plus fctnl is used
instead and it may lead to file descriptor leak in parent (as decribed by fcntl
documentation).

The child process stack is allocate with a mmap with MAP_STACK flag using
default architecture stack size.  Although it is slower than use a stack buffer
from parent, it allows some slack for the compatibility code to run scripts
with no shebang (which may use a buffer with size depending of argument list
count).

Performance should be similar to the vfork default posix implementation and
way faster than fork path (vfork on mostly linux ports are basically
clone with CLONE_VM plus CLONE_VFORK).  The only difference is the syscalls
required for the stack allocation/deallocation.

It fixes BZ#10354, BZ#14750, and BZ#18433.

Tested on i386, x86_64, powerpc64le, and aarch64.

	[BZ #14750]
	[BZ #10354]
	[BZ #18433]
	* include/sched.h (__clone): Add hidden prototype.
	(__clone2): Likewise.
	* include/unistd.h (__dup): Likewise.
	* posix/Makefile (tests): Add tst-spawn2.
	* posix/tst-spawn2.c: New file.
	* sysdeps/posix/dup.c (__dup): Add hidden definition.
	* sysdeps/unix/sysv/linux/aarch64/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/alpha/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/arm/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/hppa/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/i386/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/ia64/clone2.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/m68k/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/microblaze/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/mips/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/nios2/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S (__clone):
	Likewise.
	* sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S (__clone):
	Likewise.
	* sysdeps/unix/sysv/linux/s390/s390-32/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/s390/s390-64/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/sh/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/sparc/sparc32/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/sparc/sparc64/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/tile/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/x86_64/clone.S (__clone): Likewise.
	* sysdeps/unix/sysv/linux/nptl-signals.h
	(____nptl_is_internal_signal): New function.
	* sysdeps/unix/sysv/linux/spawni.c: New file.
---
 ChangeLog                                         |  34 ++
 include/sched.h                                   |   2 +
 include/unistd.h                                  |   1 +
 posix/Makefile                                    |   2 +-
 posix/tst-spawn2.c                                |  73 ++++
 sysdeps/posix/dup.c                               |   2 +-
 sysdeps/unix/sysv/linux/aarch64/clone.S           |   1 +
 sysdeps/unix/sysv/linux/alpha/clone.S             |   1 +
 sysdeps/unix/sysv/linux/arm/clone.S               |   1 +
 sysdeps/unix/sysv/linux/hppa/clone.S              |   1 +
 sysdeps/unix/sysv/linux/i386/clone.S              |   1 +
 sysdeps/unix/sysv/linux/ia64/clone2.S             |   2 +
 sysdeps/unix/sysv/linux/m68k/clone.S              |   1 +
 sysdeps/unix/sysv/linux/microblaze/clone.S        |   1 +
 sysdeps/unix/sysv/linux/mips/clone.S              |   1 +
 sysdeps/unix/sysv/linux/nios2/clone.S             |   1 +
 sysdeps/unix/sysv/linux/nptl-signals.h            |  10 +
 sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S |   1 +
 sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S |   1 +
 sysdeps/unix/sysv/linux/s390/s390-32/clone.S      |   2 +
 sysdeps/unix/sysv/linux/s390/s390-64/clone.S      |   2 +
 sysdeps/unix/sysv/linux/sh/clone.S                |   1 +
 sysdeps/unix/sysv/linux/sparc/sparc32/clone.S     |   1 +
 sysdeps/unix/sysv/linux/sparc/sparc64/clone.S     |   1 +
 sysdeps/unix/sysv/linux/spawni.c                  | 426 ++++++++++++++++++++++
 sysdeps/unix/sysv/linux/tile/clone.S              |   1 +
 sysdeps/unix/sysv/linux/x86_64/clone.S            |   1 +
 27 files changed, 570 insertions(+), 2 deletions(-)
 create mode 100644 posix/tst-spawn2.c
 create mode 100644 sysdeps/unix/sysv/linux/spawni.c

diff --git a/include/sched.h b/include/sched.h
index 4f59397..b4d7406 100644
--- a/include/sched.h
+++ b/include/sched.h
@@ -19,7 +19,9 @@ extern int __sched_rr_get_interval (__pid_t __pid, struct timespec *__t);
 /* These are Linux specific.  */
 extern int __clone (int (*__fn) (void *__arg), void *__child_stack,
 		    int __flags, void *__arg, ...);
+libc_hidden_proto (__clone)
 extern int __clone2 (int (*__fn) (void *__arg), void *__child_stack_base,
 		     size_t __child_stack_size, int __flags, void *__arg, ...);
+libc_hidden_proto (__clone2)
 #endif
 #endif
diff --git a/include/unistd.h b/include/unistd.h
index 5152f64..d2802b2 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -81,6 +81,7 @@ char *__canonicalize_directory_name_internal (const char *__thisdir,
 					      size_t __size) attribute_hidden;
 
 extern int __dup (int __fd);
+libc_hidden_proto (__dup)
 extern int __dup2 (int __fd, int __fd2);
 libc_hidden_proto (__dup2)
 extern int __dup3 (int __fd, int __fd2, int flags);
diff --git a/posix/Makefile b/posix/Makefile
index f94e023..9ad5f2e 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -91,7 +91,7 @@ tests		:= tstgetopt testfnm runtests runptests	     \
 xtests		:= bug-ga2
 ifeq (yes,$(build-shared))
 test-srcs	:= globtest
-tests           += wordexp-test tst-exec tst-spawn
+tests           += wordexp-test tst-exec tst-spawn tst-spawn2
 endif
 tests-static	= tst-exec-static tst-spawn-static
 tests		+= $(tests-static)
diff --git a/posix/tst-spawn2.c b/posix/tst-spawn2.c
new file mode 100644
index 0000000..41bfde8
--- /dev/null
+++ b/posix/tst-spawn2.c
@@ -0,0 +1,73 @@
+/* Further tests for spawn in case of invalid binary paths.
+   Copyright (C) 2016 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <spawn.h>
+#include <error.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/wait.h>
+
+#include <stdio.h>
+
+int
+posix_spawn_test (void)
+{
+  /* Check if posix_spawn correctly returns an error and an invalid pid
+     by trying to spawn an invalid binary.  */
+
+  const char *program = "/path/to/invalid/binary";
+  char * const args[] = { 0 };
+  pid_t pid = -1;
+
+  int ret = posix_spawn (&pid, program, 0, 0, args, environ);
+  if (ret != ENOENT)
+    error (EXIT_FAILURE, errno, "posix_spawn");
+
+  /* POSIX states the value returned on pid variable in case of an error
+     is not specified.  GLIBC will update the value iff the child 
+     execution is successful.  */
+  if (pid != -1)
+    error (EXIT_FAILURE, errno, "posix_spawn returned pid != -1");
+
+  /* Check if no child is actually created.  */
+  ret = waitpid (-1, NULL, 0);
+  if (ret != -1 || errno != ECHILD)
+    error (EXIT_FAILURE, errno, "waitpid");
+
+  /* Same as before, but with posix_spawnp.  */
+  char *args2[] = { (char*) program, 0 };
+
+  ret = posix_spawnp (&pid, args2[0], 0, 0, args2, environ);
+  if (ret != ENOENT)
+    error (EXIT_FAILURE, errno, "posix_spawnp");
+  
+  if (pid != -1)
+    error (EXIT_FAILURE, errno, "posix_spawnp returned pid != -1");
+
+  ret = waitpid (-1, NULL, 0);
+  if (ret != -1 || errno != ECHILD)
+    error (EXIT_FAILURE, errno, "waitpid");
+
+  return 0;
+}
+
+#define TEST_FUNCTION  posix_spawn_test ()
+#include "../test-skeleton.c"
+
diff --git a/sysdeps/posix/dup.c b/sysdeps/posix/dup.c
index 9525e76..abf3ff5 100644
--- a/sysdeps/posix/dup.c
+++ b/sysdeps/posix/dup.c
@@ -26,5 +26,5 @@ __dup (int fd)
 {
   return fcntl (fd, F_DUPFD, 0);
 }
-
+libc_hidden_def (__dup)
 weak_alias (__dup, dup)
diff --git a/sysdeps/unix/sysv/linux/aarch64/clone.S b/sysdeps/unix/sysv/linux/aarch64/clone.S
index 596fb9c..8f3ab40 100644
--- a/sysdeps/unix/sysv/linux/aarch64/clone.S
+++ b/sysdeps/unix/sysv/linux/aarch64/clone.S
@@ -93,4 +93,5 @@ thread_start:
 	cfi_endproc
 	.size thread_start, .-thread_start
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/alpha/clone.S b/sysdeps/unix/sysv/linux/alpha/clone.S
index ec9c06d..556aa63 100644
--- a/sysdeps/unix/sysv/linux/alpha/clone.S
+++ b/sysdeps/unix/sysv/linux/alpha/clone.S
@@ -137,4 +137,5 @@ thread_start:
 	cfi_endproc
 	.end thread_start
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/arm/clone.S b/sysdeps/unix/sysv/linux/arm/clone.S
index 460a8f5..c103123 100644
--- a/sysdeps/unix/sysv/linux/arm/clone.S
+++ b/sysdeps/unix/sysv/linux/arm/clone.S
@@ -93,4 +93,5 @@ PSEUDO_END (__clone)
 
 	.fnend
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/hppa/clone.S b/sysdeps/unix/sysv/linux/hppa/clone.S
index e80fd8d..0a18137 100644
--- a/sysdeps/unix/sysv/linux/hppa/clone.S
+++ b/sysdeps/unix/sysv/linux/hppa/clone.S
@@ -175,4 +175,5 @@ ENTRY(__clone)
 
 PSEUDO_END(__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/i386/clone.S b/sysdeps/unix/sysv/linux/i386/clone.S
index 7d818c1..ef447d1 100644
--- a/sysdeps/unix/sysv/linux/i386/clone.S
+++ b/sysdeps/unix/sysv/linux/i386/clone.S
@@ -139,4 +139,5 @@ L(nomoregetpid):
 	cfi_startproc
 PSEUDO_END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/ia64/clone2.S b/sysdeps/unix/sysv/linux/ia64/clone2.S
index 9b257b3..fc046eb 100644
--- a/sysdeps/unix/sysv/linux/ia64/clone2.S
+++ b/sysdeps/unix/sysv/linux/ia64/clone2.S
@@ -96,6 +96,8 @@ ENTRY(__clone2)
 	ret			/* Not reached.		*/
 PSEUDO_END(__clone2)
 
+libc_hidden_def (__clone2)
+
 /* For now we leave __clone undefined.  This is unlikely to be a	*/
 /* problem, since at least the i386 __clone in glibc always failed	*/
 /* with a 0 sp (eventhough the kernel explicitly handled it).		*/
diff --git a/sysdeps/unix/sysv/linux/m68k/clone.S b/sysdeps/unix/sysv/linux/m68k/clone.S
index 8b40df2..33474cc 100644
--- a/sysdeps/unix/sysv/linux/m68k/clone.S
+++ b/sysdeps/unix/sysv/linux/m68k/clone.S
@@ -127,4 +127,5 @@ donepid:
 	cfi_startproc
 PSEUDO_END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/microblaze/clone.S b/sysdeps/unix/sysv/linux/microblaze/clone.S
index 035d88b..cbc95ce 100644
--- a/sysdeps/unix/sysv/linux/microblaze/clone.S
+++ b/sysdeps/unix/sysv/linux/microblaze/clone.S
@@ -69,4 +69,5 @@ L(thread_start):
 	nop
 PSEUDO_END(__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone,clone)
diff --git a/sysdeps/unix/sysv/linux/mips/clone.S b/sysdeps/unix/sysv/linux/mips/clone.S
index 755e8cc..feb8250 100644
--- a/sysdeps/unix/sysv/linux/mips/clone.S
+++ b/sysdeps/unix/sysv/linux/mips/clone.S
@@ -166,4 +166,5 @@ L(gotpid):
 
 	END(__thread_start)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/nios2/clone.S b/sysdeps/unix/sysv/linux/nios2/clone.S
index 4da5c19..e4d3970 100644
--- a/sysdeps/unix/sysv/linux/nios2/clone.S
+++ b/sysdeps/unix/sysv/linux/nios2/clone.S
@@ -104,4 +104,5 @@ thread_start:
 
 	cfi_startproc
 PSEUDO_END (__clone)
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/nptl-signals.h
index 7560a21..01f34c2 100644
--- a/sysdeps/unix/sysv/linux/nptl-signals.h
+++ b/sysdeps/unix/sysv/linux/nptl-signals.h
@@ -16,6 +16,8 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
+#include <signal.h>
+
 /* The signal used for asynchronous cancelation.  */
 #define SIGCANCEL       __SIGRTMIN
 
@@ -29,5 +31,13 @@
 /* Signal used to implement the setuid et.al. functions.  */
 #define SIGSETXID       (__SIGRTMIN + 1)
 
+
+/* Return is sig is used internally.  */
+static inline int
+__nptl_is_internal_signal (int sig)
+{
+  return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID);
+}
+
 /* Used to communicate with signal handler.  */
 extern struct xid_command *__xidcmd attribute_hidden;
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S b/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S
index 28948ea..eb973db 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S
@@ -108,4 +108,5 @@ L(badargs):
 	cfi_startproc
 END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S b/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S
index c8c6de8..959fdb7 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S
@@ -126,4 +126,5 @@ L(parent):
 
 END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/clone.S b/sysdeps/unix/sysv/linux/s390/s390-32/clone.S
index cb2afbb..f774e90 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/clone.S
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/clone.S
@@ -70,4 +70,6 @@ thread_start:
 	xc	0(4,%r15),0(%r15)
 	basr    %r14,%r1        /* jump to fn */
 	DO_CALL (exit, 1)
+
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/clone.S b/sysdeps/unix/sysv/linux/s390/s390-64/clone.S
index eddab35..928a881 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/clone.S
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/clone.S
@@ -73,4 +73,6 @@ thread_start:
 	xc	0(8,%r15),0(%r15)
 	basr	%r14,%r1	/* jump to fn */
 	DO_CALL	(exit, 1)
+
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/sh/clone.S b/sysdeps/unix/sysv/linux/sh/clone.S
index 53cc08b..a73dd7d 100644
--- a/sysdeps/unix/sysv/linux/sh/clone.S
+++ b/sysdeps/unix/sysv/linux/sh/clone.S
@@ -123,4 +123,5 @@ ENTRY(__clone)
 	.word	TID - TLS_PRE_TCB_SIZE
 PSEUDO_END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S b/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S
index 6f41f0f..68f8202 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S
@@ -100,4 +100,5 @@ __thread_start:
 
 	.size	__thread_start, .-__thread_start
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S b/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S
index e2d3914..cecffa7 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S
+++ b/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S
@@ -96,4 +96,5 @@ __thread_start:
 
 	.size	__thread_start, .-__thread_start
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
new file mode 100644
index 0000000..16f602d
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -0,0 +1,426 @@
+/* POSIX spawn interface.  Linux version.
+   Copyright (C) 2016 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <spawn.h>
+#include <fcntl.h>
+#include <paths.h>
+#include <string.h>
+#include <sys/resource.h>
+#include <sys/wait.h>
+#include <sys/param.h>
+#include <sys/mman.h>
+#include <not-cancel.h>
+#include <local-setxid.h>
+#include <shlib-compat.h>
+#include <nptl/pthreadP.h>
+#include <dl-sysdep.h>
+#include <ldsodefs.h>
+#include "spawn_int.h"
+
+/* The Linux implementation of posix_spawn{p} uses the clone syscall directly
+   with CLONE_VM and CLONE_VFORK flags and an allocated stack.  The new stack
+   and start function solves most the vfork limitation (possible parent
+   clobber due stack spilling). The remaning issue are:
+
+   1. That no signal handlers must run in child context, to avoid corrupt
+      parent's state.
+   2. The parent must ensure child's stack freeing.
+   3. Child must synchronize with parent to enforce 2. and to possible
+      return execv issues.
+
+   The first issue is solved by blocking all signals in child, even the
+   NPTL-internal ones (SIGCANCEL and SIGSETXID).  The second and third issue
+   is done by a stack allocation in parent and a synchronization with using
+   a pipe or waitpid (in case or error).  The pipe has the advantage of
+   allowing the child the signal an exec error.  */
+
+
+/* The Unix standard contains a long explanation of the way to signal
+   an error after the fork() was successful.  Since no new wait status
+   was wanted there is no way to signal an error using one of the
+   available methods.  The committee chose to signal an error by a
+   normal program exit with the exit code 127.  */
+#define SPAWN_ERROR	127
+
+/* We need to block both SIGCANCEL and SIGSETXID.  */
+#define SIGALL_SET ((sigset_t *)(const unsigned long long [2]){ -1,-1 })
+
+#ifdef __ia64__
+# define CLONE(__fn, __stack, __flags, __args) \
+  __clone2 (__fn, __stack, __flags, __args, 0, 0, 0)
+#else
+# define CLONE(__fn, __stack, __flags, __args) \
+  __clone (__fn, __stack, __flags, __args)
+#endif
+
+#if _STACK_GROWS_DOWN
+# define STACK(__stack, __stack_size) (__stack + __stack_size)
+#elif _STACK_GROWS_UP
+# define STACK(__stack, __stack_size) (__stack)
+#endif
+
+/* Issues a 'pipe2' when the architecture supports or a 'pipe' plus O_CLOCEXEC
+   otherwise.  When pipe2 syscall is not available the pipe plus fcntl have a
+   inherent race condition (pipe2 is the right solution to avoid it).  */
+static int __do_pipe2_cloexec (int pipe_fds[2])
+{
+  /* Try pipe2 even if __ASSUME_PIPE2 is not define and returning an error
+     iff the call returns ENOSYS.  */
+  int r = -1;
+#ifdef O_CLOEXEC
+# ifndef __ASSUME_PIPE2
+  if (__have_pipe2 >= 0)
+# endif
+    { 
+      r = __pipe2 (pipe_fds, O_CLOEXEC);
+# ifndef __ASSUME_PIPE2
+      if (__have_pipe2 == 0)
+        __have_pipe2 = r != -1 || errno != ENOSYS ? 1 : -1;
+
+      if (__have_pipe2 > 0)
+# endif 
+        return r;
+    }
+#endif /* O_CLOEXEC  */
+#ifndef __ASSUME_PIPE2
+# ifdef O_CLOEXEC
+  if (__have_pipe2 < 0)
+# endif
+    if (__pipe (pipe_fds) < 0)
+      return -1;
+#endif /* __ASSUME_PIPE2  */
+
+  /* Then apply FD_CLOCEXEC if it is supported in the pipe call case.  */
+#ifndef __ASSUME_PIPE2
+# ifdef O_CLOEXEC
+  if (__have_pipe2 < 0)
+# endif
+    {
+      if ((r = __fcntl (pipe_fds[0], F_SETFD, FD_CLOEXEC)) == -1
+	  || (r = __fcntl (pipe_fds[1], F_SETFD, FD_CLOEXEC)) == -1)
+        {
+	  close_not_cancel (pipe_fds[0]);
+	  close_not_cancel (pipe_fds[1]);
+	  return r;
+	}
+    }
+#endif /* __ASSUME_PIPE2  */
+
+  return r;
+}
+
+struct posix_spawn_args {
+  int p[2];
+  sigset_t oldmask;
+  const char *path;
+  int (*exec)(const char *, char *const *, char *const *);
+  const posix_spawn_file_actions_t *fa;
+  const posix_spawnattr_t *restrict attr;
+  char *const *argv, *const *envp;
+  int xflags;
+};
+
+/* Older version requires that shell script without shebang definition
+   to be called explicity using /bin/sh (_PATH_BSHELL).  */
+static void
+maybe_script_execute (struct posix_spawn_args *args)
+{
+  if (SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_15)
+      && (args->xflags & SPAWN_XFLAGS_TRY_SHELL)
+      && errno == ENOEXEC)
+    {
+      /* Count the arguments.  */
+      int argc = 0;
+      char *const *argv = args->argv;
+      while (argv[argc++]);
+
+      /* Construct an argument list for the shell.  */
+      char *new_argv[argc];
+      new_argv[0] = (char *) _PATH_BSHELL;
+      new_argv[1] = (char *) args->path;
+      while (argc > 1)
+	{
+	  new_argv[argc] = argv[argc - 1];
+	  --argc;
+	}
+
+      /* Execute the shell.  */
+      args->exec (new_argv[0], new_argv, args->envp);
+    }
+}
+
+/* Function used in the clone call to setup the signals mask, posix_spawn
+   attributes, and file actions.  It run on its own stack (provided by the
+   posix_spawn call).  */
+static int
+__spawni_child (void *arguments)
+{
+  struct posix_spawn_args *args = arguments;
+  const posix_spawnattr_t *restrict attr = args->attr;
+  const posix_spawn_file_actions_t *file_actions = args->fa;
+  int p = args->p[1];
+  int ret;
+
+  close_not_cancel (args->p[0]);
+
+  /* The child must ensure that no signal handler are enabled because it shared
+     memory with parent, so the signal disposition must be either SIG_DFL or
+     SIG_IGN.  It does by iterating over all signals and although it could 
+     possibly be more optimized (by tracking which signal potentially have a
+     signal handler), it might requires system specific solutions (since the
+     sigset_t data type can be very different on different architectures).  */
+  struct sigaction sa;
+  memset (&sa, '\0', sizeof (sa));
+
+  sigset_t hset;
+  __sigprocmask (SIG_BLOCK, 0, &hset);
+  for (int sig = 1; sig < _NSIG; ++sig)
+    {
+      if ((attr->__flags & POSIX_SPAWN_SETSIGDEF)
+          && sigismember (&attr->__sd, sig))
+	{
+	  sa.sa_handler = SIG_DFL;
+	}
+      else if (sigismember (&hset, sig))
+	{
+	   if (__nptl_is_internal_signal (sig))
+	      sa.sa_handler = SIG_IGN;
+	   else
+	     {
+		__libc_sigaction (sig, 0, &sa);
+		if (sa.sa_handler == SIG_IGN)
+		  continue;
+		sa.sa_handler = SIG_DFL;
+	     }
+	}
+      else
+	continue;
+
+      __libc_sigaction (sig, &sa, 0);
+    }
+
+#ifdef _POSIX_PRIORITY_SCHEDULING
+  /* Set the scheduling algorithm and parameters.  */
+  if ((attr->__flags & (POSIX_SPAWN_SETSCHEDPARAM | POSIX_SPAWN_SETSCHEDULER))
+      == POSIX_SPAWN_SETSCHEDPARAM)
+    {
+      if ((ret = __sched_setparam (0, &attr->__sp)) == -1)
+	goto fail;
+    }
+  else if ((attr->__flags & POSIX_SPAWN_SETSCHEDULER) != 0)
+    {
+      if ((ret = __sched_setscheduler (0, attr->__policy, &attr->__sp)) == -1)
+        goto fail;
+    }
+#endif
+
+  /* Set the process group ID.  */
+  if ((attr->__flags & POSIX_SPAWN_SETPGROUP) != 0
+      && (ret = __setpgid (0, attr->__pgrp)) != 0)
+    goto fail;
+
+  /* Set the effective user and group IDs.  */
+  if ((attr->__flags & POSIX_SPAWN_RESETIDS) != 0
+      && ((ret = local_seteuid (__getuid ())) != 0
+	  || (ret = local_setegid (__getgid ())) != 0))
+    goto fail;
+
+  /* Execute the file actions.  */
+  if (file_actions != 0)
+    {
+      int cnt;
+      struct rlimit64 fdlimit;
+      bool have_fdlimit = false;
+
+      for (cnt = 0; cnt < file_actions->__used; ++cnt)
+	{
+	  struct __spawn_action *action = &file_actions->__actions[cnt];
+
+	  /* Dup the pipe fd onto an unoccupied one to avoid any file
+	     operation to clobber it.  */
+	  if ((action->action.close_action.fd == p)
+	      || (action->action.open_action.fd == p)
+	      || (action->action.dup2_action.fd == p))
+	    {
+	      if ((ret = __dup (p)) < 0)
+		goto fail;
+	      p = ret;
+	    }
+
+	  switch (action->tag)
+	    {
+	    case spawn_do_close:
+	      if ((ret = close_not_cancel (action->action.close_action.fd)) != 0)
+		{
+		  if (!have_fdlimit)
+		    {
+		      __getrlimit64 (RLIMIT_NOFILE, &fdlimit);
+		      have_fdlimit = true;
+		    }
+
+		  /* Only signal errors for file descriptors out of range.  */
+		  if (action->action.close_action.fd < 0
+		      || action->action.close_action.fd >= fdlimit.rlim_cur)
+		    goto fail;
+		}
+	      break;
+
+	    case spawn_do_open:
+	      {
+		ret = open_not_cancel (action->action.open_action.path,
+			    action->action.open_action.oflag | O_LARGEFILE,
+			    action->action.open_action.mode);
+
+		if (ret == -1)
+		  goto fail;
+
+		int new_fd = ret;
+
+		/* Make sure the desired file descriptor is used.  */
+		if (ret != action->action.open_action.fd)
+		  {
+		    if ((ret = __dup2 (new_fd, action->action.open_action.fd))
+			!= action->action.open_action.fd)
+		      goto fail;
+
+		    if ((ret = close_not_cancel (new_fd)) != 0)
+		      goto fail;
+		  }
+	      }
+	      break;
+
+	    case spawn_do_dup2:
+	      if ((ret = __dup2 (action->action.dup2_action.fd,
+			  action->action.dup2_action.newfd))
+		  != action->action.dup2_action.newfd)
+		goto fail;
+	      break;
+	    }
+	}
+    }
+
+  /* Set the initial signal mask of the child if POSIX_SPAWN_SETSIGMASK
+     is set, otherwise restore the previous one.  */
+  __sigprocmask (SIG_SETMASK, (attr->__flags & POSIX_SPAWN_SETSIGMASK)
+    ? &attr->__ss : &args->oldmask, 0);
+
+  args->exec (args->path, args->argv, args->envp);
+
+  /* This is compatibility function required to enable posix_spawn run
+     script without shebang definition for older posix_spawn versions
+     (2.15).  */
+  maybe_script_execute (args);
+
+  ret = -errno;
+
+fail:
+  /* Since sizeof errno < PIPE_BUF, the write is atomic. */
+  ret = -ret;
+  if (ret)
+    while (write_not_cancel (p, &ret, sizeof ret) < 0);
+  exit (SPAWN_ERROR);
+}
+
+/* Spawn a new process executing PATH with the attributes describes in *ATTRP.
+   Before running the process perform the actions described in FILE-ACTIONS. */
+static int
+__spawnix (pid_t *pid, const char *file,
+  const posix_spawn_file_actions_t *file_actions,
+  const posix_spawnattr_t *attrp, char *const argv[], char *const envp[],
+  int xflags, int (*exec)(const char *, char *const *, char *const *))
+{
+  pid_t new_pid;
+  struct posix_spawn_args args;
+  int ec;
+
+  if (__do_pipe2_cloexec (args.p))
+    return errno;
+
+  /* Allocates a child stack with sufficient size to handle a large argument
+     set in both the 'maybe_script_execute' compatibility code and on the
+     execvpe (posix_spawnp) where it might allocate stack size when path
+     does not contain a slash.  */
+  const int prot = (PROT_READ | PROT_WRITE
+                    | ((GL(dl_stack_flags) & PF_X) ? PROT_EXEC : 0));
+
+  const int stack_size = ARCH_STACK_DEFAULT_SIZE;
+  void *stack = __mmap (NULL, stack_size, prot,
+                        MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+  if (__glibc_unlikely (stack == MAP_FAILED))
+    {
+      close_not_cancel (args.p[0]);
+      close_not_cancel (args.p[1]);
+      return errno;
+    }
+
+  /* Disable asynchronous cancellation.  */
+  int cs = LIBC_CANCEL_ASYNC ();
+
+  args.path = file;
+  args.exec = exec;
+  args.fa = file_actions;
+  args.attr = attrp ? attrp : &(const posix_spawnattr_t){ 0 };
+  args.argv = argv;
+  args.envp = envp;
+  args.xflags = xflags;
+
+  __sigprocmask (SIG_BLOCK, SIGALL_SET, &args.oldmask);
+
+  new_pid = CLONE (__spawni_child, STACK (stack, stack_size),
+                   CLONE_VM | CLONE_VFORK | SIGCHLD, &args);
+
+  close_not_cancel (args.p[1]);
+
+  if (new_pid > 0)
+    {
+       if (__read (args.p[0], &ec, sizeof ec) != sizeof ec)
+          ec = 0;
+       else
+          __waitpid (new_pid, NULL, 0);
+    }
+  else
+    {
+       ec = -new_pid;
+    }
+
+  __munmap (stack, stack_size);
+
+  close_not_cancel (args.p[0]);
+
+  if (!ec && new_pid)
+    *pid = new_pid;
+
+  __sigprocmask (SIG_SETMASK, &args.oldmask, 0);
+
+  LIBC_CANCEL_RESET (cs);
+
+  return ec;
+}
+
+/* Spawn a new process executing PATH with the attributes describes in *ATTRP.
+   Before running the process perform the actions described in FILE-ACTIONS. */
+int
+__spawni (pid_t *pid, const char *file,
+          const posix_spawn_file_actions_t *acts,
+          const posix_spawnattr_t *attrp, char *const argv[],
+          char *const envp[], int xflags)
+{
+  if (xflags & SPAWN_XFLAGS_USE_PATH)
+    return __spawnix (pid, file, acts, attrp, argv, envp, xflags, __execvpe);
+  return __spawnix (pid, file, acts, attrp, argv, envp, xflags, __execve);
+}
diff --git a/sysdeps/unix/sysv/linux/tile/clone.S b/sysdeps/unix/sysv/linux/tile/clone.S
index 02fe3a8..a300eb5 100644
--- a/sysdeps/unix/sysv/linux/tile/clone.S
+++ b/sysdeps/unix/sysv/linux/tile/clone.S
@@ -219,4 +219,5 @@ ENTRY (__clone)
 	}
 PSEUDO_END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/x86_64/clone.S b/sysdeps/unix/sysv/linux/x86_64/clone.S
index 382568a..1a50957 100644
--- a/sysdeps/unix/sysv/linux/x86_64/clone.S
+++ b/sysdeps/unix/sysv/linux/x86_64/clone.S
@@ -115,4 +115,5 @@ L(thread_start):
 	cfi_startproc;
 PSEUDO_END (__clone)
 
+libc_hidden_def (__clone)
 weak_alias (__clone, clone)
-- 
1.9.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-01-28 17:45 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-27 12:32 [PATCH v2 1/2] posix: execvpe cleanup Adhemerval Zanella
2016-01-27 12:32 ` [PATCH 2/2] posix: New Linux posix_spawn{p} implementation Adhemerval Zanella
2016-01-27 16:17   ` Paul Eggert
2016-01-28 17:22     ` Mike Frysinger
2016-01-28 17:45       ` Adhemerval Zanella
2016-01-27 13:34 ` [PATCH v2 1/2] posix: execvpe cleanup Andreas Schwab
2016-01-27 16:17 ` Paul Eggert
2016-01-27 20:15   ` Adhemerval Zanella
2016-01-27 22:16     ` Paul Eggert
  -- strict thread matches above, loose matches on Subject: below --
2016-01-25 14:24 [PATCH " Adhemerval Zanella
2016-01-25 14:24 ` [PATCH 2/2] posix: New Linux posix_spawn{p} implementation Adhemerval Zanella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).