linux-kernel - Re: [PATCH bpf-next 0/4] Make inode storage available to tracing prog

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAHC9VhT4-aVbx_4EV3jAj27CUT=Lk0eb_fTRzFjHU8OO=ske8g@mail.gmail.com>
Date: Sat, 23 Nov 2024 14:11:27 -0500
From: Paul Moore <paul@...l-moore.com>
To: James Bottomley <James.Bottomley@...senpartnership.com>, 
	Casey Schaufler <casey@...aufler-ca.com>, Song Liu <songliubraving@...a.com>
Cc: "Dr. Greg" <greg@...ellic.com>, Song Liu <song@...nel.org>, 
	"bpf@...r.kernel.org" <bpf@...r.kernel.org>, 
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"linux-security-module@...r.kernel.org" <linux-security-module@...r.kernel.org>, Kernel Team <kernel-team@...a.com>, 
	"andrii@...nel.org" <andrii@...nel.org>, "eddyz87@...il.com" <eddyz87@...il.com>, "ast@...nel.org" <ast@...nel.org>, 
	"daniel@...earbox.net" <daniel@...earbox.net>, "martin.lau@...ux.dev" <martin.lau@...ux.dev>, 
	"viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>, "brauner@...nel.org" <brauner@...nel.org>, 
	"jack@...e.cz" <jack@...e.cz>, "kpsingh@...nel.org" <kpsingh@...nel.org>, 
	"mattbobrowski@...gle.com" <mattbobrowski@...gle.com>, "amir73il@...il.com" <amir73il@...il.com>, 
	"repnop@...gle.com" <repnop@...gle.com>, "jlayton@...nel.org" <jlayton@...nel.org>, 
	Josef Bacik <josef@...icpanda.com>, "mic@...ikod.net" <mic@...ikod.net>, 
	"gnoack@...gle.com" <gnoack@...gle.com>
Subject: Re: [PATCH bpf-next 0/4] Make inode storage available to tracing prog

On Thu, Nov 14, 2024 at 4:49 PM James Bottomley
<James.Bottomley@...senpartnership.com> wrote:
>
> Got to say I'm with Casey here, this will generate horrible and failure
> prone code.
>
> Since effectively you're making i_security always present anyway,
> simply do that and also pull the allocation code out of security.c in a
> way that it's always available?  That way you don't have to special
> case the code depending on whether CONFIG_SECURITY is defined.
> Effectively this would give everyone a generic way to attach some
> memory area to an inode.  I know it's more complex than this because
> there are LSM hooks that run from security_inode_alloc() but if you can
> make it work generically, I'm sure everyone will benefit.

My apologies on such a delayed response to this thread, I've had very
limited network access for a bit due to travel and the usual merge
window related distractions (and some others that were completely
unrelated) have left me with quite the mail backlog to sift through.

Enough with the excuses ...

Quickly skimming this thread and the v3 patchset, I would advise you
that there may be issues around using BPF LSMs and storing inode LSM
state outside the LSM managed inode storage blob.  Beyond the
conceptual objections that Casey has already mentioned, there have
been issues relating to the disjoint inode and inode->i_security
lifetimes.  While I believe we have an okay-ish solution in place now
for LSMs, I can't promise everything will work fine for BPF LSMs that
manage their inode LSM state outside of the LSM managed inode blob.
I'm sure you've already looked at it, but if you haven't it might be
worth looking at security_inode_free() to see some of the details.  In
a perfect world inode->i_security would be better synchronized with
the inode lifetime, but that would involve changes that the VFS folks
dislike.

However, while I will recommend against it, I'm not going to object to
you storing BPF LSM inode state elsewhere, that is up to you and KP
(he would need to ACK that as the BPF LSM maintainer).  I just want
you to know that if things break, there isn't much we (the LSM folks)
will be able to do to help other than suggest you go back to using the
LSM managed inode storage.

As far as some of the other ideas in this thread are concerned, at
this point in time I don't think we want to do any massive rework or
consolidation around i_security.  That's a critical field for the LSM
framework and many individual LSMs and there is work underway which
relies on this as a LSM specific inode storage blob; having to share
i_security with non-LSM users or moving the management of i_security
outside of the LSM is not something I'm overly excited about right
now.

-- 
paul-moore.com