From patchwork Thu Oct 12 18:39:22 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adhemerval Zanella X-Patchwork-Id: 115664 Delivered-To: patch@linaro.org Received: by 10.80.163.170 with SMTP id s39csp2043664edb; Thu, 12 Oct 2017 11:39:43 -0700 (PDT) X-Received: by 10.98.15.77 with SMTP id x74mr2949876pfi.17.1507833583510; Thu, 12 Oct 2017 11:39:43 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1507833583; cv=none; d=google.com; s=arc-20160816; b=sOZjKMJaU60AC+s+TjaApqMA08AWJq50548bxy/ssbXlje+EidlEySCBKxy+9OATai 00V8n3AClqdbjqK1/z3PB+BndDN9tmB9bkb3sOjKVlQrQqFwBCVFrjrOKVde/zbemD/G +YIh8JMHhnx4cQPlgfoLrUy2MCzxgP6m8zstOTfsBaSWTLWyAYOKljiJM82fK208dKkR zVaQHE672YBfsCC/JxYrxztVXNDeElSvK4PdCL3N4CVQ8enYKuevX4JUZLQIUmAmUsax lYBjG1SBQUFkLedngH4OzzEhgIq4qYHfawTKYRcrhwbL5PbwhxKN6zfEjca+RpUoXe8M qqTA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=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=gPA4r42B8G2I733feFa78YjgtvIWCdyaQCuMy+CYtl4=; b=BASz8gtcyoy/ldF8N1uogC6Esx1S7YrGL/Gax2eeQtUyrRMbs/Y94wytlIlVEW506T Ko7mlVyhmWzNyd6TcOdgru4KUj/e4jw4dMCwifa5gi9z2iPxgxMb6F0Zn+tjn1HZn1w/ ZenqIIFCBETHYOuBf3ptmlj9oCTL2wo6kUvwMqA+mfAzVpil1Stpdg9Vhn+ZfjpZ5vLm dotjmSColNHZQjqRXJrTnyv42jQ+JZsWKu6QyO20l9kr4VBX1Aj+Crjqdmey5MqMGUU4 0H7OhsspoBkYYUWwIQrgLGIHIAd4KxCEXwxGz5rV2P7gEto5VFrHW/EYt0GJRV/TRRBc OvWQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@sourceware.org header.s=default header.b=wzF2aoSl; spf=pass (google.com: domain of libc-alpha-return-85744-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-85744-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 e85si12845409pfb.56.2017.10.12.11.39.43 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Oct 2017 11:39:43 -0700 (PDT) Received-SPF: pass (google.com: domain of libc-alpha-return-85744-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=wzF2aoSl; spf=pass (google.com: domain of libc-alpha-return-85744-patch=linaro.org@sourceware.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=libc-alpha-return-85744-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; q=dns; s= default; b=LbTV0R0P+76yV3QETt0Z1TCBbojdmWfH86KgCjHeOrOUtHJ8oOG9s PbjDVAMqYElAvgt5oJVTyzJD5WfKb4VGWgIMFUURy0nJhu7mddb+KiOAb3NMh6K6 fNACCRybnuYKhGzAh7Ao+uHxsBPuv6l7QQKsBFnjfy8MxNW/BLPZ4k= 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; s=default; bh=q18/tP3LUVm5/++Fot2x3HHjjgI=; b=wzF2aoSlJqAxadgeEjcTuPegJEep cA6ejBla7V+RrhSmTK0iOZDhnU+d+E4+hMd4L5idn5Y6u4kEW51kBXZ2Wg9WW7LH Me67Nw9njV8F/IJKJgYOgQn26bONOcRcu7Pb0t1jcAlMhFdCkiUwY+4oUfPxiUrz cBjjMpHiHgj1AVg= Received: (qmail 72461 invoked by alias); 12 Oct 2017 18:39:33 -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 71039 invoked by uid 89); 12 Oct 2017 18:39:32 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=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.2 spammy= X-HELO: mail-qk0-f180.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; bh=gPA4r42B8G2I733feFa78YjgtvIWCdyaQCuMy+CYtl4=; b=S0Zu+FEzcNW0tAjWwdBf7bt3+3nlbXrWHfhLzNM4vb5bom/YdJL6ykr7AU9m6PkOWa uJ80DjXPllRDGaVQ2hN5lDtizd3cKR55T3FFeX6L+selxTVnNc4WsW96zru+iiAmyLZD d6Q2uB6wo0+3OTqYXcvlv3MqqgNSVG3U6929zTqZrstVI6YBwisLRfjVEVZLxFNvpcom T5stBif576gLMi13+p5lQpKBwZnJoJJ/7DW1+DKgi6dzkc4A6XkANdMqpva+Wq8iT60I 5NJaQjx0oTbw3pa/LSDLZOnDZPN0GLCo4MIEsHVesJu1mF/Uw4qBEar8ILaWDgUqS1QA 6Y1Q== X-Gm-Message-State: AMCzsaXO4m64x7rGvYEqHOaYFnR4W8D/JWZTxxbcSDGEBHwPSHLIRI/V sBZiwJ9z8GH2tOjMRNeM+cyQkYkI3TA= X-Google-Smtp-Source: ABhQp+ScN6lomy3BiHo19+yDAG743h73KDRzuBcYU03gUQbKyHIRKNA2kyEGT81MK0Qllzdj2s8GNQ== X-Received: by 10.55.72.75 with SMTP id v72mr1873728qka.92.1507833568543; Thu, 12 Oct 2017 11:39:28 -0700 (PDT) From: Adhemerval Zanella To: libc-alpha@sourceware.org Subject: [PATCH] posix: Fix improper assert in Linux posix_spawn (BZ#22273) Date: Thu, 12 Oct 2017 15:39:22 -0300 Message-Id: <1507833562-6945-1-git-send-email-adhemerval.zanella@linaro.org> As noted by Florian Weimer, current Linux posix_spawn implementation can trigger an assert if the auxiliary process is terminated before actually setting the err member: 340 /* Child must set args.err to something non-negative - we rely on 341 the parent and child sharing VM. */ 342 args.err = -1; [...] 362 new_pid = CLONE (__spawni_child, STACK (stack, stack_size), stack_size, 363 CLONE_VM | CLONE_VFORK | SIGCHLD, &args); 364 365 if (new_pid > 0) 366 { 367 ec = args.err; 368 assert (ec >= 0); Another possible issue is killing the child between setting the err and actually calling execve. In this case the process will not ran, but posix_spawn also will not report any error: 269 270 args->err = 0; 271 args->exec (args->file, args->argv, args->envp); As suggested by Andreas Schwab, this patch removes the faulty assert and also handles any signal that happens before fork and execve as the spawn was successful (and thus relaying the handling to the caller to figure this out). Different than Florian, I can not see why using atomics to set err would help here, essentially the code runs sequentially (due CLONE_VFORK) and I think it would not be legal the compiler evaluate ec without checking for new_pid result (thus there is no need to compiler barrier). Checked on x86_64-linux-gnu. [BZ #22273] * sysdeps/unix/sysv/linux/spawni.c (__spawnix): Handle the case where the auxiliary process is terminated by a signal before calling _exit or execve. --- ChangeLog | 6 ++++++ sysdeps/unix/sysv/linux/spawni.c | 12 ++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) -- 2.7.4 diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c index dea1650..d6acc1e 100644 --- a/sysdeps/unix/sysv/linux/spawni.c +++ b/sysdeps/unix/sysv/linux/spawni.c @@ -365,9 +365,17 @@ __spawnix (pid_t * pid, const char *file, if (new_pid > 0) { ec = args.err; - assert (ec >= 0); if (ec != 0) - __waitpid (new_pid, NULL, 0); + { + /* It handles the unlikely case where the auxiliary process is + terminated by a signal before calling _exit or execve. For + this case the signal is relayed to the called, as the spawn + was successful. */ + int status; + __waitpid (new_pid, &status, WNOHANG); + if (WIFSIGNALED (status)) + ec = 0; + } } else ec = -new_pid;