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.