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 Netto X-Patchwork-Id: 825015 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=sourceware.org (client-ip=209.132.180.131; helo=sourceware.org; envelope-from=libc-alpha-return-85744-incoming=patchwork.ozlabs.org@sourceware.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="FR/81uK8"; dkim-atps=neutral Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3yCflc2ljwz9s7p for ; Fri, 13 Oct 2017 05:39:40 +1100 (AEDT) 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=Y3cdEE2lAi2QUZGnZqraHR2nXhfT21jTrWTzKc7TDYWwByPkZmvkk M3+t+xDtlThX5+P5s8xtTxwWaabvabMwymOznzzbL6tpvoabEfXhkUYMAmY187Ly n5kqSIxv4nzjTY4A3hmrb/Ry1QeTdnXG7fU1NkQz406zDln0zVz2Sk= 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=FR/81uK8WllTqdsrvfcQFfC+GwAu P2VV3mh1iAp8mK5+juCNDhZxqNFMELgPkwtO238d1Hpn/+EHCbIzYZ8T55R/MTQs rYL4Ds7c/5F5ULoyNTha0BapyuYnk11waeDUg+1CSuZCLeCIto5rZMExv+LYFWFo ANQAf8oqTWga4bg= 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(-) 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;