From: "Dr. Greg" <greg@enjellic.com>
To: Song Liu <song@kernel.org>
Cc: Casey Schaufler <casey@schaufler-ca.com>,
Song Liu <songliubraving@meta.com>,
"bpf@vger.kernel.org" <bpf@vger.kernel.org>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-security-module@vger.kernel.org"
<linux-security-module@vger.kernel.org>,
Kernel Team <kernel-team@meta.com>,
"andrii@kernel.org" <andrii@kernel.org>,
"eddyz87@gmail.com" <eddyz87@gmail.com>,
"ast@kernel.org" <ast@kernel.org>,
"daniel@iogearbox.net" <daniel@iogearbox.net>,
"martin.lau@linux.dev" <martin.lau@linux.dev>,
"viro@zeniv.linux.org.uk" <viro@zeniv.linux.org.uk>,
"brauner@kernel.org" <brauner@kernel.org>,
"jack@suse.cz" <jack@suse.cz>,
"kpsingh@kernel.org" <kpsingh@kernel.org>,
"mattbobrowski@google.com" <mattbobrowski@google.com>,
"amir73il@gmail.com" <amir73il@gmail.com>,
"repnop@google.com" <repnop@google.com>,
"jlayton@kernel.org" <jlayton@kernel.org>,
Josef Bacik <josef@toxicpanda.com>,
"mic@digikod.net" <mic@digikod.net>,
"gnoack@google.com" <gnoack@google.com>
Subject: Re: [PATCH bpf-next 0/4] Make inode storage available to tracing prog
Date: Thu, 14 Nov 2024 10:36:41 -0600 [thread overview]
Message-ID: <20241114163641.GA8697@wind.enjellic.com> (raw)
In-Reply-To: <CAPhsuW5zDzUp7eSut9vekzH7WZHpk38fKHmFVRTMiBbeW10_SQ@mail.gmail.com>
On Wed, Nov 13, 2024 at 10:57:05AM -0800, Song Liu wrote:
Good morning, I hope the week is going well for everyone.
> On Wed, Nov 13, 2024 at 10:06???AM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> > On 11/12/2024 5:37 PM, Song Liu wrote:
> [...]
> > > Could you provide more information on the definition of "more
> > > consistent" LSM infrastructure?
> >
> > We're doing several things. The management of security blobs
> > (e.g. inode->i_security) has been moved out of the individual
> > modules and into the infrastructure. The use of a u32 secid is
> > being replaced with a more general lsm_prop structure, except
> > where networking code won't allow it. A good deal of work has
> > gone into making the return values of LSM hooks consistent.
>
> Thanks for the information. Unifying per-object memory usage of
> different LSMs makes sense. However, I don't think we are limiting
> any LSM to only use memory from the lsm_blobs. The LSMs still
> have the freedom to use other memory allocators. BPF inode
> local storage, just like other BPF maps, is a way to manage
> memory. BPF LSM programs have full access to BPF maps. So
> I don't think it makes sense to say this BPF map is used by tracing,
> so we should not allow LSM to use it.
>
> Does this make sense?
As involved bystanders, some questions and thoughts that may help
further the discussion.
With respect to inode specific storage, the currently accepted pattern
in the LSM world is roughly as follows:
The LSM initialization code, at boot, computes the total amount of
storage needed by all of the LSM's that are requesting inode specific
storage. A single pointer to that 'blob' of storage is included in
the inode structure.
In an include file, an inline function similar to the following is
declared, whose purpose is to return the location inside of the
allocated storage or 'LSM inode blob' where a particular LSM's inode
specific data structure is located:
static inline struct tsem_inode *tsem_inode(struct inode *inode)
{
return inode->i_security + tsem_blob_sizes.lbs_inode;
}
In an LSM's implementation code, the function gets used in something
like the following manner:
static int tsem_inode_alloc_security(struct inode *inode)
{
struct tsem_inode *tsip = tsem_inode(inode);
/* Do something with the structure pointed to by tsip. */
}
Christian appears to have already chimed in and indicated that there
is no appetite to add another pointer member to the inode structure.
So, if this were to proceed forward, is it proposed that there will be
a 'flag day' requirement to have each LSM that uses inode specific
storage implement a security_inode_alloc() event handler that creates
an LSM specific BPF map key/value pair for that inode?
Which, in turn, would require that the accessor functions be converted
to use a bpf key request to return the LSM specific information for
that inode?
A flag day event is always somewhat of a concern, but the larger
concern may be the substitution of simple pointer arithmetic for a
body of more complex code. One would assume with something like this,
that there may be a need for a shake-out period to determine what type
of potential regressions the more complex implementation may generate,
with regressions in security sensitive code always a concern.
In a larger context. Given that the current implementation works on
simple pointer arithmetic over a common block of storage, there is not
much of a safety guarantee that one LSM couldn't interfere with the
inode storage of another LSM. However, using a generic BPF construct
such as a map, would presumably open the level of influence over LSM
specific inode storage to a much larger audience, presumably any BPF
program that would be loaded.
The LSM inode information is obviously security sensitive, which I
presume would be be the motivation for Casey's concern that a 'mistake
by a BPF programmer could cause the whole system to blow up', which in
full disclosure is only a rough approximation of his statement.
We obviously can't speak directly to Casey's concerns. Casey, any
specific technical comments on the challenges of using a common inode
specific storage architecture?
Song, FWIW going forward. I don't know how closely you follow LSM
development, but we believe an unbiased observer would conclude that
there is some degree of reticence about BPF's involvement with the LSM
infrastructure by some of the core LSM maintainers, that in turn makes
these types of conversations technically sensitive.
> Song
We will look forward to your thoughts on the above.
Have a good week.
As always,
Dr. Greg
The Quixote Project - Flailing at the Travails of Cybersecurity
https://github.com/Quixote-Project
next prev parent reply other threads:[~2024-11-14 17:06 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-12 8:25 [PATCH bpf-next 0/4] Make inode storage available to tracing prog Song Liu
2024-11-12 8:25 ` [PATCH bpf-next 1/4] bpf: lsm: Remove hook to bpf_task_storage_free Song Liu
2024-11-12 8:25 ` [PATCH bpf-next 2/4] bpf: Make bpf inode storage available to tracing program Song Liu
2024-11-13 10:19 ` Christian Brauner
2024-11-13 14:15 ` Song Liu
2024-11-13 18:29 ` Casey Schaufler
2024-11-13 19:00 ` Song Liu
2024-11-21 9:04 ` Christian Brauner
2024-11-14 21:11 ` Song Liu
2024-11-15 11:19 ` Jan Kara
2024-11-15 17:35 ` Song Liu
2024-11-19 14:21 ` Jeff Layton
2024-11-19 15:25 ` Amir Goldstein
2024-11-19 15:30 ` Amir Goldstein
2024-11-19 21:53 ` Song Liu
2024-11-20 9:19 ` Amir Goldstein
2024-11-20 9:28 ` Christian Brauner
2024-11-20 11:19 ` Amir Goldstein
2024-11-21 8:43 ` Christian Brauner
2024-11-21 13:48 ` Jeff Layton
2024-11-21 8:08 ` Song Liu
2024-11-21 9:14 ` Christian Brauner
2024-11-23 0:08 ` Alexei Starovoitov
2024-11-12 8:25 ` [PATCH bpf-next 3/4] bpf: Add recursion avoid logic for inode storage Song Liu
2024-11-12 8:25 ` [PATCH bpf-next 3/4] bpf: Add recursion prevention " Song Liu
2024-11-12 8:25 ` [PATCH bpf-next 4/4] selftest/bpf: Add test for inode local storage recursion Song Liu
2024-11-12 8:26 ` [PATCH bpf-next 4/4] selftest/bpf: Test inode local storage recursion prevention Song Liu
2024-11-12 8:35 ` [PATCH bpf-next 0/4] Make inode storage available to tracing prog Song Liu
2024-11-12 18:09 ` Casey Schaufler
2024-11-12 18:44 ` Song Liu
2024-11-13 1:10 ` Casey Schaufler
2024-11-13 1:37 ` Song Liu
2024-11-13 18:06 ` Casey Schaufler
2024-11-13 18:57 ` Song Liu
2024-11-14 16:36 ` Dr. Greg [this message]
2024-11-14 17:29 ` Casey Schaufler
2024-11-14 18:08 ` Song Liu
2024-11-14 21:49 ` James Bottomley
2024-11-14 22:30 ` Song Liu
2024-11-17 22:59 ` Song Liu
2024-11-19 12:27 ` Dr. Greg
2024-11-19 18:14 ` Casey Schaufler
2024-11-19 22:35 ` Song Liu
2024-11-20 16:54 ` Dr. Greg
2024-11-21 8:28 ` Song Liu
2024-11-21 16:02 ` Dr. Greg
2024-11-21 18:11 ` Casey Schaufler
2024-11-23 17:01 ` Dr. Greg
2024-11-25 20:49 ` Casey Schaufler
2024-11-21 17:47 ` Casey Schaufler
2024-11-21 18:28 ` Song Liu
2024-11-23 19:11 ` Paul Moore
2024-11-14 17:51 ` Song Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241114163641.GA8697@wind.enjellic.com \
--to=greg@enjellic.com \
--cc=amir73il@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brauner@kernel.org \
--cc=casey@schaufler-ca.com \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=gnoack@google.com \
--cc=jack@suse.cz \
--cc=jlayton@kernel.org \
--cc=josef@toxicpanda.com \
--cc=kernel-team@meta.com \
--cc=kpsingh@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=mattbobrowski@google.com \
--cc=mic@digikod.net \
--cc=repnop@google.com \
--cc=song@kernel.org \
--cc=songliubraving@meta.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).