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 DCB87C5475B for ; Fri, 8 Mar 2024 03:11:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3292B6B031E; Thu, 7 Mar 2024 22:11:39 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 2DAED6B031F; Thu, 7 Mar 2024 22:11:39 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 153626B0320; Thu, 7 Mar 2024 22:11:39 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id F12B96B031E for ; Thu, 7 Mar 2024 22:11:38 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id B79A51C1589 for ; Fri, 8 Mar 2024 03:11:37 +0000 (UTC) X-FDA: 81872396634.15.0BA8A0B Received: from mail-ej1-f43.google.com (mail-ej1-f43.google.com [209.85.218.43]) by imf22.hostedemail.com (Postfix) with ESMTP id C71F0C0004 for ; Fri, 8 Mar 2024 03:11:35 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=BGkoDoZE; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf22.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.218.43 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1709867496; a=rsa-sha256; cv=none; b=GNFG4XHPGerAnYE+kk1YTFnwoA5Eauwuc3JwRS+nL/2JBrDQTTCThrieoZx9w55PcC08u5 lbKrebi7zCPezLbiGLsd5lSRGv0dRgibGsJd1oUqf8xi5M60oRuKNXi5hc2brJDrU/0Fb/ XtbcKpIhA7tAXsNSQZRcgD1SkYW3OoE= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=gmail.com header.s=20230601 header.b=BGkoDoZE; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf22.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.218.43 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1709867496; 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=Zj6q92htsofy/f5VcV6/t4KrLGQPNiFAadjpchJR4oU=; b=GnEdPmDcMJjgYCA5wxE+1M7OuxerqGeKtcxZ6VDjo+Ah9uu6F52wD4gph7FB27KiN90YFz HmN/53yd5ykINtM2b9sLxU6PePlGfduZP+nGJEP12YhNxSjATe/7gIqbC0XaPuKQHu5Y3z KcYFqvTuJnbJinKyHxrmvdRFKTf4wNY= Received: by mail-ej1-f43.google.com with SMTP id a640c23a62f3a-a44f2d894b7so230817166b.1 for ; Thu, 07 Mar 2024 19:11:35 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709867494; x=1710472294; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=Zj6q92htsofy/f5VcV6/t4KrLGQPNiFAadjpchJR4oU=; b=BGkoDoZEpxhV3Rrxai5fSdf8DoOsyFdlf92vLR34S6YVyhEV1g5K5EEcip1OjfRYWk PV5GfmQ1m2pA1iIiOUBGIOiY0SYnZUcwK+S4eWi7HDszT8LF8Rh9sLA/9paHPwY/zWqB ca9RpjCreBR2cRBLK8x3rF1c5xsh7ozVmHrtrOyJTcwzRZZKFSeNTRej0hlm+LRpFdHv zwTC4T3PddoFrY7w6nXbhoYSwXeTxDhdKYA4lWAOQItaziJXyLWhjLHRA2y9PRcH2Pf/ OVYCv277DuhJ1rMAvhkdxTio4xPyIyA+TmWJHv8R8caO6GAl/zVuBaHC1wIOxGvyZAPS ew9g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709867494; x=1710472294; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Zj6q92htsofy/f5VcV6/t4KrLGQPNiFAadjpchJR4oU=; b=VSrl57/SQKpjKdZZajieeDuU+YkIXyJ0gjjY8EvedlBVTOjlGE1OxMiregSKiiUIQa 3AP/RJd9V3hQIk+Rfw49V1ur+FAR08BfJlH8kO3z0cobjWApemsZDu3ua2kgkjCGLJN2 WiMMBxUOiZ7ugeaICmcDwn/3bZO6yiYvdsV79NwD9oE+Eznoq1sb8lLNHRIu8dRaFGOS vVdAt0eYZ6XNw+D9gfSLb7V7hJpS/LZ9mY39gGJCcEo31MlLLgJ3JclBQk9agpiEruQ9 Worzea2Ej9xR1I9CujZhZKk/7IrjUNCpoJthJKQsqdiZW8OQKYeC2PqwJtVVaWASXZhB GTMg== X-Forwarded-Encrypted: i=1; AJvYcCU0DZcjRG7ZHOoNum+f/xeGUTDGBNIpIKL3M/sjjRjM4hckqvWkTfC6Zu/eI6fEif6tFUqOWVMWlkCPRldDZo3kNYE= X-Gm-Message-State: AOJu0Ywj4lXbk5J6Zkr6K8ipZlbrZE16ZAGiufIs9453+OMejhfxC1Rl gNBrJXro7u0PIdjG2f95R4vGSK1WBorvfv/XjWu7i3lrkD1AKIVH71WPscqe7TsaFBmU73HRIes VFR7ofenkEMbaTgBSm/CunTQs8KU= X-Google-Smtp-Source: AGHT+IGeKNrA4flz39TyY21Qq+eMtzgSQnDG1XmDYjlybgEaOqh2F/oNtXZp6fFMkAue7HFtuFShEgO4kQ1nU/EueDw= X-Received: by 2002:a17:906:5953:b0:a44:b90b:3262 with SMTP id g19-20020a170906595300b00a44b90b3262mr13474080ejr.5.1709867493755; Thu, 07 Mar 2024 19:11:33 -0800 (PST) MIME-Version: 1.0 References: <20240306-flach-tragbar-b2b3c531bf0d@brauner> <20240306-sandgrube-flora-a61409c2f10c@brauner> <20240307-phosphor-entnahmen-8ef28b782abf@brauner> In-Reply-To: <20240307-phosphor-entnahmen-8ef28b782abf@brauner> From: Alexei Starovoitov Date: Thu, 7 Mar 2024 19:11:22 -0800 Message-ID: Subject: Re: [PATCH v2 bpf-next 0/9] add new acquire/release BPF kfuncs To: Christian Brauner 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 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: C71F0C0004 X-Stat-Signature: kqi99bkf3wq1uh6jg4xkd6pfdbb8ndue X-HE-Tag: 1709867495-750599 X-HE-Meta: U2FsdGVkX1+ZN68/BjaLRYDnY8HkgBWvs/B+TuHdgE34TH66PIneCZzo/8Ryl1x/NpDcQ/olA6qnCzwZuUHNRAtz2aX/zNzehh2Tvxy6nyGP5pHa43XfE6/Dg2vyxNKiHjGf34W15Rf8BmpnVF9VnROfKUFQMu1nj/Vh7O0l3+UWk71Y/531CjyVEl4ELH7YPZ9qv2vWURFz91eFKLrkveJlMW+S9UK18LvfhjfSVGHOOtKF0q/vFZBqTnPQzB2KFibFT/Tl2Q1zwf+qxt/FaU5Z1HrRwsy2Z/KyU/jkhQkaj6wwxGHGEdW8iMddfXbZxXt/4HcsAZw1ESBLyI1Q7zRYggYVFG0QwKqaMYk6XKTPesiDUxklai1MOW7iPF9jyWDo3c3YikvTTnY292tGAZPQrTv1Rif4oZcMA9nKpbKfN0qTjLArmud0aG1deCM15MGbFIF6KEnQOIxF8Ypc6Wbeok/Pcch6ND1jeE9aqAkOZieWFxkjJT1KTXPZ3U2t2gjdDyGH03dykdj2SRqhtNiQ6FmVl9PAkm6HjnJBfkk160YJfW8DV/XSuRHFOwiYpotSN8rx854Xy1FFKvB1iKCNF5gE87f5FgupL683QojVhDjlW1OONuqidsgN1i28akziWW/lSTRuse7jxuqEK2tDQtj6q6ROMCbRZmaSSa+D3a+aCc2rCVTompiC64rd7TQeM5u8sErDRvYjGnjkFej8/IKEQeHBEGu/rd8Eq8mReKj/NFNKxCEwM/HYAwHtb1uupAVhI/gNAoJCDYCLu9xIouGOZge4Gk5zu2uZp0URJVvLGGaOHe4MBkwFKatHdcnpYaOy6q5C5VT95dA1aHTk5wU92D8WnhAJKEUIu8MBPGdqIXTEgsZ2Q7JLdnivqCjLlkwgdCB0YEyiAaPF0NrYShNfb2s3PrdDdmDzCXqce2rsZDWOiFu2P4YuCoUmJ+FlTZbzdy4EQuV14aF 3t+CV1ds ACUxC+ZMlCEaPzeZxrxMYfMc6DjhH8hakx6Jx7I9g9zFy9fvjcdlMrNBgEkm9JLcnnZNvNyhNcq4NGD6QLzYJgIhAjbIqP7UwL9nLN9j3oGfmPslUseBoLLROWM9f+lR9J1NAnw4MX+dH+jV/Zh2nhyzzZLUNY3mYbOk1BQ7RFAVDVJQJbpr6L77hEL1W8b2HOQrmHCAOTKN9n7Iq75Pd7nuH0+k68P5rcrO3ezBo71RZPH2N4v6IYkk4oECFtF6iUKNnmynw/JNEfGzzb0T0gkNeceFrh+bNP+5r8k0pfUGjBEhczbI7KbxpO/6OM7pqN48CLBEpt/KG1s+Kx5n8MzznUNox+udXcUeAioAaTY+h7DgP0V/JUe3/MXMNFVRLd3xLdIA4f9ieLPq786ihbuHUoSUgcD//nZvm3QkhtlNp1la1MO4jf4ka0H6yDKcvMWi7 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 7, 2024 at 1:55=E2=80=AFAM 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 argument= s. > > > So you're now scrambling to fix this by asking for a bunch of low-lev= el > > > exports. > > > > > > What is the guarantee that you don't end up writing another BPF LSM t= hat > > > 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 =3D bpf_get_current_task_btf(); > > + /* Walking a trusted pointer returned from bpf_get_current_task= _btf() > > + * yields and untrusted pointer. */ > > + pwd =3D ¤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 =3D bpf_get_current_task_btf(); > > pwd =3D 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. Quoting previous reply: " The above "low level" helpers are all either static inline in .h or they call EXPORT_SYMBOL[_GPL] or simply inc/dec refcnt. " Let's look at your list: > > > (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() First of all, there is no such thing as get_task_fs_pwd/root in the kernel. The hypothetical out-of-tree ZFS can use the rest just fine. 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. kfunc-s in patch 6 wrap get_fs_root/pwd from linux/fs_struct.h that calls EXPORT_SYMBOL(path_get). So upon close examination no new EXPORT_SYMBOL-s were requested. Christian, I understand your irritation with anything bpf related due to our mistake with fd=3D0, but as I said earlier it was an honest mistake. There was no malicious intent. Time to move on.