From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ams.source.kernel.org (ams.source.kernel.org [IPv6:2604:1380:4601:e00::1]) by sourceware.org (Postfix) with ESMTPS id 4D6033835697 for ; Mon, 27 Jun 2022 15:03:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4D6033835697 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 47160B81840; Mon, 27 Jun 2022 15:03:36 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 03156C3411D; Mon, 27 Jun 2022 15:03:33 +0000 (UTC) Date: Mon, 27 Jun 2022 17:03:30 +0200 From: Christian Brauner To: Florian Weimer Cc: Mark Wielaard , libc-alpha@sourceware.org Subject: Re: [PATCH 3/4] tst-pidfd.c: Test is UNSUPPORTED without PTRACE_MODE_ATTACH_REALCREDS Message-ID: <20220627150330.frmn2vump7h4axli@wittgenstein> References: <20220626205915.33201-1-mark@klomp.org> <20220626205915.33201-4-mark@klomp.org> <87h747nmud.fsf@mid.deneb.enyo.de> <874k06cq9t.fsf@oldenburg.str.redhat.com> <20220627115141.s4zjaac7ixceq7rs@wittgenstein> <87sfnq9nhj.fsf@oldenburg.str.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <87sfnq9nhj.fsf@oldenburg.str.redhat.com> X-Spam-Status: No, score=-11.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Jun 2022 15:03:47 -0000 On Mon, Jun 27, 2022 at 04:42:32PM +0200, Florian Weimer wrote: > * Christian Brauner: > > > On Mon, Jun 27, 2022 at 01:14:06PM +0200, Florian Weimer via Libc-alpha wrote: > >> * Mark Wielaard: > >> > >> > Hi Florian, > >> > > >> > On Sun, 2022-06-26 at 23:20 +0200, Florian Weimer wrote: > >> >> * Mark Wielaard: > >> >> > >> >> > pidfd_getfd will fail with errno EPERM if the calling process did > >> >> > not > >> >> > have PTRACE_MODE_ATTACH_REALCREDS permissions. Use FAIL_UNSUPPORTED > >> >> > in that case. > >> >> > --- > >> >> > sysdeps/unix/sysv/linux/tst-pidfd.c | 6 ++++-- > >> >> > 1 file changed, 4 insertions(+), 2 deletions(-) > >> >> > > >> >> > diff --git a/sysdeps/unix/sysv/linux/tst-pidfd.c > >> >> > b/sysdeps/unix/sysv/linux/tst-pidfd.c > >> >> > index d93b6faa6f..28349b2f91 100644 > >> >> > --- a/sysdeps/unix/sysv/linux/tst-pidfd.c > >> >> > +++ b/sysdeps/unix/sysv/linux/tst-pidfd.c > >> >> > @@ -95,8 +95,10 @@ do_test (void) > >> >> > kernel has pidfd support that we can test. */ > >> >> > int r = pidfd_getfd (0, 0, 1); > >> >> > TEST_VERIFY_EXIT (r == -1); > >> >> > - if (errno == ENOSYS) > >> >> > - FAIL_UNSUPPORTED ("kernel does not support pidfd_getfd, > >> >> > skipping test"); > >> >> > + if (errno == ENOSYS || errno == EPERM) > >> >> > + FAIL_UNSUPPORTED ("kernel does not support pidfd_getfd," > >> >> > + " or we don't have > >> >> > PTRACE_MODE_ATTACH_REALCREDS" > >> >> > + " permissions, skipping test"); > >> >> > } > >> >> > >> >> This also hints towards a broken seccomp filter. > >> > > >> > pidfd_getfd is mentioned (and allowed) by the seccomp filter, but the > >> > syscall also needs the process to have PTRACE_MODE_ATTACH_REALCREDS > >> > (which is really PTRACE_MODE_ATTACH | PTRACE_MODE_REALCREDS). Which it > >> > doesn't have. If the process doesn't then pidfd_getfd is defined as > >> > failing and setting errno to EPERM. > >> > >> But what does it mean for a process to have PTRACE_MODE_REALCREDS? > > > > #define PTRACE_MODE_ATTACH_REALCREDS (PTRACE_MODE_ATTACH | PTRACE_MODE_REALCREDS) > > > > PTRACE_MODE_ATTACH_REALCREDS means > > * PTRACE_MODE_REALCREDS which means cred->{g,u}id of the task are used > > for permission checking: > > > > So it's a bit nasty here but roughly if the ptracer's real {g,u}id match > > the target task's (ptracee's) effective, save, and real [g,u]id then > > you'passed the first stage of permission checking. > > > > If ptracer's [g,u]id aren't matching the ptracee's effective, saved, and > > real [g,u]id the ptracer needs CAP_SYS_PTRACE in the ptracee's userns. > > That will also get you past the first state of permission checking. > > > > If both don't apply the request is denied with -EPERM. > > I think this doesn't apply to the > > pidfd_getfd (0, 0, 1) > > call because the arguments are invalid and we never get to the kernel > permission check. I thought this might be a self-get call, but that's > clearly not the case (0 does not refer to a process file descriptor). > > So the comment should not mention PTRACE_MODE_ATTACH_REALCREDS at all > because it's really not relevant to the situation. This is just another > case of broken seccomp filters. So the usual discussion whether we > should paper over that or not in the test suite applies here as well. Yeah, that absolutely is a seccomp filter issue. Your example should give you EINVAL. Btw, I think you want to use pidfd_get(-EBADF, 0, 0). That isn't as error prone as passing 1 in the flags argument. And you should be able to always expect -EBADF for this. I consider this to be API in all honesty. IOW, if additional arguments are sane and you pass an invalid fd to any pidfd call then they should give you -EBADF back. So based on this assumption you could say that if you don't get -ENOSYS and don't get -EBADF back then this must be seccomp or something else... Christian