From patchwork Mon May 6 19:31:19 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 163457 Delivered-To: patch@linaro.org Received: by 2002:a05:6e02:81:0:0:0:0 with SMTP id l1csp374751ilm; Mon, 6 May 2019 12:31:37 -0700 (PDT) X-Google-Smtp-Source: APXvYqwIZ243eouNH3AMuH5i8WiUXafV0w+SsLI07DluPIl4XIvVjvmJMVIe/0DSsp4w2FhEx/HY X-Received: by 2002:a63:295:: with SMTP id 143mr34421404pgc.279.1557171097198; Mon, 06 May 2019 12:31:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1557171097; cv=none; d=google.com; s=arc-20160816; b=ck8vKKAklMCimLQ81SuzfsevUdHn8w9AXtKFEFxUja1T7z8Q4EMyZH+wr7jiKf3SpJ 4LFMWb2FlAYEYJFOYRnbmxHJ3H43md9VvrwE3NXiitLPTcKC72sbDsBP4Tf9a5Sd6DAG SNvc8LmkMRREVeilBXYUiI0CpksQJOcR0FYqR9wE39Hiqf7Es6cZkuhyYBGTY+hcyV2o EEjRbAIvRxsijfArgkyKQR+voMMLwjfDUq90RmMoktS5i7T9R0PnCevDQ8hixWuUbnrB K/v5NbhBp2T76nTbFRcELgIDZXj7rNO0z1McFbS+aXVovOjxXhKMc31iQ4Fej22Z1msE NSgQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=message-id:date:subject:cc:to:from:dkim-signature:delivered-to :sender:list-help:list-post:list-archive:list-subscribe :list-unsubscribe:list-id:precedence:mailing-list:dkim-signature :domainkey-signature; bh=9NtJwTZzk/0vKWk/xWM0VUgGwu4tf7cUnVfcQyD5/G4=; b=YBTF/ATF8/ecEKfZ/+6bjBZvdXd8j98EdmpiVFrrf3+bP5qixwLk/CPaWdBSez5c29 R1R+Nn0X3zqQEFhh39DAYr8EkcYg3F0m7JCrsmfKWSIVesl0OGXIWR3NHca1bfhUiYCy FxAQHZPMphY3LgS84x2KYFRIVj+2xp0AyOmACb3upHuCCur+3h5469tyG1dxq6P9EokA p3g4ftdOrPcUAxam7YDoB31bRZJ7UmFBgz2xFQmVmeYAE0P1DgAXPdtpbmiUCtSVIZWv e/GQeIFnng3HyFTm4B0FLuXiiG83K0vMnt+qYlBnmDYgDahHn47xorwPJ7x6iZhLLMQz 7XMA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=C0g9122k; dkim=pass header.i=@linaro.org header.s=google header.b=kroRVy9y; spf=pass (google.com: domain of libc-alpha-return-101763-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-101763-patch=linaro.org@sourceware.org"; dmarc=pass (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 v20si9089687pgn.266.2019.05.06.12.31.36 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 06 May 2019 12:31:37 -0700 (PDT) Received-SPF: pass (google.com: domain of libc-alpha-return-101763-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 header.s=default header.b=C0g9122k; dkim=pass header.i=@linaro.org header.s=google header.b=kroRVy9y; spf=pass (google.com: domain of libc-alpha-return-101763-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-101763-patch=linaro.org@sourceware.org"; dmarc=pass (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:cc:subject:date:message-id; q=dns; s= default; b=W2QU1C8ppoE+gdkOaaCMpttxRaJCwHIfoHvEmCM8Xb2TcRVaT36eQ z22uLHg/Q8Q3PwZZSAnodgzIye6trX4MZql1rOJZpEHZbnfme7kvdiCaT82RAzp6 cnWFJ/hQgYEha3MZlnUqfNzo56MRTfItcBLtJktonji/ohru6EIj3w= 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:cc:subject:date:message-id; s=default; bh=PDkC1hKWOb6t3kz7SL/6T8BjsVg=; b=C0g9122k9shFdijBpp09rN1DWUTX vgAYLvthg4zNP2GdnFCN5EBoKGtIFHv+P/RJK11aooA817olzaajLAfK/McONuNa AfqRbIEpZcPWFX98Ild9En39RRfZMOE1SN7VZSKCx/axhIzXY/cMiu3KMLbime0u larsRRAQPtVDKD4= Received: (qmail 122182 invoked by alias); 6 May 2019 19:31:28 -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 121406 invoked by uid 89); 6 May 2019 19:31:28 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-20.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.1 spammy=Child, accomplish, collected, cancelled X-HELO: mail-vs1-f67.google.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=9NtJwTZzk/0vKWk/xWM0VUgGwu4tf7cUnVfcQyD5/G4=; b=kroRVy9y8X5T9A4/SZeqBpdfCCdC5CcpHOkaj5NftaXoOai3qrWMMJkqBxgHJU+dno h3aygaXO9I327YxsmjvnfiKR55Ri/JUif4fRffuV1qfjvbxdPTOmNZJ45yOCBxxPz92h +2z+qMKB4gUeiW49AWfGQvyzcZDF4M0PivqgAJt1xpndl3ZFG3dJkY24Pf+ZmirJ2zSQ AMnuZGHA3l3xdxPFR92Q+9Enx3ykmNs4b1cdJYaLLWOoNwD3qBbJWIC3CvGGU0fEohRr +RrrSsVjB2iVVsmyxcZ3apEAb/eiO1dvwtXKi2bBGg4QVaTFKpVfhFMPk/fZW/trsKo4 O9lw== Return-Path: From: Adhemerval Zanella To: libc-alpha@sourceware.org Cc: Florian Weimer Subject: [PATCH] posix: Optimize Linux posix_spawn Date: Mon, 6 May 2019 16:31:19 -0300 Message-Id: <20190506193119.20505-1-adhemerval.zanella@linaro.org> The current internal posix_spawn symbol for Linux (__spawni) requires to allocate a dynamic stack based on input arguments to handle the SPAWN_XFLAGS_USE_PATH internal flag, which re-issue the input binary as a shell script if execve call return ENOEXEC (to execute shell scripts with an initial shebang). This is done only for compatibility mode and the generic case does not require the extra calculation plus the potential large mmap/munmap call. For default case, a pre-defined buffer is sufficed to use on the clone call instead. This patch optimizes Linux spawni by allocating a dynamic stack only for compatibility symbol (SPAWN_XFLAGS_USE_PATH). For generic case, a stack allocated buffer is used instead. Checked x86_64-linux-gnu and i686-linux-gnu. * sysdeps/unix/sysv/linux/spawni.c (posix_spawn_args): Remove argc member. (maybe_script_execute): Remove function. (execve_compat, __spawni_clone, __spawnix_compat): New function. (__spawni_child): Remove maybe_script_execute call. (__spawnix): Remove magic stack slack constant with stack_slack identifier. (__spawni): Only allocates a variable stack when SPAWN_XFLAGS_TRY_SHELL is used. --- sysdeps/unix/sysv/linux/spawni.c | 205 ++++++++++++++++++------------- 1 file changed, 118 insertions(+), 87 deletions(-) -- 2.17.1 diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c index c1abf3f960..822cce2a77 100644 --- a/sysdeps/unix/sysv/linux/spawni.c +++ b/sysdeps/unix/sysv/linux/spawni.c @@ -74,6 +74,11 @@ # define STACK(__stack, __stack_size) (__stack + __stack_size) #endif +/* Additional stack size added on required space to execute the clone child + function (__spawni_child). */ +enum { + stack_slack = 512 +}; struct posix_spawn_args { @@ -83,35 +88,39 @@ struct posix_spawn_args 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 err; }; -/* 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) +/* This is compatibility function required to enable posix_spawn run + script without shebang definition for older posix_spawn versions + (2.15). */ +static int +execve_compat (const char *filename, char *const argv[], char *const envp[]) { - if (SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_15) - && (args->xflags & SPAWN_XFLAGS_TRY_SHELL) && errno == ENOEXEC) + __execve (filename, argv, envp); + + if (errno == ENOEXEC) { - char *const *argv = args->argv; - ptrdiff_t argc = args->argc; + char *const *cargv = argv; + ptrdiff_t argc = 0; + while (cargv[argc++] != NULL); /* Construct an argument list for the shell. */ char *new_argv[argc + 2]; new_argv[0] = (char *) _PATH_BSHELL; - new_argv[1] = (char *) args->file; + new_argv[1] = (char *) filename; if (argc > 1) memcpy (new_argv + 2, argv + 1, argc * sizeof (char *)); else new_argv[2] = NULL; /* Execute the shell. */ - args->exec (new_argv[0], new_argv, args->envp); + __execve (new_argv[0], new_argv, envp); } + + return -1; } /* Function used in the clone call to setup the signals mask, posix_spawn @@ -291,11 +300,6 @@ __spawni_child (void *arguments) args->exec (args->file, 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); - fail: /* errno should have an appropriate non-zero value; otherwise, there's a bug in glibc or the kernel. For lack of an error code @@ -306,18 +310,61 @@ fail: _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 *)) +__spawni_clone (struct posix_spawn_args *args, void *stack, size_t stack_size, + pid_t *pid) { - pid_t new_pid; - struct posix_spawn_args args; int ec; + pid_t new_pid; + + /* The clone flags used will create a new child that will run in the same + memory space (CLONE_VM) and the execution of calling thread will be + suspend until the child calls execve or _exit. + + Also since the calling thread execution will be suspend, there is not + need for CLONE_SETTLS. Although parent and child share the same TLS + namespace, there will be no concurrent access for TLS variables (errno + for instance). */ + new_pid = CLONE (__spawni_child, STACK (stack, stack_size), stack_size, + CLONE_VM | CLONE_VFORK | SIGCHLD, args); + + /* It needs to collect the case where the auxiliary process was created + but failed to execute the file (due either any preparation step or + for execve itself). */ + if (new_pid > 0) + { + /* Also, it handles the unlikely case where the auxiliary process was + terminated before calling execve as if it was successfully. The + args.err is set to 0 as default and changed to a positive value + only in case of failure, so in case of premature termination + due a signal args.err will remain zeroed and it will be up to + caller to actually collect it. */ + ec = args->err; + if (ec > 0) + /* There still an unlikely case where the child is cancelled after + setting args.err, due to a positive error value. Also there is + possible pid reuse race (where the kernel allocated the same pid + to an unrelated process). Unfortunately due synchronization + issues where the kernel might not have the process collected + the waitpid below can not use WNOHANG. */ + __waitpid (new_pid, NULL, 0); + } + else + ec = -new_pid; + + if ((ec == 0) && (pid != NULL)) + *pid = new_pid; + + return ec; +} + +/* Allocates a stack using mmap to call clone. The stack size is based on + number of arguments since it would be used on compat mode which may call + execvpe/execve_compat. */ +static int +__spawnix_compat (struct posix_spawn_args *args, pid_t *pid) +{ + char *const *argv = args->argv; /* To avoid imposing hard limits on posix_spawn{p} the total number of arguments is first calculated to allocate a mmap to hold all possible @@ -340,7 +387,8 @@ __spawnix (pid_t * pid, const char *file, | ((GL (dl_stack_flags) & PF_X) ? PROT_EXEC : 0)); /* Add a slack area for child's stack. */ - size_t argv_size = (argc * sizeof (void *)) + 512; + size_t argv_size = (argc * sizeof (void *)) + stack_slack; + /* We need at least a few pages in case the compiler's stack checking is enabled. In some configs, it is known to use at least 24KiB. We use 32KiB to be "safe" from anything the compiler might do. Besides, the @@ -352,64 +400,61 @@ __spawnix (pid_t * pid, const char *file, if (__glibc_unlikely (stack == MAP_FAILED)) return errno; + int ec = __spawni_clone (args, stack, stack_size, pid); + + __munmap (stack, stack_size); + + 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 * file_actions, + const posix_spawnattr_t * attrp, char *const argv[], + char *const envp[], int xflags) +{ + /* For SPAWN_XFLAGS_TRY_SHELL we need to execute a script even without + a shebang. To accomplish it we pass as callback to __spawni_child + __execvpe (which call maybe_script_execute for such case) or + execve_compat (which mimics the semantic using execve). */ + int (*exec) (const char *, char *const *, char *const *) = + xflags & SPAWN_XFLAGS_TRY_SHELL + ? xflags & SPAWN_XFLAGS_USE_PATH ? __execvpe : execve_compat + : xflags & SPAWN_XFLAGS_USE_PATH ? __execvpex : __execve; + + /* Child must set args.err to something non-negative - we rely on + the parent and child sharing VM. */ + struct posix_spawn_args args = { + .err = 0, + .file = file, + .exec = exec, + .fa = file_actions, + .attr = attrp ? attrp : &(const posix_spawnattr_t) { 0 }, + .argv = argv, + .envp = envp, + .xflags = xflags + }; + /* Disable asynchronous cancellation. */ int state; __libc_ptf_call (__pthread_setcancelstate, (PTHREAD_CANCEL_DISABLE, &state), 0); - /* Child must set args.err to something non-negative - we rely on - the parent and child sharing VM. */ - args.err = 0; - 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; - __libc_signal_block_all (&args.oldmask); - /* The clone flags used will create a new child that will run in the same - memory space (CLONE_VM) and the execution of calling thread will be - suspend until the child calls execve or _exit. - - Also since the calling thread execution will be suspend, there is not - need for CLONE_SETTLS. Although parent and child share the same TLS - namespace, there will be no concurrent access for TLS variables (errno - for instance). */ - new_pid = CLONE (__spawni_child, STACK (stack, stack_size), stack_size, - CLONE_VM | CLONE_VFORK | SIGCHLD, &args); + int ec; - /* It needs to collect the case where the auxiliary process was created - but failed to execute the file (due either any preparation step or - for execve itself). */ - if (new_pid > 0) + if (__glibc_likely ((xflags & SPAWN_XFLAGS_TRY_SHELL) == 0)) { - /* Also, it handles the unlikely case where the auxiliary process was - terminated before calling execve as if it was successfully. The - args.err is set to 0 as default and changed to a positive value - only in case of failure, so in case of premature termination - due a signal args.err will remain zeroed and it will be up to - caller to actually collect it. */ - ec = args.err; - if (ec > 0) - /* There still an unlikely case where the child is cancelled after - setting args.err, due to a positive error value. Also there is - possible pid reuse race (where the kernel allocated the same pid - to an unrelated process). Unfortunately due synchronization - issues where the kernel might not have the process collected - the waitpid below can not use WNOHANG. */ - __waitpid (new_pid, NULL, 0); + /* execvpe allocates at least (NAME_MAX + 1) + PATH_MAX to create the + combination of PATH entry and program name. */ + char stack[(NAME_MAX + 1) + PATH_MAX + stack_slack]; + ec = __spawni_clone (&args, stack, sizeof stack, pid); } else - ec = -new_pid; - - __munmap (stack, stack_size); - - if ((ec == 0) && (pid != NULL)) - *pid = new_pid; + ec = __spawnix_compat (&args, pid); __libc_signal_restore_set (&args.oldmask); @@ -417,17 +462,3 @@ __spawnix (pid_t * pid, const char *file, 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) -{ - /* It uses __execvpex to avoid run ENOEXEC in non compatibility mode (it - will be handled by maybe_script_execute). */ - return __spawnix (pid, file, acts, attrp, argv, envp, xflags, - xflags & SPAWN_XFLAGS_USE_PATH ? __execvpex :__execve); -}