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 C2EC81F461 for ; Sun, 21 Jul 2019 12:15:56 +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= auQpFanwrNTnGabwjYRN49keYyVwUlSvz5GICV9fPwRaouOxuuyJN8WcwNIQALhL 0U40Lg1x6SQ2I2CKLegudo9LJ4eipA7pcKqal823R1AuFFXWAPPJWyaj5JWPY9k8 0MSfiTyBzPcIrRp8eSqry7rqe3mLA89eQSfBZSTiL3I= 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=PXO Sf+gQir041Vq4jiSyWQbOt8o=; b=fBEyi3zJkEE0OrGL9eYbcAf//BbSl5ehUa/ Sj1KiGxxg+Dx3BG5Bccul0u4Hnu1jVVndWYyetRvv8NtdOmNohSiG+KEQ01b6hCU fdV01FebnzlNHHDxYkoriiU6dzHz48fzTQ1+qcfvOjLrPs9ct0/oakAhIcrRelbM 8nhgGh4Q= Received: (qmail 122217 invoked by alias); 21 Jul 2019 12:15:53 -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 122131 invoked by uid 89); 21 Jul 2019 12:15:44 -0000 Authentication-Results: sourceware.org; auth=none X-HELO: out01.mta.xmission.com From: ebiederm@xmission.com (Eric W. Biederman) To: Arnd Bergmann Cc: Rich Felker , Alistair Francis , GNU C Library , Adhemerval Zanella , Florian Weimer , Palmer Dabbelt , macro@wdc.com, Zong Li , Alistair Francis , Andrew Morton , Linus Torvalds , Al Viro , Christian Brauner , "H. Peter Anvin" References: <2d3359a195633b85e01f83bf536330d72f7bc8aa.1563321715.git.alistair.francis@wdc.com> <20190721040310.GA3283@brightrain.aerifal.cx> Date: Sun, 21 Jul 2019 07:15:24 -0500 In-Reply-To: (Arnd Bergmann's message of "Sun, 21 Jul 2019 09:57:03 +0200") Message-ID: <87o91nbn37.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=1hpAkT-0008OR-2B;;;mid=<87o91nbn37.fsf@xmission.com>;;;hst=in02.mta.xmission.com;;;ip=68.227.160.95;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX19+gmYDhOcxkgI0Ubck2SANc8RjrFhjtCY= 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) Arnd Bergmann writes: > On Sun, Jul 21, 2019 at 6:03 AM Rich Felker wrote: >> On Tue, Jul 16, 2019 at 05:08:48PM -0700, Alistair Francis wrote: >> > #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. > > Interesting, I don't think anyone ever considered this race, as waitid() > was clearly always /intended/ as a superset of wait(), waitpid(), wait3() > and wait4(), as Roland McGrath described in the initial creation > of the waitid() syscall, see > https://lore.kernel.org/lkml/200408152303.i7FN3Vc3021030@magilla.sf.frob.com/ > It is definitley a problem as setpid and setpgrp and setsid are all required to be signal safe, and this waitpid emulation is clearly not. > I originally planned to have a new waitid_time64() syscall to replace > them in the y2038 series, but that got deferred, so at the moment > the older 32-bit architectures don't have any changes for wait*(), and > riscv32 just removed wait4() but kept waitid(). > > It would be easy enough to restore wait4() for riscv32, or we could > decide to extend waitid() in a backward-compatible way to close > this race, e.g. by allowing P_PGID with pid=0 (this is currently > -EINVAL), by adding a new P_SAME_PGID macro to cover that > case, or by creating a new waitid replacement that does other things > as well (nanosecond ru_*time, pidfd, ...). > > Adding a few more kernel developers that may have an opinion on this. In a different reply it was mentioned that wait4 has had an issue since 2008, where it does not track changes in the current processes pgrp. I would like to get resolution on if that is a real problem for anyone or if it is a something theoretical found on code review, before we deal with this issue. Because tracking pgrp changes will require a deeper code change. My gut feel says either fix waitid (with P_SAME_PGID) or clearly document why the kernel's waitid is not a replacement for wait4, and add wait4 to riscv. Would adding wait4 to riscv allow us to get rid of the rusage parameter of the kernel's waitid? I don't oppose a new syscall to take advantage of pidfd but I don't think there is any need to tie the two fixes together so I would rather keep them separate. Just so we don't rush through fixing one to deal with the other. Eric