From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-4.0 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_EF,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 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 dcvr.yhbt.net (Postfix) with ESMTPS id D50C51F461 for ; Sun, 21 Jul 2019 11:59:24 +0000 (UTC) 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:references:date:in-reply-to :message-id:mime-version:content-type:subject; q=dns; s=default; b= PqMHK3s1BCL1Znz3jADJ6D6cOdfyfROQhxuIBF1jnWk3UpcC+PUjpH+XSHUJl/UG 3regrS2AQXOwwtxEbZViw2h9oDjMhhsuiDzAJJcuQ6Sd7m2nD/tlZVsiEGLPuEJ/ KBMlmak/3sKUuk4df42fccPFLJZN+EAdxpo9yvDl8NQ= 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:references:date:in-reply-to :message-id:mime-version:content-type:subject; s=default; bh=Pl4 SgsIlAYvTzNlpb98Q1BU0+CA=; b=AUCa6Nz4KwUNPWJ+gQt/XDocOYREc7UP5rL J0xMsEpbFrhqbKZtF3ZYCqWQ9/JNI3ADvuzvm9hkNmo1MHYdZt+ccxIQWu6QC6Rg 6B8KICkz6BN4YNeJ5ethPyw08uNUWCUf4r74waNJmOfmd/WGv6I97Ow7eZA8AdRa eB/3dwrE= Received: (qmail 112945 invoked by alias); 21 Jul 2019 11:59:21 -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 Received: (qmail 112937 invoked by uid 89); 21 Jul 2019 11:59:21 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: out02.mta.xmission.com From: ebiederm@xmission.com (Eric W. Biederman) To: Rich Felker Cc: Alistair Francis , libc-alpha@sourceware.org, arnd@arndb.de, adhemerval.zanella@linaro.org, fweimer@redhat.com, palmer@sifive.com, macro@wdc.com, zongbox@gmail.com, alistair23@gmail.com References: <2d3359a195633b85e01f83bf536330d72f7bc8aa.1563321715.git.alistair.francis@wdc.com> <20190721040310.GA3283@brightrain.aerifal.cx> <20190721042032.GA3423@brightrain.aerifal.cx> Date: Sun, 21 Jul 2019 06:59:09 -0500 In-Reply-To: <20190721042032.GA3423@brightrain.aerifal.cx> (Rich Felker's message of "Sun, 21 Jul 2019 00:20:32 -0400") Message-ID: <87ftmzd2eq.fsf@xmission.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1hpAUl-0006tO-9r;;;mid=<87ftmzd2eq.fsf@xmission.com>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1+OwIQI9+yzFZgWc/L5oyMR03PVUiVSrrQ= X-SA-Exim-Connect-IP: 68.227.160.95 X-SA-Exim-Mail-From: ebiederm@xmission.com Subject: Re: [RFC v3 03/23] sysdeps/wait: Use waitid if avaliable X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Rich Felker writes: > On Sun, Jul 21, 2019 at 12:03:10AM -0400, Rich Felker wrote: >> On Tue, Jul 16, 2019 at 05:08:48PM -0700, Alistair Francis wrote: >> > If the waitid syscall is avaliable let's use that as waitpid >> > and wait4 aren't always avaliable (they aren't avaliable on RV32). >> > >> > Unfortunately waitid is substantially differnt to waitpid and wait4, so >> > the conversion ends up being complex. >> > >> > Signed-off-by: Alistair Francis >> > --- >> > ChangeLog | 3 ++ >> > sysdeps/unix/sysv/linux/wait.c | 39 ++++++++++++++++-- >> > sysdeps/unix/sysv/linux/waitpid.c | 46 ++++++++++++++++++++++ >> > sysdeps/unix/sysv/linux/waitpid_nocancel.c | 45 +++++++++++++++++++++ >> > 4 files changed, 130 insertions(+), 3 deletions(-) >> > [...] >> > >> > weak_alias (__libc_wait, __wait) >> > diff --git a/sysdeps/unix/sysv/linux/waitpid.c b/sysdeps/unix/sysv/linux/waitpid.c >> > index f0897574c0..7d4e0bb77d 100644 >> > --- a/sysdeps/unix/sysv/linux/waitpid.c >> > +++ b/sysdeps/unix/sysv/linux/waitpid.c >> > @@ -20,12 +20,58 @@ >> > #include >> > #include >> > #include >> > +#include >> > >> > __pid_t >> > __waitpid (__pid_t pid, int *stat_loc, int options) >> > { >> > #ifdef __NR_waitpid >> > return SYSCALL_CANCEL (waitpid, pid, stat_loc, options); >> > +#elif defined(__NR_waitid) >> > + __pid_t ret; >> > + idtype_t idtype = P_PID; >> > + siginfo_t infop; >> > + >> > + if (pid < -1) { >> > + idtype = P_PGID; >> > + pid *= -1; >> > + } else if (pid == -1) { >> > + idtype = P_ALL; >> > + } else if (pid == 0) { >> > + idtype = P_PGID; >> > + pid = getpgrp(); >> > + } >> > + >> > + options |= WEXITED; >> > + >> > + ret = SYSCALL_CANCEL (waitid, idtype, pid, &infop, options, NULL); >> >> This emulation has a fundamental race condition. Between getpgrp and >> waitid, a signal handler may perform setpgrp, setsid, and/or fork in >> ways that cause the wrong pgid to be passed to the waitid syscall. >> There is no way around this because you cannot block signals for the >> interval, since signals must be able to interrupt the waitid syscall. >> >> Unless there's some trick I'm missing here, the kernel folks' removal >> of the wait4 syscall is just a bug in the kernel that they need to >> fix. It also makes it impossible to implement the wait4 function, >> since there's no way to get rusage for the exited process. > > Reportedly (via Stefan O'Rear just now) there was a similar kernel bug > introduced in 161550d74c07303ffa6187ba776f62df5a906a21 that makes > wait4 fail to honor pgrp changes that happen while already in the > syscall (e.g. performed on the caller by another thread or even > another process). I could not find the report from Stefan O'Rear. Does that result in actual problems for programs or is this a theoretical race noticed upon code review? > But the race condition here in userspace is even > more egregious I think, since it violates the contract in a case where > there is a clear observable order between the pgrp change and the > blocking wait -- for instance, the signal handler could change pgrp > of itself and a child process, and then whether or not the signal > handler had executed before the waitpid, the waitpid should catch the > child's exit. But with the above race, it fails to. Definitely a bigger issue. I believe posix requires waitpid is required to be signal safe. Eric