From patchwork Thu Oct 25 17:41:02 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 149540 Delivered-To: patch@linaro.org Received: by 2002:a2e:299d:0:0:0:0:0 with SMTP id p29-v6csp1635042ljp; Thu, 25 Oct 2018 10:41:23 -0700 (PDT) X-Google-Smtp-Source: AJdET5fIizn5RJwqZ3VIJKPN86xd4vV4Y7xLVdtPSq1vDSGcPB7Z0Bk3KqvQW5lldXcnnLQqndX0 X-Received: by 2002:a17:902:be11:: with SMTP id r17-v6mr145395pls.95.1540489283758; Thu, 25 Oct 2018 10:41:23 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1540489283; cv=none; d=google.com; s=arc-20160816; b=EmMyc9tJmVuoI4A6pSgr3dpxYdUipkEsEdBi330Qjb4C48v6JS3y1wlVigPV925cvq EmBkeYrVpsFREae2HrujMIkj/jbEkAL+Orp6Ga1enn+sVQFaGbstXfBW2A+QpcdyIBKr tgAecPB8UARphV7g3E2IXCW+Eb4AdotAldbM4Fnw9BFzHrWC/Wsc479JA/LPaU4r0lFU CU5IfDwmODvUttjg1TMnrCDsT6uYyBRTC6d21NrAw69ngyf8HIoXKOrU6TwGo9MO2GvN LpQGKBooQgsEFNyFv8TL7+NmFsveISxfFv+8Yuh1detv1MyM2hn5Uvl5bbWMvIpnbx1C gIzA== 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=VVxbSTJiXPo/pA8pgavLue1cDhdzwquhYQphqnLohwI=; b=Um8modkG1uGHOg6iXRLkcbZNxSzsHuzOu46inUmvSaJ5K39sb1y23J2zvwUszepEAp ALHrsYFv73FtRmoywlbqnS9o08RcZmu+lCExnxsDfdRvcJIwSVVXv1Xkoy++IQ651WRk upuevyRbYvt4WU2esIv4FXSbDnjBYcX+dZD5/Wefa3DXahIfdzB4IjMC7sDYk3lqzMwt q6CzTIET25GaarEEJo0TVImPWciB3k8lvIzuIqmL5ZG9axW9e+R+0UhsTVqabRr9hs05 HzV18m/LymCocvCY3PbFA8s6LtKbeEG3TPKGRUhLOorjUHAj26FM4KlRHJUn2Zb76VZ8 mh2g== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=Ygazj3eN; dkim=pass header.i=@linaro.org header.s=google header.b=LLF8BQ13; spf=pass (google.com: domain of libc-alpha-return-96743-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-96743-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 g198-v6si8202370pfb.165.2018.10.25.10.41.23 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 25 Oct 2018 10:41:23 -0700 (PDT) Received-SPF: pass (google.com: domain of libc-alpha-return-96743-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=Ygazj3eN; dkim=pass header.i=@linaro.org header.s=google header.b=LLF8BQ13; spf=pass (google.com: domain of libc-alpha-return-96743-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-96743-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=A9Dt08+f8rFVYdJPN0yGz4HUCy4l9kqv8kmHcz+kdnlQiGakIr/1y DFAzKfBCSJMop05HsB1ZZFmCha0GITxMu7qA8DVWICnOHz+wbNY0Kz3HU1mkMlGX J2/2h2JegOUGV2EUBOFEGO9IZvGfQNgleKlfgNtdqe/v5odj2sAAvw= 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=Dr9p2HfvSI26Ef4N8dBxzmMJmKI=; b=Ygazj3eNujfNYPZKwA0btRGQO3ae LKHMxzAiAMSKW1c3S9qvcw5jV9Ml5S5ZQXC71Ul+p6IbxbIb89iL6sCsqzpkoDfQ cu/IhsRqxjtHLi1bWKm5kUm/LccLeNoqpVGXkKRpEywZth93iqsAy9/mm9sHzYG2 TnOr8g5rmtww6HI= Received: (qmail 120902 invoked by alias); 25 Oct 2018 17:41:14 -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 120887 invoked by uid 89); 25 Oct 2018 17:41:13 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.1 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_ASCII_DIVIDERS, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=lazily X-HELO: mail-qt1-f195.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=VVxbSTJiXPo/pA8pgavLue1cDhdzwquhYQphqnLohwI=; b=LLF8BQ130m4n8R8izY2TDGyNh7scfGyWeufEfU4Q9zOWYq/wScjE+s/Jx38JkfiJJx Mv//6I28Zqee8r9xOMUXUJvq114s3u1S4fCAmSyfIWXJMHfJoOhMG7CR8N89jqU5y+qW p23+bOILYFVWAs+W+Im6Yb3Yj4Vo4n5dXrOfs= Return-Path: From: Adhemerval Zanella To: libc-alpha@sourceware.org Cc: fweimer@redhat.com Subject: [PATCH v3 1/2] posix: Use posix_spawn on popen Date: Thu, 25 Oct 2018 14:41:02 -0300 Message-Id: <20181025174103.31596-1-adhemerval.zanella@linaro.org> This patch uses posix_spawn on popen instead of fork and execl. On Linux this has the advantage of much lower memory consumption (usually 32 Kb minimum for the mmap stack area). Two issues are also fixed with this change: * BZ#17490: although POSIX pthread_atfork description only list 'fork' as the function that should execute the atfork handlers, popen description states that: '[...] shall be *as if* a child process were created within the popen() call using the fork() function [...]' Other libc/system seems to follow the idea atfork handlers should not be executed for popen: libc/system | run atfork handles | notes ------------|----------------------|--------------------------------------- Freebsd | no | uses vfork Solaris 11 | no | MacOSX 11 | no | implemented through posix_spawn syscall ------------|----------------------|---------------------------------------- Similar to posix_spawn and system, popen idea is to spawn a different binary so all the POSIX rationale to run the atfork handlers to avoid internal process inconsistency is not really required and in some cases might be unsafe. * BZ#22834: now that proc_file_chain is not copied on another process, it just require to access is through the proc_file_chain_lock. Checked on x86_64-linux-gnu and i686-linux-gnu. [BZ #22834] [BZ #17490] * NEWS: Add new semantic for atfork with popen and system. * libio/iopopen.c (_IO_new_proc_open): use posix_spawn instead of fork and execl. --- ChangeLog | 6 +++ NEWS | 6 +++ libio/iopopen.c | 116 +++++++++++++++++++++++++++++++----------------- 3 files changed, 87 insertions(+), 41 deletions(-) -- 2.17.1 diff --git a/NEWS b/NEWS index f054dc0433..c76813d12c 100644 --- a/NEWS +++ b/NEWS @@ -30,6 +30,12 @@ Major new features: HTM state is saved and restore lazily (the state being saved even when the process actually does not use HTM). +* The popen and system do not run atfork handlers anymore (BZ#17490). + Although it is a possible POSIX violation, the POSIX rationale in + pthread_atfork documentation regarding atfork handlers is to handle + incosistent mutex state after fork call in multithread environment. + In both popen and system there is no direct access to user-defined mutexes. + Deprecated and removed features, and other changes affecting compatibility: * The glibc.tune tunable namespace has been renamed to glibc.cpu and the diff --git a/libio/iopopen.c b/libio/iopopen.c index 2eff45b4c8..da24e60eef 100644 --- a/libio/iopopen.c +++ b/libio/iopopen.c @@ -34,7 +34,8 @@ #include #include #include -#include +#include +#include struct _IO_proc_file { @@ -64,8 +65,8 @@ _IO_new_proc_open (FILE *fp, const char *command, const char *mode) { int read_or_write; int parent_end, child_end; + int child_std_end; int pipe_fds[2]; - pid_t child_pid; int do_read = 0; int do_write = 0; @@ -108,59 +109,92 @@ _IO_new_proc_open (FILE *fp, const char *command, const char *mode) if (do_read) { - parent_end = pipe_fds[0]; - child_end = pipe_fds[1]; + parent_end = 0; + child_end = 1; read_or_write = _IO_NO_WRITES; + child_std_end = 1; } else { - parent_end = pipe_fds[1]; - child_end = pipe_fds[0]; + parent_end = 1; + child_end = 0; read_or_write = _IO_NO_READS; + child_std_end = 0; } - ((_IO_proc_file *) fp)->pid = child_pid = __fork (); - if (child_pid == 0) - { - int child_std_end = do_read ? 1 : 0; - struct _IO_proc_file *p; - - if (child_end != child_std_end) - __dup2 (child_end, child_std_end); - else - /* The descriptor is already the one we will use. But it must - not be marked close-on-exec. Undo the effects. */ - __fcntl (child_end, F_SETFD, 0); - /* POSIX.2: "popen() shall ensure that any streams from previous - popen() calls that remain open in the parent process are closed - in the new child process." */ - for (p = proc_file_chain; p; p = p->next) - { - int fd = _IO_fileno ((FILE *) p); + { + posix_spawn_file_actions_t fa; + /* posix_spawn_file_actions_init does not fail. */ + __posix_spawn_file_actions_init (&fa); - /* If any stream from previous popen() calls has fileno - child_std_end, it has been already closed by the dup2 syscall - above. */ - if (fd != child_std_end) - __close_nocancel (fd); - } + /* The descriptor is already the one the child will use. In this case + it must be moved to another one otherwise, there is no safe way to + remove the close-on-exec flag in the child without creating a FD leak + race in the parent. */ + if (pipe_fds[child_end] == child_std_end) + { + int tmp = __fcntl (child_std_end, F_DUPFD_CLOEXEC, 0); + if (tmp < 0) + goto spawn_failure; + __close_nocancel (pipe_fds[child_end]); + pipe_fds[child_end] = tmp; + } - execl ("/bin/sh", "sh", "-c", command, (char *) 0); - _exit (127); - } - __close_nocancel (child_end); - if (child_pid < 0) - { - __close_nocancel (parent_end); - return NULL; - } + if (__posix_spawn_file_actions_adddup2 (&fa, pipe_fds[child_end], + child_std_end) != 0) + goto spawn_failure; + + /* POSIX.2: "popen() shall ensure that any streams from previous popen() + calls that remain open in the parent process are closed in the new + child process." */ + bool addclose_failure = false; +#ifdef _IO_MTSAFE_IO + _IO_cleanup_region_start_noarg (unlock); + _IO_lock_lock (proc_file_chain_lock); +#endif + for (struct _IO_proc_file *p = proc_file_chain; p; p = p->next) + { + int fd = _IO_fileno ((FILE *) p); + + /* If any stream from previous popen() calls has fileno + child_send, it has been already closed by the dup2 syscall + above. */ + if (fd != child_std_end + && __posix_spawn_file_actions_addclose (&fa, fd) != 0) + { + addclose_failure = true; + break; + } + } +#ifdef _IO_MTSAFE_IO + _IO_lock_unlock (proc_file_chain_lock); + _IO_cleanup_region_end (0); +#endif + if (addclose_failure) + goto spawn_failure; + + if (__posix_spawn (&((_IO_proc_file *) fp)->pid, _PATH_BSHELL, &fa, 0, + (char *const[]){ (char*) "sh", (char*) "-c", + (char *) command, NULL }, __environ) != 0) + { + spawn_failure: + __posix_spawn_file_actions_destroy (&fa); + __close_nocancel (pipe_fds[child_end]); + __close_nocancel (pipe_fds[parent_end]); + __set_errno (ENOMEM); + return NULL; + } + + __posix_spawn_file_actions_destroy (&fa); + } + __close_nocancel (pipe_fds[child_end]); if (!do_cloexec) /* Undo the effects of the pipe2 call which set the close-on-exec flag. */ - __fcntl (parent_end, F_SETFD, 0); + __fcntl (pipe_fds[parent_end], F_SETFD, 0); - _IO_fileno (fp) = parent_end; + _IO_fileno (fp) = pipe_fds[parent_end]; /* Link into proc_file_chain. */ #ifdef _IO_MTSAFE_IO From patchwork Thu Oct 25 17:41:03 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 149541 Delivered-To: patch@linaro.org Received: by 2002:a2e:299d:0:0:0:0:0 with SMTP id p29-v6csp1635207ljp; Thu, 25 Oct 2018 10:41:33 -0700 (PDT) X-Google-Smtp-Source: AJdET5dvdQwGNcm2A8JRZHTXjimeUmeRsC/lAxKfYf+H3ePsIrTYksihUVJhwjNk5Vkdy6taRiTW X-Received: by 2002:a63:2315:: with SMTP id j21mr137450pgj.297.1540489293462; Thu, 25 Oct 2018 10:41:33 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1540489293; cv=none; d=google.com; s=arc-20160816; b=D55+4nAn64OFGLbqafnYDUx+FkN0z/tEWh0GD7BW8Fr7bn+SBYNNjwh2DqRl1K1H+J w2tPPZkKfj/Zdoks0iInsDC/VKoEqfIm6j0PG0nXyJMDqe14vCln0RlpSTa2VxHC7gJ6 WCqssm3ym04Btd8P7mxqawJodDMcaqkkOPPI+NHToh1I4GJ28pQiuoPGNcZn6pvsi2UE bFkpMlOh6v6Ah4ILj/yYro1nJW99Ixw8vQpAqMdjc7fmnxQxjSeHNB4Xhn+GjaMiu7af 7pVOH9U5vdPG9B/sn1fL7fLO7CGduMm57JsCJeMHkZ/CLIFWY/uqoe05yKcWFcaBST6g H+cA== 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: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=hAie3QBuMpA5+D2XCIL230w5DBHntDgmr7JE84NVbwI=; b=MRs5Dix7V8HcDu1llg3IsjzV+V1nVEOrD5tdtm8AA53YGIYV2etcPbSSArIqO8jFla XZCeGWW8wx+4MW7J7Ld4wTVdprwMCdYuJmrXgJQMlYN6F9xX4/dF+vHgXFfyVjs8doXo 5lorwWTBDxRoxhuAy5zFHuACBMQPzZdtoEkK6cZPck/r6RI66wBhLz8FsLaEUgjgsGtp LJ84BcZW+0XuV/kD4zIYDteBYCjxiNA215UoblrcjrxCrBzSKay9Gmg4JTdH1ajaf5kY jUNcYe1lih3Wno0P+6mu+sEPWZr6DJbSw7GxxeLNKuOmsiax8H9shcrSeeys0ohwwyD5 CuWw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=JU19li89; dkim=pass header.i=@linaro.org header.s=google header.b=V5Cf1Zhq; spf=pass (google.com: domain of libc-alpha-return-96744-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-96744-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 n4-v6si8569944pgj.364.2018.10.25.10.41.33 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 25 Oct 2018 10:41:33 -0700 (PDT) Received-SPF: pass (google.com: domain of libc-alpha-return-96744-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=JU19li89; dkim=pass header.i=@linaro.org header.s=google header.b=V5Cf1Zhq; spf=pass (google.com: domain of libc-alpha-return-96744-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom="libc-alpha-return-96744-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:in-reply-to :references; q=dns; s=default; b=VqZuymYzQQqUkxKqwvfDcJwILXSABZh gfvNPcnyLv6uAGcJTHNFrbsKtZnakAUBI0vZmXQL9vBlv3qkwYy9VJU3LR3gYgd+ w9uQ0YnFGdAlg2dtPxJLjQyprsqQhSlDpJEQUr6jeQ83bv/ixShggpmqFCWdbq0o +p8CXJ0PY8+k= 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:in-reply-to :references; s=default; bh=uyXbBquRMxuSoNJJcOy4i47Bob8=; b=JU19l i89eKCOCnOt2EVceZgmcnV06U+NaqxHNbXvIrAA+sV8HC1XslxYZFkxRxHZlDvir qNDqEzGXLbHB765SUsobVfTHFuVRAD6wtpM5pa8fjps5ZMhXi1WKIDl768/PNShD m78Vyo2w6dt+vt1aqV8uITclrbDrXgC+2a+mNs= Received: (qmail 121900 invoked by alias); 25 Oct 2018 17:41:22 -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 121410 invoked by uid 89); 25 Oct 2018 17:41:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.8 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=broader X-HELO: mail-qt1-f180.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:in-reply-to:references; bh=hAie3QBuMpA5+D2XCIL230w5DBHntDgmr7JE84NVbwI=; b=V5Cf1ZhqlwRxVKWzb+vAu+15+woSfM628rdc0n/fmHgXff2VJ9GTHKvfD/2HOeXT1z /kxdq/vtpC7fg11HSTfDN1avYX404ucXyQQj0PTjOujJwYx1H237vYm7kjXgtUVhJ9N0 e967cju3XVsJ9IjCxnthvR0T3KmTnkCy6leiQ= Return-Path: From: Adhemerval Zanella To: libc-alpha@sourceware.org Cc: fweimer@redhat.com Subject: [PATCH v3 2/2] posix: Use posix_spawn on system Date: Thu, 25 Oct 2018 14:41:03 -0300 Message-Id: <20181025174103.31596-2-adhemerval.zanella@linaro.org> In-Reply-To: <20181025174103.31596-1-adhemerval.zanella@linaro.org> References: <20181025174103.31596-1-adhemerval.zanella@linaro.org> This patch uses posix_spawn on system implementation. On Linux this has the advantage of much lower memory consumption (usually 32 Kb minimum for the mmap stack area). Although POSIX does not require, glibc system implementation aims to be thread and cancellation safe. The cancellation code is moved to generic implementation and enabled iff SIGCANCEL is defined (similar on how the cancellation handler is enabled on nptl-init.c). The _LIBC_REENTRANT related code is removed and the signal handler reset when cancellation happens is handled all by the cleanup handler (system is already a AS-unsafe function). Checked on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu, arm-linux-gnueabihf, and powerpc64le-linux-gnu. * sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Use __sigismember instead of sigismember. * sysdeps/posix/system.c [SIGCANCEL] (cancel_handler_args, cancel_handler): New definitions. (CLEANUP_HANDLER, CLEANUP_RESET): Likewise. (DO_LOCK, DO_UNLOCK, INIT_LOCK, ADD_REF, SUB_REF): Remove. (do_system): Use posix_spawn instead of fork and execl and remove reentracy code. * sysdeps/generic/not-errno.h (__kill_noerrno): New prototype. * sysdeps/unix/sysv/linux/not-errno.h (__kill_noerrno): Likewise. * sysdeps/unix/sysv/linux/ia64/system.c: Remove file. * sysdeps/unix/sysv/linux/s390/system.c: Likewise. * sysdeps/unix/sysv/linux/sparc/system.c: Likewise. * sysdeps/unix/sysv/linux/system.c: Likewise. --- ChangeLog | 15 ++ sysdeps/generic/not-errno.h | 2 + sysdeps/posix/system.c | 201 +++++++++++-------------- sysdeps/unix/sysv/linux/ia64/system.c | 30 ---- sysdeps/unix/sysv/linux/not-errno.h | 14 ++ sysdeps/unix/sysv/linux/s390/system.c | 29 ---- sysdeps/unix/sysv/linux/sparc/system.c | 29 ---- sysdeps/unix/sysv/linux/spawni.c | 4 +- sysdeps/unix/sysv/linux/system.c | 76 ---------- 9 files changed, 119 insertions(+), 281 deletions(-) delete mode 100644 sysdeps/unix/sysv/linux/ia64/system.c delete mode 100644 sysdeps/unix/sysv/linux/s390/system.c delete mode 100644 sysdeps/unix/sysv/linux/sparc/system.c delete mode 100644 sysdeps/unix/sysv/linux/system.c -- 2.17.1 diff --git a/sysdeps/generic/not-errno.h b/sysdeps/generic/not-errno.h index 93617a3266..0fd66b5c5e 100644 --- a/sysdeps/generic/not-errno.h +++ b/sysdeps/generic/not-errno.h @@ -17,3 +17,5 @@ . */ extern __typeof (__access) __access_noerrno attribute_hidden; + +extern __typeof (__kill) __kill_noerrno attribute_hidden; diff --git a/sysdeps/posix/system.c b/sysdeps/posix/system.c index d7594436ed..c762f15995 100644 --- a/sysdeps/posix/system.c +++ b/sysdeps/posix/system.c @@ -17,36 +17,63 @@ #include #include -#include #include #include +#include +#include +#include #include #include -#include -#include -#include +#include +#include +#include +#include #define SHELL_PATH "/bin/sh" /* Path of the shell. */ #define SHELL_NAME "sh" /* Name to give it. */ +/* We have to and actually can handle cancelable system(). The big + problem: we have to kill the child process if necessary. To do + this a cleanup handler has to be registered and it has to be able + to find the PID of the child. The main problem is to reliable have + the PID when needed. It is not necessary for the parent thread to + return. It might still be in the kernel when the cancellation + request comes. Therefore we have to use the clone() calls ability + to have the kernel write the PID into the user-level variable. */ -#ifdef _LIBC_REENTRANT -static struct sigaction intr, quit; -static int sa_refcntr; -__libc_lock_define_initialized (static, lock); +#ifdef SIGCANCEL + +struct cancel_handler_args +{ + struct sigaction *quit; + struct sigaction *intr; + pid_t pid; +}; + +static void +cancel_handler (void *arg) +{ + struct cancel_handler_args *args = (struct cancel_handler_args *) (arg); -# define DO_LOCK() __libc_lock_lock (lock) -# define DO_UNLOCK() __libc_lock_unlock (lock) -# define INIT_LOCK() ({ __libc_lock_init (lock); sa_refcntr = 0; }) -# define ADD_REF() sa_refcntr++ -# define SUB_REF() --sa_refcntr + __kill_noerrno (args->pid, SIGKILL); + + TEMP_FAILURE_RETRY (__waitpid (args->pid, NULL, 0)); + + __sigaction (SIGQUIT, args->quit, NULL); + __sigaction (SIGINT, args->intr, NULL); +} +# define CLEANUP_HANDLER(q, i, p) \ + __libc_cleanup_region_start (1, cancel_handler, \ + &((struct cancel_handler_args) { \ + .quit = &(q), \ + .intr = &(i), \ + .pid = (p) })) +# define CLEANUP_RESET() \ + __libc_cleanup_region_end (0) #else -# define DO_LOCK() -# define DO_UNLOCK() -# define INIT_LOCK() -# define ADD_REF() 0 -# define SUB_REF() 0 +# define CLEANUP_HANDLER(q, i, p) +# define CLEANUP_RESET() #endif @@ -54,122 +81,66 @@ __libc_lock_define_initialized (static, lock); static int do_system (const char *line) { - int status, save; + int status; pid_t pid; struct sigaction sa; -#ifndef _LIBC_REENTRANT struct sigaction intr, quit; -#endif sigset_t omask; + sigset_t reset; sa.sa_handler = SIG_IGN; sa.sa_flags = 0; __sigemptyset (&sa.sa_mask); - DO_LOCK (); - if (ADD_REF () == 0) - { - if (__sigaction (SIGINT, &sa, &intr) < 0) - { - (void) SUB_REF (); - goto out; - } - if (__sigaction (SIGQUIT, &sa, &quit) < 0) - { - save = errno; - (void) SUB_REF (); - goto out_restore_sigint; - } - } - DO_UNLOCK (); + /* sigaction can not fail with SIGINT/SIGQUIT used with SIG_IGN. */ + __sigaction (SIGINT, &sa, &intr); + __sigaction (SIGQUIT, &sa, &quit); - /* We reuse the bitmap in the 'sa' structure. */ __sigaddset (&sa.sa_mask, SIGCHLD); - save = errno; - if (__sigprocmask (SIG_BLOCK, &sa.sa_mask, &omask) < 0) - { -#ifndef _LIBC - if (errno == ENOSYS) - __set_errno (save); - else -#endif - { - DO_LOCK (); - if (SUB_REF () == 0) - { - save = errno; - (void) __sigaction (SIGQUIT, &quit, (struct sigaction *) NULL); - out_restore_sigint: - (void) __sigaction (SIGINT, &intr, (struct sigaction *) NULL); - __set_errno (save); - } - out: - DO_UNLOCK (); - return -1; - } - } - -#ifdef CLEANUP_HANDLER - CLEANUP_HANDLER; -#endif - -#ifdef FORK - pid = FORK (); -#else - pid = __fork (); -#endif - if (pid == (pid_t) 0) - { - /* Child side. */ - const char *new_argv[4]; - new_argv[0] = SHELL_NAME; - new_argv[1] = "-c"; - new_argv[2] = line; - new_argv[3] = NULL; - - /* Restore the signals. */ - (void) __sigaction (SIGINT, &intr, (struct sigaction *) NULL); - (void) __sigaction (SIGQUIT, &quit, (struct sigaction *) NULL); - (void) __sigprocmask (SIG_SETMASK, &omask, (sigset_t *) NULL); - INIT_LOCK (); - - /* Exec the shell. */ - (void) __execve (SHELL_PATH, (char *const *) new_argv, __environ); - _exit (127); - } - else if (pid < (pid_t) 0) - /* The fork failed. */ - status = -1; - else - /* Parent side. */ + /* sigprocmask can not fail with SIG_BLOCK used with valid input + arguments. */ + __sigprocmask (SIG_BLOCK, &sa.sa_mask, &omask); + + __sigemptyset (&reset); + if (intr.sa_handler != SIG_IGN) + __sigaddset(&reset, SIGINT); + if (quit.sa_handler != SIG_IGN) + __sigaddset(&reset, SIGQUIT); + + posix_spawnattr_t spawn_attr; + /* None of the posix_spawnattr_* function returns an error, including + posix_spawnattr_setflags for the follow specific usage (using valid + flags). */ + __posix_spawnattr_init (&spawn_attr); + __posix_spawnattr_setsigmask (&spawn_attr, &omask); + __posix_spawnattr_setsigdefault (&spawn_attr, &reset); + __posix_spawnattr_setflags (&spawn_attr, + POSIX_SPAWN_SETSIGDEF | POSIX_SPAWN_SETSIGMASK); + + status = __posix_spawn (&pid, SHELL_PATH, 0, &spawn_attr, + (char *const[]){ (char*) SHELL_NAME, + (char*) "-c", + (char *) line, NULL }, + __environ); + __posix_spawnattr_destroy (&spawn_attr); + + if (status == 0) { + /* Cancellation results in cleanup handlers running as exceptions in + the block where they were installed, so it is safe to reference + stack variable allocate in the broader scope. */ + CLEANUP_HANDLER (quit, intr, pid); /* Note the system() is a cancellation point. But since we call waitpid() which itself is a cancellation point we do not have to do anything here. */ if (TEMP_FAILURE_RETRY (__waitpid (pid, &status, 0)) != pid) status = -1; + CLEANUP_RESET (); } -#ifdef CLEANUP_HANDLER - CLEANUP_RESET; -#endif - - save = errno; - DO_LOCK (); - if ((SUB_REF () == 0 - && (__sigaction (SIGINT, &intr, (struct sigaction *) NULL) - | __sigaction (SIGQUIT, &quit, (struct sigaction *) NULL)) != 0) - || __sigprocmask (SIG_SETMASK, &omask, (sigset_t *) NULL) != 0) - { -#ifndef _LIBC - /* glibc cannot be used on systems without waitpid. */ - if (errno == ENOSYS) - __set_errno (save); - else -#endif - status = -1; - } - DO_UNLOCK (); + __sigaction (SIGINT, &intr, NULL); + __sigaction (SIGQUIT, &quit, NULL); + __sigprocmask (SIG_SETMASK, &omask, NULL); return status; } diff --git a/sysdeps/unix/sysv/linux/ia64/system.c b/sysdeps/unix/sysv/linux/ia64/system.c deleted file mode 100644 index d09fefefe6..0000000000 --- a/sysdeps/unix/sysv/linux/ia64/system.c +++ /dev/null @@ -1,30 +0,0 @@ -/* Copyright (C) 2002-2018 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 - . */ - -/* We have to and actually can handle cancelable system(). The big - problem: we have to kill the child process if necessary. To do - this a cleanup handler has to be registered and is has to be able - to find the PID of the child. The main problem is to reliable have - the PID when needed. It is not necessary for the parent thread to - return. It might still be in the kernel when the cancellation - request comes. Therefore we have to use the clone() calls ability - to have the kernel write the PID into the user-level variable. */ -#define FORK() \ - INLINE_SYSCALL (clone2, 6, CLONE_PARENT_SETTID | SIGCHLD, NULL, 0, \ - &pid, NULL, NULL) - -#include diff --git a/sysdeps/unix/sysv/linux/not-errno.h b/sysdeps/unix/sysv/linux/not-errno.h index 106ba5c72e..b2f72cfb3d 100644 --- a/sysdeps/unix/sysv/linux/not-errno.h +++ b/sysdeps/unix/sysv/linux/not-errno.h @@ -16,6 +16,9 @@ License along with the GNU C Library; if not, see . */ +#include +#include + /* This function is used on maybe_enable_malloc_check (elf/dl-tunables.c) and to avoid having to build/use multiple versions if stack protection in enabled it is defined as inline. */ @@ -33,3 +36,14 @@ __access_noerrno (const char *pathname, int mode) return INTERNAL_SYSCALL_ERRNO (res, err); return 0; } + +static inline int +__kill_noerrno (pid_t pid, int sig) +{ + int res; + INTERNAL_SYSCALL_DECL (err); + res = INTERNAL_SYSCALL_CALL (kill, err, pid, sig); + if (INTERNAL_SYSCALL_ERROR_P (res, err)) + return INTERNAL_SYSCALL_ERRNO (res, err); + return 0; +} diff --git a/sysdeps/unix/sysv/linux/s390/system.c b/sysdeps/unix/sysv/linux/s390/system.c deleted file mode 100644 index d8ef461334..0000000000 --- a/sysdeps/unix/sysv/linux/s390/system.c +++ /dev/null @@ -1,29 +0,0 @@ -/* Copyright (C) 2003-2018 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 - . */ - -/* We have to and actually can handle cancelable system(). The big - problem: we have to kill the child process if necessary. To do - this a cleanup handler has to be registered and is has to be able - to find the PID of the child. The main problem is to reliable have - the PID when needed. It is not necessary for the parent thread to - return. It might still be in the kernel when the cancellation - request comes. Therefore we have to use the clone() calls ability - to have the kernel write the PID into the user-level variable. */ -#define FORK() \ - INLINE_SYSCALL (clone, 3, 0, CLONE_PARENT_SETTID | SIGCHLD, &pid) - -#include "../system.c" diff --git a/sysdeps/unix/sysv/linux/sparc/system.c b/sysdeps/unix/sysv/linux/sparc/system.c deleted file mode 100644 index 1f65c83399..0000000000 --- a/sysdeps/unix/sysv/linux/sparc/system.c +++ /dev/null @@ -1,29 +0,0 @@ -/* Copyright (C) 2003-2018 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 - . */ - -/* We have to and actually can handle cancelable system(). The big - problem: we have to kill the child process if necessary. To do - this a cleanup handler has to be registered and is has to be able - to find the PID of the child. The main problem is to reliable have - the PID when needed. It is not necessary for the parent thread to - return. It might still be in the kernel when the cancellation - request comes. Therefore we have to use the clone() calls ability - to have the kernel write the PID into the user-level variable. */ -#define FORK() \ - INLINE_CLONE_SYSCALL (CLONE_PARENT_SETTID | SIGCHLD, 0, &pid, NULL, NULL) - -#include "../system.c" diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c index 85239cedbf..6a8bd2ed2e 100644 --- a/sysdeps/unix/sysv/linux/spawni.c +++ b/sysdeps/unix/sysv/linux/spawni.c @@ -138,11 +138,11 @@ __spawni_child (void *arguments) for (int sig = 1; sig < _NSIG; ++sig) { if ((attr->__flags & POSIX_SPAWN_SETSIGDEF) - && sigismember (&attr->__sd, sig)) + && __sigismember (&attr->__sd, sig)) { sa.sa_handler = SIG_DFL; } - else if (sigismember (&hset, sig)) + else if (__sigismember (&hset, sig)) { if (__is_internal_signal (sig)) sa.sa_handler = SIG_IGN; diff --git a/sysdeps/unix/sysv/linux/system.c b/sysdeps/unix/sysv/linux/system.c deleted file mode 100644 index 7cc68a1528..0000000000 --- a/sysdeps/unix/sysv/linux/system.c +++ /dev/null @@ -1,76 +0,0 @@ -/* Copyright (C) 2002-2018 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 - . */ - -#include -#include -#include /* For the real memset prototype. */ -#include -#include -#include -#include - -/* We have to and actually can handle cancelable system(). The big - problem: we have to kill the child process if necessary. To do - this a cleanup handler has to be registered and is has to be able - to find the PID of the child. The main problem is to reliable have - the PID when needed. It is not necessary for the parent thread to - return. It might still be in the kernel when the cancellation - request comes. Therefore we have to use the clone() calls ability - to have the kernel write the PID into the user-level variable. */ -#ifndef FORK -# define FORK() \ - INLINE_SYSCALL (clone, 3, CLONE_PARENT_SETTID | SIGCHLD, 0, &pid) -#endif - -#ifdef _LIBC_REENTRANT -static void cancel_handler (void *arg); - -# define CLEANUP_HANDLER \ - __libc_cleanup_region_start (1, cancel_handler, &pid) - -# define CLEANUP_RESET \ - __libc_cleanup_region_end (0) -#endif - - -/* Linux has waitpid(), so override the generic unix version. */ -#include - - -#ifdef _LIBC_REENTRANT -/* The cancellation handler. */ -static void -cancel_handler (void *arg) -{ - pid_t child = *(pid_t *) arg; - - INTERNAL_SYSCALL_DECL (err); - INTERNAL_SYSCALL (kill, err, 2, child, SIGKILL); - - TEMP_FAILURE_RETRY (__waitpid (child, NULL, 0)); - - DO_LOCK (); - - if (SUB_REF () == 0) - { - (void) __sigaction (SIGQUIT, &quit, (struct sigaction *) NULL); - (void) __sigaction (SIGINT, &intr, (struct sigaction *) NULL); - } - - DO_UNLOCK (); -} -#endif