Re: [PATCH v5 2/3] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
"Kumar Kartikeya Dwivedi" <[email protected]> Mon, 20 Jul 2026 23:24:49 +0200
| Newsgroups | org.kernel.vger.linux-integrity,org.kernel.vger.bpf,org.kernel.vger.linux-fsdevel,org.kernel.vger.linux-kernel,org.kernel.vger.linux-kselftest,org.kernel.vger.linux-security-module,org.kernel.vger.selinux |
|---|---|
| Message-ID | <[email protected]> |
On Mon Jul 20, 2026 at 10:15 PM CEST, Paul Moore wrote: > On Mon, Jul 20, 2026 at 2:27=E2=80=AFPM Kumar Kartikeya Dwivedi > <[email protected]> wrote: >> On Mon Jul 20, 2026 at 8:12 PM CEST, David Windsor wrote: >> > On Thu, Jul 16, 2026 at 5:55=E2=80=AFPM Paul Moore <[email protected]= m> wrote: >> >> >> >> >> >> I'm sorry David, now that I'm seeing this function again, especially >> >> with the LSM specific bits extracted into a LSM function, this absolu= tely >> >> belongs somewhere under security/. It's only callable from within a >> >> BPF LSM callback and all it does outside of some BPF pointer boilerpl= ate >> >> is call right back into a LSM helper function. >> >> >> >> If the BPF maintainers aren't willing to accept that, then we will al= l >> >> need to find another way. >> > >> > Where this code lands doesn't matter to me, so I'll stay out of the >> > decision of where it lives. > > Yes, you're caught up in some territorial wrangling that goes a fair > bit beyond what you're looking to accomplish. My apologies for my > half of that, it is an unfortunate part of upstream kernel > development. > >> > If this kfunc goes to security/, would we also want to move eg >> > bpf_set_dentry_xattr similarly? >> > >> > I'll roll v6 of this series meanwhile and we'll see what the BPF >> > maintainers say. > > Thanks. Fortunately, it should be easy enough to move the kfunc > around, that's likely one of the smaller changes needed in v6. > >> I don't think there was any preference expressed from the BPF side. > > Look at the second patch in the v4 patchset, you'll see a preference. > >> Logically, >> one cannot be faulted for adding a kfunc to set the xattr for inode in t= he same >> file where similar kfuncs to set xattr on other FS objects / entities ex= ist. > > First off, I can't say that the existing BPF kfuncs intended for use > within a BPF LSM program are in a good location. Unlike every other > LSM which locates its code under security, the BPF LSM devs have > chosen to locate their associated code all over the place. Some of > that happened before I was responsible for the LSM framework, and some > of that which has happened afterwards has likely intentionally avoided > traditional LSM review. Unfortunately, this has muddied things with > respect to what is possible generally in BPF versus what is possible > only when the BPF LSM is enabled; we just recently saw where this was > arguably the root cause of an exploitable vulnerability. > >> The question about bpf_set_dentry_xattr is thus valid. > > It's a very valid question. It is my opinion that kfuncs which are > only intended to be called from within BPF LSMs should be located > under security/, just like every other LSM. If there is a kfunc that > is generally useful from other contexts, that is a different story, > but in the bpf_set_dentry_xattr() case, it is explicitly only useful > for setting 'security.bpf.*' xattrs. I don't think we should do that in v6 though. What you're describing affect= s all other xattr kfuncs. That's a broader discussion we should have separately w= hen folks are back from vacation, and more importantly involve FS folks where t= he code currently lives. As you say yourself, the lines are blurry, xattrs are= as much a filesystem concept. So I wouldn't burden David with it right now. David, just respin addressing= all other comments and the CI issue.