From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by sourceware.org (Postfix) with ESMTPS id 0AA1D38327FB for ; Mon, 27 Jun 2022 15:08:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0AA1D38327FB 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 CF34BB8182F; Mon, 27 Jun 2022 15:08:36 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 5AB4FC3411D; Mon, 27 Jun 2022 15:08:34 +0000 (UTC) Date: Mon, 27 Jun 2022 17:08:26 +0200 From: Christian Brauner To: Adhemerval Zanella Cc: Florian Weimer , Mark Wielaard , libc-alpha@sourceware.org Subject: Re: [PATCH 3/4] tst-pidfd.c: Test is UNSUPPORTED without PTRACE_MODE_ATTACH_REALCREDS Message-ID: <20220627150826.f4ezg3t4k7sy7kih@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 Content-Transfer-Encoding: 8bit In-Reply-To: X-Spam-Status: No, score=-12.0 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:08:39 -0000 On Mon, Jun 27, 2022 at 11:57:02AM -0300, Adhemerval Zanella wrote: > > > > On 27 Jun 2022, at 11:42, Florian Weimer via Libc-alpha 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) > > The init has PPID 0, which leads to the idea pid argument 0 is valid. > So fdget won’t fail and EPERM will be returned by pidfd_pid. Maybe > use pidfd_getfd (TYPE_MAXIMUM (pid_t), 0, 1) instead to trigger a > EBADF. No, I don't think that's the case. EPERM can only ever be returned from ptrace_may_access() in pidfd_getfd() or if there's something like seccomp in the mix. You should see EINVAL, I think. SYSCALL_DEFINE3(pidfd_getfd, int, pidfd, int, fd, unsigned int, flags) 0 0 1 { struct pid *pid; struct fd f; int ret; /* flags is currently unused - make sure it's unset */ if (flags) // that's 1 so you should see -EINVAL return -EINVAL; f = fdget(pidfd); // That'll get you stdin most likely. if (!f.file) return -EBADF; pid = pidfd_pid(f.file); // That would give you -EBADF see [1] if (IS_ERR(pid)) ret = PTR_ERR(pid); else ret = pidfd_getfd(pid, fd); fdput(f); return ret; } [1]: struct pid *pidfd_pid(const struct file *file) { if (file->f_op == &pidfd_fops) // stdin is no pidfd and thus doesn't have pidfd_fops return file->private_data; return ERR_PTR(-EBADF); // that's what you'd see for stdin }