From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id BD5DEC54E4A for ; Fri, 8 Mar 2024 10:36:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 18DED6B0366; Fri, 8 Mar 2024 05:36:09 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0F21F6B0368; Fri, 8 Mar 2024 05:36:09 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EF8FD6B0369; Fri, 8 Mar 2024 05:36:08 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id D836F6B0366 for ; Fri, 8 Mar 2024 05:36:08 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id B037D8157C for ; Fri, 8 Mar 2024 10:36:08 +0000 (UTC) X-FDA: 81873516816.23.9B52531 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf04.hostedemail.com (Postfix) with ESMTP id 0854D40019 for ; Fri, 8 Mar 2024 10:36:06 +0000 (UTC) Authentication-Results: imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=LWV6sPna; spf=pass (imf04.hostedemail.com: domain of brauner@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=brauner@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1709894167; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=nXaPoCalyCMACEJzBa1YtIWrJH3TgytLZ4ly0ROawZo=; b=ReAp+BA3vsDwmLGHJoxjNslphMPCzIqJD1fYpeH2vhHX1qCy3A3spJ0zVQG6LlMsHilgqh cfMl80D7kxN0aryrjG2BhZDrmRPmYoTUC3DQ6uEtPmzHH6xoSnwhrc/bnJcJTpLZvrUGkn noiuQL8U7O8bgkCDR9l+pbkiBARTRWc= ARC-Authentication-Results: i=1; imf04.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=LWV6sPna; spf=pass (imf04.hostedemail.com: domain of brauner@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=brauner@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1709894167; a=rsa-sha256; cv=none; b=wZ98wB+M8i6AoW5nGOK6zr4bWzH5fD3MpRbPgDmPeGoufI1KZganKufT0Q5HHI5zwiW9cc JvnmMFFdXRQd2n2nlsftK0rbnjZHMb9/5cynGKZbrPS0n2RQVVH6GZ2aBqzOxJmOgDJMxq 76s2K4RY5iFpOTJhHp+RjI3Ok4xUi9U= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id DE38761DEA; Fri, 8 Mar 2024 10:36:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 18E63C433F1; Fri, 8 Mar 2024 10:36:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1709894165; bh=QkVpUYeV3CtNb7O8+udH/aZB4OwfinzkvvEEuTNnMFg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=LWV6sPnaWP9YplC1t/94tZ0sy+b7KIBGbPH7SZg7w22R3lrl43cvJu+W0zOigCh/P I92ydODnQMf1mYrgkxTwXgQenW748JseAHzrawivMp4Vjm51Da8xToKCz3CWZA3lcc 7C6FhA6xiosVjYQIjGghK3t8ZafGyl4Zo+0wPyzavJr/bZQAW16L62A2I2YFGLyB10 k6XsMFxdvsG2NGZSMHqG38TjUxDw1NKxHpsBaJJViirMn6+z+CKhzDhUY7B03lfl+9 1bFiRYxoOOI3fLPcIrNHQuGmV6J6n52Yu7ECxmB8CNj7AyKhc7LnIoA2wDvxUk0KtU 0fyy0HRt/jeGw== Date: Fri, 8 Mar 2024 11:35:59 +0100 From: Christian Brauner To: Alexei Starovoitov Cc: Matt Bobrowski , bpf , Alexei Starovoitov , Andrii Nakryiko , KP Singh , Jann Horn , Jiri Olsa , Daniel Borkmann , Linus Torvalds , Linux-Fsdevel , Andrew Morton , linux-mm , LSM List Subject: Re: [PATCH v2 bpf-next 0/9] add new acquire/release BPF kfuncs Message-ID: <20240308-kleben-eindecken-73c993fb3ebd@brauner> References: <20240306-flach-tragbar-b2b3c531bf0d@brauner> <20240306-sandgrube-flora-a61409c2f10c@brauner> <20240307-phosphor-entnahmen-8ef28b782abf@brauner> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: 0854D40019 X-Rspam-User: X-Stat-Signature: oqbrk4he3zqemazeuf3gudyj9akj158p X-Rspamd-Server: rspam01 X-HE-Tag: 1709894166-372863 X-HE-Meta: U2FsdGVkX19RBazmyrWJgWJbGSpTCvFe8OJPjGjtiorWiC4FXbxZS2G3xK5AATVvlLhTeeGuezFgX0SVmy14jUj2onE5qNCGtm2CcpXnLH1JBTkglw0Ap2vQjFgV7Q4TtrhNdzpo7PkuXnSc2dFirH6NW/+YFKt7oWpsoTLQV83w793+WiIMJiBg++kC8y1bwH1UMUGutMh+O3qinfNGGytD8tOgz6H/xTQF2yvCdMaQxLtUIfVvDjmbiEym1IooNsnax4Wi8rR2aR/70icRh+4nTq4hIi06uVW1fKBm4tRaRfp9asrQR+3WW70Lo27cqHmoHOMS2nCBxyiiLKk7pIlia57LQzTzTNDPMXw23M/Y99YV2TnvTuobLsqBeGmxD/nh5B04x5cbetqMmhbPkLRvUCMqJvarY3zficZeceXyWEm/o6EOip0i+aT+nxG/Zi4WnyY+wYMQG5/KirsT6TyukaeM4MK97vJeaYQmdrHaIfS/bp7jU1CGKFdvTARUhfHC7GxnzIoXU+cXOmpqJ+pghb/eo/4kUKI+LwFAQrCqiOcU9dqLOHTdYNq2LfJkEYorwtAwwslPWZfdt58s6HxRwbZUce5gWSlc3LAlzHh0qdmSyqtBkrSxPuP/jxy+eSdTcDdGC1ygSBR17X18rcdJhGKMvKXzdgTrg+98E+BB+rSSH0R4RWyt0FfUDZbpqpQbm778zj+k//YFlAAi1vkl8/UOTJLfDtCqCh6uMMBfNqh274+N8i/m7cL/AgHBv9jFCzARDCV03au+UwA6cBZQWYuCfOIMOSkPTk7pVnrFRcZun5i3Q5Powca9uTG9LilBWPU28/R060ZuzyZpVizazp5EsxcSBCM9PY5JTKvEnVNsexPsl0lxABZACdTz8KA+QW4Oh9iM6LXooTCVfU5GQjE+M7kmM7A1aZS6XDMu/yZeewBD/zGBaMSvakdBm6ftvQGcOCImZ55+O9U jMA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Thu, Mar 07, 2024 at 07:11:22PM -0800, Alexei Starovoitov wrote: > On Thu, Mar 7, 2024 at 1:55 AM Christian Brauner wrote: > > > > > > > > > > So, looking at this series you're now asking us to expose: > > > > > > > > (1) mmgrab() > > > > (2) mmput() > > > > (3) fput() > > > > (5) get_mm_exe_file() > > > > (4) get_task_exe_file() > > > > (7) get_task_fs_pwd() > > > > (6) get_task_fs_root() > > > > (8) path_get() > > > > (9) path_put() > > > > > > > > in one go and the justification in all patches amounts to "This is > > > > common in some BPF LSM programs". > > > > > > > > So, broken stuff got exposed to users or at least a broken BPF LSM > > > > program was written somewhere out there that is susceptible to UAFs > > > > becauase you didn't restrict bpf_d_path() to trusted pointer arguments. > > > > So you're now scrambling to fix this by asking for a bunch of low-level > > > > exports. > > > > > > > > What is the guarantee that you don't end up writing another BPF LSM that > > > > abuses these exports in a way that causes even more issues and then > > > > someone else comes back asking for the next round of bpf funcs to be > > > > exposed to fix it. > > > > > > There is no guarantee. > > > We made a safety mistake with bpf_d_path() though > > > we restricted it very tight. And that UAF is tricky. > > > I'm still amazed how Jann managed to find it. > > > We all make mistakes. > > > It's not the first one and not going to be the last. > > > > > > What Matt is doing is an honest effort to fix it > > > in the upstream kernel for all bpf users to benefit. > > > He could have done it with a kernel module. > > > The above "low level" helpers are all either static inline > > > in .h or they call EXPORT_SYMBOL[_GPL] or simply inc/dec refcnt. > > > > > > One can implement such kfuncs in an out of tree kernel module > > > and be done with it, but in the bpf community we encourage > > > everyone to upstream their work. > > > > > > So kudos to Matt for working on these patches. > > > > > > His bpf-lsm use case is not special. > > > It just needs a safe way to call d_path. > > > > > > +SEC("lsm.s/file_open") > > > +__failure __msg("R1 must be referenced or trusted") > > > +int BPF_PROG(path_d_path_kfunc_untrusted_from_current) > > > +{ > > > + struct path *pwd; > > > + struct task_struct *current; > > > + > > > + current = bpf_get_current_task_btf(); > > > + /* Walking a trusted pointer returned from bpf_get_current_task_btf() > > > + * yields and untrusted pointer. */ > > > + pwd = ¤t->fs->pwd; > > > + bpf_path_d_path(pwd, buf, sizeof(buf)); > > > + return 0; > > > +} > > > > > > This test checks that such an access pattern is unsafe and > > > the verifier will catch it. > > > > > > To make it safe one needs to do: > > > > > > current = bpf_get_current_task_btf(); > > > pwd = bpf_get_task_fs_pwd(current); > > > if (!pwd) // error path > > > bpf_path_d_path(pwd, ...); > > > bpf_put_path(pwd); > > > > > > these are the kfuncs from patch 6. > > > > > > And notice that they have KF_ACQUIRE and KF_RELEASE flags. > > > > > > They tell the verifier to recognize that bpf_get_task_fs_pwd() > > > kfunc acquires 'struct path *'. > > > Meaning that bpf prog cannot just return without releasing it. > > > > > > The bpf prog cannot use-after-free that 'pwd' either > > > after it was released by bpf_put_path(pwd). > > > > > > The verifier static analysis catches such UAF-s. > > > It didn't catch Jann's UAF earlier, because we didn't have > > > these kfuncs! Hence the fix is to add such kfuncs with > > > acquire/release semantics. > > > > > > > The difference between a regular LSM asking about this and a BPF LSM > > > > program is that we can see in the hook implementation what the LSM > > > > intends to do with this and we can judge whether that's safe or not. > > > > > > See above example. > > > The verifier is doing a much better job than humans when it comes > > > to safety. > > > > > > > Here you're asking us to do this blindfolded. > > > > > > If you don't trust the verifier to enforce safety, > > > you shouldn't trust Rust compiler to generate safe code either. > > > > > > In another reply you've compared kfuncs to EXPORT_SYMBOL_GPL. > > > Such analogy is correct to some extent, > > > but unlike exported symbols kfuncs are restricted to particular > > > program types. They don't accept arbitrary pointers, > > > and reference count is enforced as well. > > > That's a pretty big difference vs EXPORT_SYMBOL. > > > > There's one fundamental question here that we'll need an official answer to: > > > > Is it ok for an out-of-tree BPF LSM program, that nobody has ever seen > > to request access to various helpers in the kernel? > > obviously not. > > > Because fundamentally this is what this patchset is asking to be done. > > Pardon ? > > > If the ZFS out-of-tree kernel module were to send us a similar patch > > series asking us for a list of 9 functions that they'd like us to export > > what would the answer to that be? > > This patch set doesn't request any new EXPORT_SYMBOL. In order for that out-of-tree BPF LSM program to get similar guarantees than an equivalent kernel module we need to export all nine functions for BPF. Included in that set are functions that aren't currently even exported to modules. These exports are specifically for an out-of-tree BPF LSM program that is not accessible to the public. The question in the other mail stands. > First of all, there is no such thing as get_task_fs_pwd/root > in the kernel. Yeah, we'd need specific helpers for a never seen before out-of-tree BPF LSM. I don't see how that's different from an out-of-tree kernel module. > One can argue that get_mm_exe_file() is not exported, > but it's nothing but rcu_lock-wrap plus get_file_rcu() > which is EXPORT_SYMBOL. Oh, good spot. That's an accident. get_file_rcu() definitely shouldn't be exported. So that'll be removed asap. > Christian, > > I understand your irritation with anything bpf related > due to our mistake with fd=0, but as I said earlier it was > an honest mistake. There was no malicious intent. > Time to move on. I understand your concern but I'm neither irritated by bpf nor do I hold grudges. As I'm writing this bpf's bpffs and token changes are on their way into mainline for v6.9 which I reviewed and acked immediately after that discussion. Rest assures that you can move on from that concern. There's no need to keep bringing it up in unrelated discussions.