From patchwork Mon May 15 19:36:24 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 99833 Delivered-To: patch@linaro.org Received: by 10.140.96.100 with SMTP id j91csp1697715qge; Mon, 15 May 2017 12:37:45 -0700 (PDT) X-Received: by 10.84.148.203 with SMTP id y11mr10849530plg.10.1494877065176; Mon, 15 May 2017 12:37:45 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1494877065; cv=none; d=google.com; s=arc-20160816; b=jtQe8dM0K+WFSUpOWJYvzuvP53+VIuwImfUO7urpiS49h+VPOV6mTjq6prhpIt/3XB o9+y7SwL4swsIG/aUumJVVHdrPsAGJtlvZNflnnhnERSMvPBYpu+h1GmBPgKy8UR7EpS lHP/qWWzQeudy68xPfkNinm6cXFW0EzX7i2O56N7J4nQCf8EixkDBYWVWrYj1oAQb3uI gzUNZ2+8hggYx0myR96vXqFrVGKjWL9LF2rOmrku+jkSHiAy08l0YzJYuNoImlTKq0mP R5zg9mXUL1R+QxVxzH4a6rzep1QCKujVauChCLZgYOTvYr9ND0Xkd6ZAGeP6MGWyRLOL B3Wg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=references:in-reply-to:message-id:date:subject:to:from:delivered-to :sender:list-help:list-post:list-archive:list-subscribe :list-unsubscribe:list-id:precedence:mailing-list:dkim-signature :domainkey-signature:arc-authentication-results; bh=nXbOIZuKVfKSJ3nEKe7D04IKBv1bSh6Q5HKEPg4vPtE=; b=Skv2yaozH0ANXaZwM7r2thPexXtw8GQbZQC+9ldEEUWYuFPOTS++OoS1R8Vs8rNtlt BH8A+51AB0UDZf5eW1yzdzybhk01t9oLtaPfn1sUygCc6y5HEulUPLCaRIlJn7iXML9Y 9jJ+ORDkOwvdFENpcc0T2CLqaZKg+8UcWK4u0sP0BqlO6mrhcIGGWLqHdxXc2ygq9zgi XIdHGTSrq3YL5XClv0RrIdP4WY0YcbPDsDKQsBn9UxmDcEHBOsLcptI+ZvCXcOCyEqpg iHTQ3prRgq7P1GTMAYVwNCMKGyrY5WsqVmgIT8OkedE53Y0HmuaEiMb/A5PpBZfdfURk TE+w== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org; spf=pass (google.com: domain of libc-alpha-return-79428-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-79428-patch=linaro.org@sourceware.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id 188si11379666pgb.399.2017.05.15.12.37.44 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 15 May 2017 12:37:45 -0700 (PDT) Received-SPF: pass (google.com: domain of libc-alpha-return-79428-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; dkim=pass header.i=@sourceware.org; spf=pass (google.com: domain of libc-alpha-return-79428-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-79428-patch=linaro.org@sourceware.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:in-reply-to :references; q=dns; s=default; b=Zt8rutCuujyBaD44UNip5rTAFrwyunh EeSAnh1X3uNJuxpucklsklkojTszi36NSRU68d7EP0bu7LHV37M21ZZQAJAaZVvz LNokyXA5gpozsEoe68Nr4EORir/pEDkLpRlE2H8uCD6vVFfPi8imSYxtGzdRsZA5 FV8B4A7aZctQ= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:subject:date:message-id:in-reply-to :references; s=default; bh=Uxw5Xn8gHIaujB7oE29y7ACQIw8=; b=mmkq0 X9n1ouI2cp9KBRxohIagIda6Q1CZXVA7filw/iR2Dm8qyp8Dlrgw/dJd+Hf+y9+e GFhdjqj8YeT9PNOKSCEpMpnX9qnKX9YHOUemyDExHwhmJd6aV8xVXGQyZ85FWpx2 FpUcrASQqaq44br5Vms0iQaQK3fEX0mxx5xYIM= Received: (qmail 61394 invoked by alias); 15 May 2017 19:36:37 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 61336 invoked by uid 89); 15 May 2017 19:36:37 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM, SPF_PASS autolearn=ham version=3.3.2 spammy=Signal X-HELO: mail-qk0-f182.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=nXbOIZuKVfKSJ3nEKe7D04IKBv1bSh6Q5HKEPg4vPtE=; b=CYgXT3TiqixvflV/MhF8tacw5MpnCUHGh6HI/JtlS4xjE14mfFcalCs2frhOuL7uBT BZOVq/X88wOpV+pPovZ620Vl6Mwk4FjGRKq3uoDihp+JX4SMgmq3iXebe7LjAdRABYow KtUXa5zAK77T5FvZHgHCafYOFLGt4XpkRLaz7J+z05VL8D5YsLFsrFk3iTz1LmExDirK 8N4AZij4LHKTd29TlJ4bpOkKmhOQo1Jzp2vCBfP4KLmoTWpWgkQYsX2k7yUV0tm7wKR2 63sBmdAIh5mNihLTVAqvIncMTkjKALNNQhEyDLTZKaypGnxn2mu/Hm7KdiMwZ6QvFvdw 0f1A== X-Gm-Message-State: AODbwcBy9vfJwiJikKO5+pDJHRRnJ7eLSdU6guACHA8SQRGMXAG5vQTx tsR1ShU+VaFIsZ6TCqVSqw== X-Received: by 10.55.73.68 with SMTP id w65mr3469043qka.278.1494876993942; Mon, 15 May 2017 12:36:33 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH 2/3] posix: Improve default posix_spawn implementation Date: Mon, 15 May 2017 16:36:24 -0300 Message-Id: <1494876985-21990-2-git-send-email-adhemerval.zanella@linaro.org> In-Reply-To: <1494876985-21990-1-git-send-email-adhemerval.zanella@linaro.org> References: <1494876985-21990-1-git-send-email-adhemerval.zanella@linaro.org> This patch improves the default posix implementation of posix_spawn{p} and align with Linux one. The main idea is to try shared common implementation bits with Linux and avoid code duplication, fix some issues already fixed in Linux code, and deprecated vfork internal usage (source of various bug reports). In a short: - It moves POSIX_SPAWN_USEVFORK usage and sets it a no-op. Since the process that actually spawn the new process do not share memory with parent (with vfork), it fixes BZ#14750 for this implementation. - It uses a pipe to correctly obtain the return upon failure of execution (BZ#18433). - It correctly enable/disable asynchronous cancellation (checked on ptl/tst-exec5.c). - It correctly disable/enable signal handling. Using this version instead of Linux shows only one regression, posix/tst-spawn3, because of pipe2 usage which increase total number of file descriptor. * sysdeps/posix/spawni.c (__spawni_child): New function. (__spawni): Rename to __spawnix. --- ChangeLog | 3 + sysdeps/posix/spawni.c | 362 ++++++++++++++++++++++++------------------------- 2 files changed, 182 insertions(+), 183 deletions(-) -- 2.7.4 diff --git a/sysdeps/posix/spawni.c b/sysdeps/posix/spawni.c index 9cad25c..e096a42 100644 --- a/sysdeps/posix/spawni.c +++ b/sysdeps/posix/spawni.c @@ -16,20 +16,23 @@ License along with the GNU C Library; if not, see . */ -#include +#include +#include #include #include -#include -#include -#include #include -#include -#include #include -#include "spawn_int.h" +#include +#include +#include #include #include #include +#include +#include +#include +#include +#include "spawn_int.h" /* The Unix standard contains a long explanation of the way to signal @@ -39,94 +42,59 @@ normal program exit with the exit code 127. */ #define SPAWN_ERROR 127 - -/* The file is accessible but it is not an executable file. Invoke - the shell to interpret it as a script. */ -static void -internal_function -script_execute (const char *file, char *const argv[], char *const envp[]) +struct posix_spawn_args { - /* Count the arguments. */ - int argc = 0; - while (argv[argc++]) - ; - - /* Construct an argument list for the shell. */ - { - char *new_argv[argc + 1]; - new_argv[0] = (char *) _PATH_BSHELL; - new_argv[1] = (char *) file; - while (argc > 1) - { - new_argv[argc] = argv[argc - 1]; - --argc; - } - - /* Execute the shell. */ - __execve (new_argv[0], new_argv, envp); - } -} - -static inline void -maybe_script_execute (const char *file, char *const argv[], char *const envp[], - int xflags) + sigset_t oldmask; + const char *file; + int (*exec) (const char *, char *const *, char *const *); + const posix_spawn_file_actions_t *fa; + const posix_spawnattr_t *restrict attr; + char *const *argv; + ptrdiff_t argc; + char *const *envp; + int xflags; + int pipe[2]; +}; + +/* Older version requires that shell script without shebang definition + to be called explicitly 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) - && (xflags & SPAWN_XFLAGS_TRY_SHELL) - && errno == ENOEXEC) - script_execute (file, argv, envp); -} - -/* 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 *file_actions, - const posix_spawnattr_t *attrp, char *const argv[], - char *const envp[], int xflags) -{ - pid_t new_pid; - char *path, *p, *name; - size_t len; - size_t pathlen; - - /* Do this once. */ - short int flags = attrp == NULL ? 0 : attrp->__flags; - - /* Generate the new process. */ - if ((flags & POSIX_SPAWN_USEVFORK) != 0 - /* If no major work is done, allow using vfork. Note that we - might perform the path searching. But this would be done by - a call to execvp(), too, and such a call must be OK according - to POSIX. */ - || ((flags & (POSIX_SPAWN_SETSIGMASK | POSIX_SPAWN_SETSIGDEF - | POSIX_SPAWN_SETSCHEDPARAM | POSIX_SPAWN_SETSCHEDULER - | POSIX_SPAWN_SETPGROUP | POSIX_SPAWN_RESETIDS - | POSIX_SPAWN_SETSID)) == 0 - && file_actions == NULL)) - new_pid = __vfork (); - else - new_pid = __fork (); - - if (new_pid != 0) + && (args->xflags & SPAWN_XFLAGS_TRY_SHELL) && errno == ENOEXEC) { - if (new_pid < 0) - return errno; - - /* The call was successful. Store the PID if necessary. */ - if (pid != NULL) - *pid = new_pid; + char *const *argv = args->argv; + ptrdiff_t argc = args->argc; + + /* Construct an argument list for the shell. */ + char *new_argv[argc + 1]; + new_argv[0] = (char *) _PATH_BSHELL; + new_argv[1] = (char *) args->file; + if (argc > 1) + memcpy (new_argv + 2, argv + 1, argc * sizeof(char *)); + else + new_argv[2] = NULL; - return 0; + /* 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. */ +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 ret; - /* Set signal mask. */ - if ((flags & POSIX_SPAWN_SETSIGMASK) != 0 - && __sigprocmask (SIG_SETMASK, &attrp->__ss, NULL) != 0) - _exit (SPAWN_ERROR); + __close (args->pipe[0]); /* Set signal default action. */ - if ((flags & POSIX_SPAWN_SETSIGDEF) != 0) + if ((attr->__flags & POSIX_SPAWN_SETSIGDEF) != 0) { /* We have to iterate over all signals. This could possibly be done better but it requires system specific solutions since @@ -139,41 +107,41 @@ __spawni (pid_t *pid, const char *file, sa.sa_handler = SIG_DFL; for (sig = 1; sig <= _NSIG; ++sig) - if (__sigismember (&attrp->__sd, sig) != 0 + if (__sigismember (&attr->__sd, sig) != 0 && __sigaction (sig, &sa, NULL) != 0) - _exit (SPAWN_ERROR); - + goto fail; } #ifdef _POSIX_PRIORITY_SCHEDULING /* Set the scheduling algorithm and parameters. */ - if ((flags & (POSIX_SPAWN_SETSCHEDPARAM | POSIX_SPAWN_SETSCHEDULER)) + if ((attr->__flags & (POSIX_SPAWN_SETSCHEDPARAM | POSIX_SPAWN_SETSCHEDULER)) == POSIX_SPAWN_SETSCHEDPARAM) { - if (__sched_setparam (0, &attrp->__sp) == -1) - _exit (SPAWN_ERROR); + if ((ret = __sched_setparam (0, &attr->__sp)) == -1) + goto fail; } - else if ((flags & POSIX_SPAWN_SETSCHEDULER) != 0) + else if ((attr->__flags & POSIX_SPAWN_SETSCHEDULER) != 0) { - if (__sched_setscheduler (0, attrp->__policy, &attrp->__sp) == -1) - _exit (SPAWN_ERROR); + if ((ret = __sched_setscheduler (0, attr->__policy, &attr->__sp) == -1)) + goto fail; } #endif - if ((flags & POSIX_SPAWN_SETSID) != 0 - && __setsid () < 0) - _exit (SPAWN_ERROR); + /* Set the process session ID. */ + if ((attr->__flags & POSIX_SPAWN_SETSID) != 0 + && (ret = __setsid ()) < 0) + goto fail; /* Set the process group ID. */ - if ((flags & POSIX_SPAWN_SETPGROUP) != 0 - && __setpgid (0, attrp->__pgrp) != 0) - _exit (SPAWN_ERROR); + if ((attr->__flags & POSIX_SPAWN_SETPGROUP) != 0 + && (ret =__setpgid (0, attr->__pgrp)) != 0) + goto fail; /* Set the effective user and group IDs. */ - if ((flags & POSIX_SPAWN_RESETIDS) != 0 - && (local_seteuid (__getuid ()) != 0 - || local_setegid (__getgid ()) != 0)) - _exit (SPAWN_ERROR); + 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 != NULL) @@ -191,7 +159,7 @@ __spawni (pid_t *pid, const char *file, case spawn_do_close: if (close_not_cancel (action->action.close_action.fd) != 0) { - if (! have_fdlimit) + if (have_fdlimit == 0) { __getrlimit64 (RLIMIT_NOFILE, &fdlimit); have_fdlimit = true; @@ -200,120 +168,148 @@ __spawni (pid_t *pid, const char *file, /* Only signal errors for file descriptors out of range. */ if (action->action.close_action.fd < 0 || action->action.close_action.fd >= fdlimit.rlim_cur) - /* Signal the error. */ - _exit (SPAWN_ERROR); + { + ret = -1; + goto fail; + } } break; case spawn_do_open: { + /* POSIX states that if fildes was already an open file descriptor, + it shall be closed before the new file is opened. This avoid + pontential issues when posix_spawn plus addopen action is called + with the process already at maximum number of file descriptor + opened and also for multiple actions on single-open special + paths (like /dev/watchdog). */ + close_not_cancel (action->action.open_action.fd); + int new_fd = open_not_cancel (action->action.open_action.path, action->action.open_action.oflag | O_LARGEFILE, action->action.open_action.mode); - if (new_fd == -1) - /* The `open' call failed. */ - _exit (SPAWN_ERROR); + if ((ret = new_fd) == -1) + goto fail; /* Make sure the desired file descriptor is used. */ if (new_fd != action->action.open_action.fd) { - if (__dup2 (new_fd, action->action.open_action.fd) + if ((ret = __dup2 (new_fd, action->action.open_action.fd)) != action->action.open_action.fd) - /* The `dup2' call failed. */ - _exit (SPAWN_ERROR); + goto fail; - if (close_not_cancel (new_fd) != 0) - /* The `close' call failed. */ - _exit (SPAWN_ERROR); + if ((ret = close_not_cancel (new_fd) != 0)) + goto fail; } } break; case spawn_do_dup2: - if (__dup2 (action->action.dup2_action.fd, - action->action.dup2_action.newfd) + if ((ret = __dup2 (action->action.dup2_action.fd, + action->action.dup2_action.newfd)) != action->action.dup2_action.newfd) - /* The `dup2' call failed. */ - _exit (SPAWN_ERROR); + goto fail; break; } } } - if ((xflags & SPAWN_XFLAGS_USE_PATH) == 0 || strchr (file, '/') != NULL) - { - /* The FILE parameter is actually a path. */ - __execve (file, argv, envp); + /* 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); - maybe_script_execute (file, argv, envp, xflags); + args->exec (args->file, args->argv, args->envp); - /* Oh, oh. `execve' returns. This is bad. */ - _exit (SPAWN_ERROR); - } + /* 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); - /* We have to search for FILE on the path. */ - path = getenv ("PATH"); - if (path == NULL) - { - /* There is no `PATH' in the environment. - The default search path is the current directory - followed by the path `confstr' returns for `_CS_PATH'. */ - len = confstr (_CS_PATH, (char *) NULL, 0); - path = (char *) __alloca (1 + len); - path[0] = ':'; - (void) confstr (_CS_PATH, path + 1, len); - } + ret = -errno; - len = strlen (file) + 1; - pathlen = strlen (path); - name = __alloca (pathlen + len + 1); - /* Copy the file name at the top. */ - name = (char *) memcpy (name + pathlen + 1, file, len); - /* And add the slash. */ - *--name = '/'; +fail: + /* Since sizeof errno < PIPE_BUF, the write is atomic. */ + ret = -ret; + if (ret) + while (write_not_cancel (args->pipe[1], &ret, sizeof (ret)) < 0); - p = path; - do - { - char *startp; + _exit (SPAWN_ERROR); +} - path = p; - p = __strchrnul (path, ':'); +/* Spawn a new process executing PATH with the attributes describes in *ATTRP. + Before running the process perform the actions described in FILE-ACTIONS. */ +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 *)) +{ + struct posix_spawn_args args; + int ec; - 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); + if (__pipe2 (args.pipe, O_CLOEXEC)) + return errno; - /* Try to execute this name. If it works, execv will not return. */ - __execve (startp, argv, envp); + /* Disable asynchronous cancellation. */ + int state; + __libc_ptf_call (__pthread_setcancelstate, + (PTHREAD_CANCEL_DISABLE, &state), 0); - maybe_script_execute (startp, argv, envp, xflags); + ptrdiff_t argc = 0; + ptrdiff_t limit = INT_MAX - 1; + while (argv[argc++] != NULL) + if (argc == limit) + { + errno = E2BIG; + return errno; + } - switch (errno) - { - case EACCES: - 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. */ - break; - - default: - /* Some other error means we found an executable file, but - something went wrong executing it; return the error to our - caller. */ - _exit (SPAWN_ERROR); - } + args.file = file; + args.exec = exec; + args.fa = file_actions; + args.attr = attrp ? attrp : &(const posix_spawnattr_t) { 0 }; + args.argv = argv; + args.argc = argc; + args.envp = envp; + args.xflags = xflags; + + /* Generate the new process. */ + pid_t new_pid = __fork (); + + if (new_pid == 0) + __spawni_child (&args); + else if (new_pid > 0) + { + __close (args.pipe[1]); + + if (__read (args.pipe[0], &ec, sizeof ec) != sizeof ec) + ec = 0; + else + __waitpid (new_pid, &(int) { 0 }, 0); } - while (*p++ != '\0'); + else + ec = -new_pid; - /* Return with an error. */ - _exit (SPAWN_ERROR); + __close (args.pipe[0]); + + if ((ec == 0) && (pid != NULL)) + *pid = new_pid; + + __libc_ptf_call (__pthread_setcancelstate, (state, NULL), 0); + + return ec; +} + +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) +{ + return __spawnix (pid, file, acts, attrp, argv, envp, xflags, + xflags & SPAWN_XFLAGS_USE_PATH ? __execvpe : __execve); }