* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Paul Moore @ 2026-04-28 1:31 UTC (permalink / raw)
To: Yeoreum Yun
Cc: Mimi Zohar, roberto.sassu, Jonathan McDowell,
linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, jmorris, serge, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene
In-Reply-To: <aexIwJpno3iPIdRD@e129823.arm.com>
On Sat, Apr 25, 2026 at 12:53 AM Yeoreum Yun <yeoreum.yun@arm.com> wrote:
> > > I understand the need to ensure that the TPM is available, but if it
> > > isn't safe to wait to initialize IMA at late_initcall_sync() then it
> > > would seem like this is a bad option and we need another mechanism to
> > > synchronize IMA with TPM devices. If it is safe to initalize IMA in
> > > late_initcall_sync(), just do that and be done with it.
> >
> > Within the same initcall level there is no way of ordering the initialization.
> > Yeorum attempted to address the ordering issue in commit 0e0546eabcd6
> > ("firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall"),
> > which is being reverted in this patch set.
> >
> > Ordering within an initcall level needs to be fixed, but for now retrying at
> > late_initcall_sync works for some, hopefully most, cases.
>
> Ordering within an initcall level is not good idea.
Agreed. That's why we have the different initcall levels.
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Paul Moore @ 2026-04-28 1:31 UTC (permalink / raw)
To: Mimi Zohar
Cc: Yeoreum Yun, roberto.sassu, Jonathan McDowell,
linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, jmorris, serge, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene
In-Reply-To: <1e51c2fd090e5ceb07b1d09e50650c70fd3ccdb1.camel@linux.ibm.com>
On Fri, Apr 24, 2026 at 6:49 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Fri, 2026-04-24 at 18:10 -0400, Paul Moore wrote:
> > (I'm assuming you meant initcall and not syscall above, but if you're
> > talking about something else, please let me know.)
> >
> > Saying that you aren't comfortable moving IMA initialization to
> > late-sync is inconsistent with allowing IMA initialization to be
> > deferred to late-sync. Either it is okay to initialize IMA in
> > late-sync or it isn't. You must pick one.
>
> Yes, we're discussing late_initcall and late_initcall_sync.
>
> I prefer to look at it as being pragmatic. I'd rather err on the side of caution
> and not move the syscall to late_initcall_sync, than move it.
If you were truly erring on the side of caution you wouldn't allow
late-sync initialization without knowing if it was safe or not.
Determine whether IMA initialization is safe at late-sync. If it is
safe, move the init to late-sync; if not, keep it at late and figure
out another mechanism to sync with the TPM availability. If needed,
you could probably use the LSM notifier to enable the TPM driver to
signal when it is up and running.
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH] selinux: don't reserve xattr slot when we won't fill it
From: Paul Moore @ 2026-04-27 23:32 UTC (permalink / raw)
To: David Windsor, Stephen Smalley
Cc: Ondrej Mosnacek, selinux, linux-security-module, linux-kernel
In-Reply-To: <20260426232349.844289-1-dwindsor@gmail.com>
On Apr 26, 2026 David Windsor <dwindsor@gmail.com> wrote:
>
> Move lsm_get_xattr_slot() below the SBLABEL_MNT check so we don't leave
> a NULL-named slot in the array when returning -EOPNOTSUPP; filesystem
> initxattrs() callbacks stop iterating at the first NULL ->name, silently
> dropping xattrs installed by later LSMs.
>
> Signed-off-by: David Windsor <dwindsor@gmail.com>
> ---
> security/selinux/hooks.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
Good catch, thanks. These seems like a stable candidate so I've merged
it into selinux/stable-7.1 and we likely send it up to Linus later this
week.
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 97801966bf32..4ff118a9395f 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2966,7 +2966,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> {
> const struct cred_security_struct *crsec = selinux_cred(current_cred());
> struct superblock_security_struct *sbsec;
> - struct xattr *xattr = lsm_get_xattr_slot(xattrs, xattr_count);
> + struct xattr *xattr;
> u32 newsid, clen;
> u16 newsclass;
> int rc;
> @@ -2992,6 +2992,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
> !(sbsec->flags & SBLABEL_MNT))
> return -EOPNOTSUPP;
>
> + xattr = lsm_get_xattr_slot(xattrs, xattr_count);
> if (xattr) {
> rc = security_sid_to_context_force(newsid,
> &context, &clen);
>
> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
> --
> 2.53.0
--
paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH v1 01/11] security: add LSM blob and hooks for namespaces
From: Paul Moore @ 2026-04-27 21:46 UTC (permalink / raw)
To: Christian Brauner
Cc: Mickaël Salaün, Günther Noack, Serge E . Hallyn,
Justin Suess, Lennart Poettering, Mikhail Ivanov,
Nicolas Bouchinet, Shervin Oloumi, Tingmao Wang, kernel-team,
linux-fsdevel, linux-kernel, linux-security-module
In-Reply-To: <20260427-belegen-euren-997f91347820@brauner>
On Mon, Apr 27, 2026 at 10:57 AM Christian Brauner <brauner@kernel.org> wrote:
> On Fri, Apr 24, 2026 at 03:28:44PM -0400, Paul Moore wrote:
> > On Fri, Apr 24, 2026 at 2:56 PM Mickaël Salaün <mic@digikod.net> wrote:
> > > On Wed, Apr 22, 2026 at 08:19:59PM -0400, Paul Moore wrote:
> > > > On Thu, Mar 12, 2026 at 6:05 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > > >
> > > > > From: Christian Brauner <brauner@kernel.org>
> > > > >
> > > > > All namespace types now share the same ns_common infrastructure. Extend
> > > > > this to include a security blob so LSMs can start managing namespaces
> > > > > uniformly without having to add one-off hooks or security fields to
> > > > > every individual namespace type.
> > > > >
> > > > > Add a ns_security pointer to ns_common and the corresponding lbs_ns
> > > > > blob size to lsm_blob_sizes. Allocation and freeing hooks are called
> > > > > from the common __ns_common_init() and __ns_common_free() paths so
> > > > > every namespace type gets covered in one go. All information about the
> > > > > namespace type and the appropriate casting helpers to get at the
> > > > > containing namespace are available via ns_common making it
> > > > > straightforward for LSMs to differentiate when they need to.
> > > > >
> > > > > A namespace_install hook is called from validate_ns() during setns(2)
> > > > > giving LSMs a chance to enforce policy on namespace transitions.
> > > > >
> > > > > Individual namespace types can still have their own specialized security
> > > > > hooks when needed. This is just the common baseline that makes it easy
> > > > > to track and manage namespaces from the security side without requiring
> > > > > every namespace type to reinvent the wheel.
> > > > >
> > > > > Cc: Günther Noack <gnoack@google.com>
> > > > > Cc: Paul Moore <paul@paul-moore.com>
> > > > > Cc: Serge E. Hallyn <serge@hallyn.com>
> > > > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > > > Link: https://lore.kernel.org/r/20260216-work-security-namespace-v1-1-075c28758e1f@kernel.org
> > > > > ---
> > > > > include/linux/lsm_hook_defs.h | 3 ++
> > > > > include/linux/lsm_hooks.h | 1 +
> > > > > include/linux/ns/ns_common_types.h | 3 ++
> > > > > include/linux/security.h | 20 ++++++++
> > > > > kernel/nscommon.c | 12 +++++
> > > > > kernel/nsproxy.c | 8 +++-
> > > > > security/lsm_init.c | 2 +
> > > > > security/security.c | 76 ++++++++++++++++++++++++++++++
> > > > > 8 files changed, 124 insertions(+), 1 deletion(-)
> >
> > ...
> >
> > > > > diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
> > > > > index 259c4b4f1eeb..f0b30d1907e7 100644
> > > > > --- a/kernel/nsproxy.c
> > > > > +++ b/kernel/nsproxy.c
> > > > > @@ -379,7 +379,13 @@ static int prepare_nsset(unsigned flags, struct nsset *nsset)
> > > > >
> > > > > static inline int validate_ns(struct nsset *nsset, struct ns_common *ns)
> > > > > {
> > > > > - return ns->ops->install(nsset, ns);
> > > > > + int ret;
> > > > > +
> > > > > + ret = ns->ops->install(nsset, ns);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > +
> > > > > + return security_namespace_install(nsset, ns);
> > > > > }
> > > >
> > > > Do we also want a security_namespace_switch() called from within
> > > > switch_task_namespaces()? Of course LSMs would not be able to fail or
> > > > return an error at that point, but it seems reasonable that LSMs might
> > > > want to update LSM state associated with the current task once the
> > > > namespaces have been changed. This is similar to all the "_post_" LSM
> > > > hooks we have for various operations in the VFS and network layers.
> > >
> > > What cannot be infered from security_namespace_install()?
> >
> > We don't actually know if the namespace is attached to a process until
> > we get to switch_task_namespaces().
> >
> > Now that I'm looking at this again, why is the
> > security_namespace_install() call placed after the ns->ops->install()
> > call? From an access control perspective we want the LSM hook before
>
> See https://lore.kernel.org/20260325-filmverleih-auffressen-e897fcf8d3f2@brauner
> where I requested the order to be changed.
So ... does anyone not want this moved? It's time to speak up :)
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v2 0/4] Firmware LSM hook
From: Leon Romanovsky @ 2026-04-27 19:09 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Paul Moore, Roberto Sassu, KP Singh, Matt Bobrowski,
Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
Saeed Mahameed, Itay Avraham, Dave Jiang, Jonathan Cameron, bpf,
linux-kernel, linux-kselftest, linux-rdma, Chiara Meiohas,
Maher Sanalla, linux-security-module
In-Reply-To: <20260426134224.GC3501894@ziepe.ca>
On Sun, Apr 26, 2026 at 10:42:24AM -0300, Jason Gunthorpe wrote:
> On Sun, Apr 26, 2026 at 01:39:57PM +0300, Leon Romanovsky wrote:
> > On Fri, Apr 24, 2026 at 11:19:21AM -0300, Jason Gunthorpe wrote:
> > > On Thu, Apr 23, 2026 at 05:09:50PM +0300, Leon Romanovsky wrote:
> > >
> > > > > > Leon mentioned that different firmware revisions would have different
> > > > > > parameters for a given opcode, and that one would need to inspect
> > > > > > those parameters to properly filter the command. Is that not true, or
> > > > > > am I misreading or misunderstanding Leon's comments?
> > > > >
> > > > > They are ABI stable, so there will be rules about future changes that
> > > > > old software can follow to ignore or reject future things it doesn't
> > > > > understand.
> > > >
> > > > It is wishful thinking and applicable only to mlx5 devices. No one
> > > > promises that other devices follow same ABI rules.
> > >
> > > Well, I will definately kick them out of fwctl if they don't.
> >
> > It is easy to say but harder to follow. The kernel includes many devices that
> > exist only in specific hyperscale environments, where the update cycle is
> > tightly controlled. They easily can break FW backward compatibility.
>
> Well Linus's rule applies here, if it doesn't bother anyone it didn't
> break..
Great, that means they can load any BPF program they want and access whatever
firmware fields they choose. Your earlier claim about 'not breaking FW
compatibility' is only partially correct.
Thanks
>
> Jason
>
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: Song Liu @ 2026-04-27 17:17 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: Matt Bobrowski, David Windsor, Alexander Viro, Christian Brauner,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, KP Singh, Paul Moore, James Morris,
Serge E. Hallyn, Jan Kara, John Fastabend, Martin KaFai Lau,
Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf,
linux-security-module
In-Reply-To: <CAP01T75SBw6NNvBKyPW11JSYY2oh449yoBsWi_GOBR5Kq1ykmw@mail.gmail.com>
On Mon, Apr 27, 2026 at 3:33 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Mon, 27 Apr 2026 at 16:21, Song Liu <song@kernel.org> wrote:
[...]
> > > Hm, perhaps this fixup approach might be the simplest in order to
> > > assure the needed safety?
> >
> > +1. I think this is the best approach I can think of.
>
> We're not going to add more and more special cases to the verifier.
> The whole approach is unscalable.
Agreed this is not scalable. One potential solution to this scalability
issue is to move the fixup logic to struct btf_kfunc_id_set, so that this
fixup logic is distributed.
> If the concern is that int xattr_count passed for xattrs can be
> unrelated int pointer obtained from elsewhere, can we pack the xattrs
> and xattr_count into a struct and pass it as an argument to the LSM?
> Then the pair struct can be passed in directly, ensuring both
> originate from the arguments passed to the LSM. That should eliminate
> concerns about either being out of sync if obtained from different
> sources.
I think a trusted pointer of the pair struct will also work. But this means
we need to refactor the LSM hook and other LSMs. The refactoring is
not difficult though.
> Even if we wanted to ensure argument provenance was stuff loaded from
> context, the right solution would be some kfunc flag that constraints
> the argument to be derived by following the ctx pointer, not whatever
> is done in this patch.
We need these two arguments to be the specific fields in the ctx. I am
not sure how to do this with kfunc flags.
Thanks,
Song
^ permalink raw reply
* Re: [RFC PATCH v1 01/11] security: add LSM blob and hooks for namespaces
From: Christian Brauner @ 2026-04-27 14:57 UTC (permalink / raw)
To: Paul Moore
Cc: Mickaël Salaün, Günther Noack, Serge E . Hallyn,
Justin Suess, Lennart Poettering, Mikhail Ivanov,
Nicolas Bouchinet, Shervin Oloumi, Tingmao Wang, kernel-team,
linux-fsdevel, linux-kernel, linux-security-module
In-Reply-To: <CAHC9VhRcokUR0ZKzCuZnZAyaFEMd6EH93BE3OTTKHY9Mo9pVkQ@mail.gmail.com>
On Fri, Apr 24, 2026 at 03:28:44PM -0400, Paul Moore wrote:
> On Fri, Apr 24, 2026 at 2:56 PM Mickaël Salaün <mic@digikod.net> wrote:
> > On Wed, Apr 22, 2026 at 08:19:59PM -0400, Paul Moore wrote:
> > > On Thu, Mar 12, 2026 at 6:05 AM Mickaël Salaün <mic@digikod.net> wrote:
> > > >
> > > > From: Christian Brauner <brauner@kernel.org>
> > > >
> > > > All namespace types now share the same ns_common infrastructure. Extend
> > > > this to include a security blob so LSMs can start managing namespaces
> > > > uniformly without having to add one-off hooks or security fields to
> > > > every individual namespace type.
> > > >
> > > > Add a ns_security pointer to ns_common and the corresponding lbs_ns
> > > > blob size to lsm_blob_sizes. Allocation and freeing hooks are called
> > > > from the common __ns_common_init() and __ns_common_free() paths so
> > > > every namespace type gets covered in one go. All information about the
> > > > namespace type and the appropriate casting helpers to get at the
> > > > containing namespace are available via ns_common making it
> > > > straightforward for LSMs to differentiate when they need to.
> > > >
> > > > A namespace_install hook is called from validate_ns() during setns(2)
> > > > giving LSMs a chance to enforce policy on namespace transitions.
> > > >
> > > > Individual namespace types can still have their own specialized security
> > > > hooks when needed. This is just the common baseline that makes it easy
> > > > to track and manage namespaces from the security side without requiring
> > > > every namespace type to reinvent the wheel.
> > > >
> > > > Cc: Günther Noack <gnoack@google.com>
> > > > Cc: Paul Moore <paul@paul-moore.com>
> > > > Cc: Serge E. Hallyn <serge@hallyn.com>
> > > > Signed-off-by: Christian Brauner <brauner@kernel.org>
> > > > Link: https://lore.kernel.org/r/20260216-work-security-namespace-v1-1-075c28758e1f@kernel.org
> > > > ---
> > > > include/linux/lsm_hook_defs.h | 3 ++
> > > > include/linux/lsm_hooks.h | 1 +
> > > > include/linux/ns/ns_common_types.h | 3 ++
> > > > include/linux/security.h | 20 ++++++++
> > > > kernel/nscommon.c | 12 +++++
> > > > kernel/nsproxy.c | 8 +++-
> > > > security/lsm_init.c | 2 +
> > > > security/security.c | 76 ++++++++++++++++++++++++++++++
> > > > 8 files changed, 124 insertions(+), 1 deletion(-)
>
> ...
>
> > > > diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c
> > > > index 259c4b4f1eeb..f0b30d1907e7 100644
> > > > --- a/kernel/nsproxy.c
> > > > +++ b/kernel/nsproxy.c
> > > > @@ -379,7 +379,13 @@ static int prepare_nsset(unsigned flags, struct nsset *nsset)
> > > >
> > > > static inline int validate_ns(struct nsset *nsset, struct ns_common *ns)
> > > > {
> > > > - return ns->ops->install(nsset, ns);
> > > > + int ret;
> > > > +
> > > > + ret = ns->ops->install(nsset, ns);
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + return security_namespace_install(nsset, ns);
> > > > }
> > >
> > > Do we also want a security_namespace_switch() called from within
> > > switch_task_namespaces()? Of course LSMs would not be able to fail or
> > > return an error at that point, but it seems reasonable that LSMs might
> > > want to update LSM state associated with the current task once the
> > > namespaces have been changed. This is similar to all the "_post_" LSM
> > > hooks we have for various operations in the VFS and network layers.
> >
> > What cannot be infered from security_namespace_install()?
>
> We don't actually know if the namespace is attached to a process until
> we get to switch_task_namespaces().
>
> Now that I'm looking at this again, why is the
> security_namespace_install() call placed after the ns->ops->install()
> call? From an access control perspective we want the LSM hook before
See https://lore.kernel.org/20260325-filmverleih-auffressen-e897fcf8d3f2@brauner
where I requested the order to be changed.
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: Kumar Kartikeya Dwivedi @ 2026-04-27 14:33 UTC (permalink / raw)
To: Song Liu
Cc: Matt Bobrowski, David Windsor, Alexander Viro, Christian Brauner,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, KP Singh, Paul Moore, James Morris,
Serge E. Hallyn, Jan Kara, John Fastabend, Martin KaFai Lau,
Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf,
linux-security-module
In-Reply-To: <CAPhsuW60SRXZbFYRDm-QaTiB8tTtJD_N4jk1=680x5UsZmpj9w@mail.gmail.com>
On Mon, 27 Apr 2026 at 16:21, Song Liu <song@kernel.org> wrote:
>
> On Mon, Apr 27, 2026 at 11:11 AM Matt Bobrowski
> <mattbobrowski@google.com> wrote:
> >
> > On Mon, Apr 27, 2026 at 05:32:47AM +0200, Kumar Kartikeya Dwivedi wrote:
> > > On Mon, 27 Apr 2026 at 05:24, David Windsor <dwindsor@gmail.com> wrote:
> > > >
> > > > On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi
> > > > <memxor@gmail.com> wrote:
> > > > >
> > > > > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote:
> > > > > >
> > > > > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set
> > > > > > xattrs via inode_init_security hook using lsm_get_xattr_slot().
> > > > > >
> > > > > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF
> > > > > > programs cannot do: hook arguments are not directly writable from BPF.
> > > > > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name,
> > > > > > value), and the verifier transparently rewrites each call into
> > > > > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and
> > > > > > xattr_count are extracted from the hook context, which the verifier
> > > > > > spills to the stack at program entry since R1 is clobbered during normal
> > > > > > execution.
> > > > > >
> > > > > > A previous attempt [1] required a kmalloc string output protocol for
> > > > > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to
> > > > > > provide xattrs for inode_init_security hook") [2], the xattr name is no
> > > > > > longer allocated; it is a static constant. We take advantage of this by
> > > > > > passing the name directly. Because we rely on the hook-specific ctx
> > > > > > layout, the kfunc is restricted to lsm/inode_init_security.
> > > > > >
> > > > > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1]
> > > > > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2]
> > > > > > Suggested-by: Song Liu <song@kernel.org>
> > > > > > Signed-off-by: David Windsor <dwindsor@gmail.com>
> > > > > > ---
> > > > >
> > > > > The explanation and code make no sense to me. Why not pass xattrs and
> > > > > xattr_count directly as arguments, even if you choose to restrict the
> > > > > kfunc to a specific hook? Why does the verifier core need the hack to
> > > > > spill the context and extract the two arguments?
> > > > >
> > > >
> > > > xattr_count is an output parameter; we cannot currently write to it in
> > > > bpf as there is no verifier support for writing to int *. xattrs and
> > > > xattr_count can be fixed up by the verifier, so we only require the
> > > > user to pass the name and value.
> > >
> > > Sure, but the kfunc can. Did you try passing them in directly?
> > > If that doesn't work for some reason, we should fix it instead.
> >
> > Hm, perhaps this fixup approach might be the simplest in order to
> > assure the needed safety?
>
> +1. I think this is the best approach I can think of.
We're not going to add more and more special cases to the verifier.
The whole approach is unscalable.
If the concern is that int xattr_count passed for xattrs can be
unrelated int pointer obtained from elsewhere, can we pack the xattrs
and xattr_count into a struct and pass it as an argument to the LSM?
Then the pair struct can be passed in directly, ensuring both
originate from the arguments passed to the LSM. That should eliminate
concerns about either being out of sync if obtained from different
sources.
Even if we wanted to ensure argument provenance was stuff loaded from
context, the right solution would be some kfunc flag that constraints
the argument to be derived by following the ctx pointer, not whatever
is done in this patch.
>
> Thanks,
> Song
>
> [...]
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: Song Liu @ 2026-04-27 14:20 UTC (permalink / raw)
To: Matt Bobrowski
Cc: Kumar Kartikeya Dwivedi, David Windsor, Alexander Viro,
Christian Brauner, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Eduard Zingerman, KP Singh, Paul Moore,
James Morris, Serge E. Hallyn, Jan Kara, John Fastabend,
Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel,
linux-kernel, bpf, linux-security-module
In-Reply-To: <ae82Vv0RWzcXqSaz@google.com>
On Mon, Apr 27, 2026 at 11:11 AM Matt Bobrowski
<mattbobrowski@google.com> wrote:
>
> On Mon, Apr 27, 2026 at 05:32:47AM +0200, Kumar Kartikeya Dwivedi wrote:
> > On Mon, 27 Apr 2026 at 05:24, David Windsor <dwindsor@gmail.com> wrote:
> > >
> > > On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi
> > > <memxor@gmail.com> wrote:
> > > >
> > > > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote:
> > > > >
> > > > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set
> > > > > xattrs via inode_init_security hook using lsm_get_xattr_slot().
> > > > >
> > > > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF
> > > > > programs cannot do: hook arguments are not directly writable from BPF.
> > > > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name,
> > > > > value), and the verifier transparently rewrites each call into
> > > > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and
> > > > > xattr_count are extracted from the hook context, which the verifier
> > > > > spills to the stack at program entry since R1 is clobbered during normal
> > > > > execution.
> > > > >
> > > > > A previous attempt [1] required a kmalloc string output protocol for
> > > > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to
> > > > > provide xattrs for inode_init_security hook") [2], the xattr name is no
> > > > > longer allocated; it is a static constant. We take advantage of this by
> > > > > passing the name directly. Because we rely on the hook-specific ctx
> > > > > layout, the kfunc is restricted to lsm/inode_init_security.
> > > > >
> > > > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1]
> > > > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2]
> > > > > Suggested-by: Song Liu <song@kernel.org>
> > > > > Signed-off-by: David Windsor <dwindsor@gmail.com>
> > > > > ---
> > > >
> > > > The explanation and code make no sense to me. Why not pass xattrs and
> > > > xattr_count directly as arguments, even if you choose to restrict the
> > > > kfunc to a specific hook? Why does the verifier core need the hack to
> > > > spill the context and extract the two arguments?
> > > >
> > >
> > > xattr_count is an output parameter; we cannot currently write to it in
> > > bpf as there is no verifier support for writing to int *. xattrs and
> > > xattr_count can be fixed up by the verifier, so we only require the
> > > user to pass the name and value.
> >
> > Sure, but the kfunc can. Did you try passing them in directly?
> > If that doesn't work for some reason, we should fix it instead.
>
> Hm, perhaps this fixup approach might be the simplest in order to
> assure the needed safety?
+1. I think this is the best approach I can think of.
Thanks,
Song
[...]
^ permalink raw reply
* [syzbot] [integrity?] [lsm?] WARNING: bad unlock balance in __filemap_add_folio
From: syzbot @ 2026-04-27 13:36 UTC (permalink / raw)
To: dmitry.kasatkin, eric.snowberg, jmorris, linux-integrity,
linux-kernel, linux-security-module, paul, roberto.sassu, serge,
syzkaller-bugs, zohar
Hello,
syzbot found the following issue on:
HEAD commit: 2e6803928193 Merge tag 'tracefs-v7.1-2' of git://git.kerne..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=117dff16580000
kernel config: https://syzkaller.appspot.com/x/.config?x=80b28e8d6ef9384a
dashboard link: https://syzkaller.appspot.com/bug?extid=914bc925a90b7e137017
compiler: Debian clang version 21.1.8 (++20251221033036+2078da43e25a-1~exp1~20251221153213.50), Debian LLD 21.1.8
Unfortunately, I don't have any reproducer for this issue yet.
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/690094a31275/disk-2e680392.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/7d17ea4e1f81/vmlinux-2e680392.xz
kernel image: https://storage.googleapis.com/syzbot-assets/c1478f49f523/bzImage-2e680392.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+914bc925a90b7e137017@syzkaller.appspotmail.com
cgroup: Unknown subsys name 'cpuset'
cgroup: Unknown subsys name 'rlimit'
=====================================
WARNING: bad unlock balance detected!
syzkaller #0 Not tainted
-------------------------------------
syz-executor/5795 is trying to release lock (rcu_read_lock) at:
[<ffffffff8b2f32cf>] rcu_lock_release include/linux/rcupdate.h:310 [inline]
[<ffffffff8b2f32cf>] rcu_read_unlock include/linux/rcupdate.h:869 [inline]
[<ffffffff8b2f32cf>] rt_spin_unlock+0x14f/0x200 kernel/locking/spinlock_rt.c:82
but there are no more locks to release!
other info that might help us debug this:
2 locks held by syz-executor/5795:
#0: ffff888035e50f58 (&ima_iint_mutex_key[depth]){+.+.}-{4:4}, at: process_measurement+0x7fd/0x1c90 security/integrity/ima/ima_main.c:319
#1: ffff8880434dc100 (mapping.invalidate_lock#2){++++}-{4:4}, at: filemap_invalidate_lock_shared include/linux/fs.h:1094 [inline]
#1: ffff8880434dc100 (mapping.invalidate_lock#2){++++}-{4:4}, at: do_page_cache_ra mm/readahead.c:333 [inline]
#1: ffff8880434dc100 (mapping.invalidate_lock#2){++++}-{4:4}, at: page_cache_ra_order+0x2a5/0x490 mm/readahead.c:538
stack backtrace:
CPU: 1 UID: 0 PID: 5795 Comm: syz-executor Not tainted syzkaller #0 PREEMPT_{RT,(full)}
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/18/2026
Call Trace:
<TASK>
dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
print_unlock_imbalance_bug+0xdc/0xf0 kernel/locking/lockdep.c:5298
__lock_release kernel/locking/lockdep.c:5537 [inline]
lock_release+0x248/0x3c0 kernel/locking/lockdep.c:5889
rcu_lock_release include/linux/rcupdate.h:310 [inline]
rcu_read_unlock include/linux/rcupdate.h:869 [inline]
rt_spin_unlock+0x15b/0x200 kernel/locking/spinlock_rt.c:82
spin_unlock_irq include/linux/spinlock_rt.h:122 [inline]
__filemap_add_folio+0xc85/0x1200 mm/filemap.c:931
filemap_add_folio+0x2de/0x610 mm/filemap.c:967
page_cache_ra_unbounded+0x407/0x980 mm/readahead.c:282
do_page_cache_ra mm/readahead.c:334 [inline]
page_cache_ra_order+0x2b5/0x490 mm/readahead.c:538
filemap_readahead mm/filemap.c:2664 [inline]
filemap_get_pages+0x832/0x1e70 mm/filemap.c:2710
filemap_read+0x44a/0x1240 mm/filemap.c:2806
__kernel_read+0x50d/0x9c0 fs/read_write.c:532
integrity_kernel_read+0x89/0xd0 security/integrity/iint.c:28
ima_calc_file_hash_tfm security/integrity/ima/ima_crypto.c:222 [inline]
ima_calc_file_hash+0x452/0x870 security/integrity/ima/ima_crypto.c:280
ima_collect_measurement+0x523/0x9d0 security/integrity/ima/ima_api.c:300
process_measurement+0x12d9/0x1c90 security/integrity/ima/ima_main.c:425
ima_file_check+0xe1/0x130 security/integrity/ima/ima_main.c:685
security_file_post_open+0xb3/0x260 security/security.c:2755
do_open fs/namei.c:4701 [inline]
path_openat+0x2e88/0x38a0 fs/namei.c:4858
do_file_open+0x23e/0x4a0 fs/namei.c:4887
file_open_name+0x162/0x1c0 fs/open.c:1322
__do_sys_swapon mm/swapfile.c:3467 [inline]
__se_sys_swapon+0x856/0x2010 mm/swapfile.c:3432
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x15f/0xf80 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f884264c7d7
Code: 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 a7 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffe6306a658 EFLAGS: 00000246 ORIG_RAX: 00000000000000a7
RAX: ffffffffffffffda RBX: 0000000000000008 RCX: 00007f884264c7d7
RDX: 0000000000000000 RSI: 0000000000008000 RDI: 00007f88426e2e5b
RBP: 00007f88426e2e5b R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000008 R11: 0000000000000246 R12: 00007f88428963e0
R13: 00007f88426fdd26 R14: 0000000000200000 R15: 00007f88428963a0
</TASK>
------------[ cut here ]------------
rrln < 0 || rrln > RCU_NEST_PMAX
WARNING: kernel/rcu/tree_plugin.h:443 at __rcu_read_unlock+0x79/0xe0 kernel/rcu/tree_plugin.h:443, CPU#1: syz-executor/5795
Modules linked in:
CPU: 1 UID: 0 PID: 5795 Comm: syz-executor Not tainted syzkaller #0 PREEMPT_{RT,(full)}
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/18/2026
RIP: 0010:__rcu_read_unlock+0x79/0xe0 kernel/rcu/tree_plugin.h:443
Code: 75 66 41 83 3e 00 75 27 43 0f b6 04 3c 84 c0 75 41 8b 03 3d 00 00 00 40 73 0f 5b 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc cc 90 <0f> 0b 90 eb eb e8 6d 00 00 00 eb d2 89 d9 80 e1 07 80 c1 03 38 c1
RSP: 0018:ffffc900046e6418 EFLAGS: 00010286
RAX: 00000000ffffffff RBX: ffff888039e82384 RCX: 0000000000000046
RDX: 0000000000000000 RSI: ffffffff8d8986dc RDI: ffff888039e81ec0
RBP: 0000000000000001 R08: 0000000000000000 R09: 0000000000000000
R10: dffffc0000000000 R11: fffffbfff1bcaacc R12: 1ffff110073d0470
R13: ffff888039e81ec0 R14: ffff8880b893c610 R15: dffffc0000000000
FS: 000055555b61b540(0000) GS:ffff8881261fb000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007fec05db0e9c CR3: 0000000042cbe000 CR4: 00000000003526f0
Call Trace:
<TASK>
rcu_read_unlock include/linux/rcupdate.h:871 [inline]
rt_spin_unlock+0x160/0x200 kernel/locking/spinlock_rt.c:82
spin_unlock_irq include/linux/spinlock_rt.h:122 [inline]
__filemap_add_folio+0xc85/0x1200 mm/filemap.c:931
filemap_add_folio+0x2de/0x610 mm/filemap.c:967
page_cache_ra_unbounded+0x407/0x980 mm/readahead.c:282
do_page_cache_ra mm/readahead.c:334 [inline]
page_cache_ra_order+0x2b5/0x490 mm/readahead.c:538
filemap_readahead mm/filemap.c:2664 [inline]
filemap_get_pages+0x832/0x1e70 mm/filemap.c:2710
filemap_read+0x44a/0x1240 mm/filemap.c:2806
__kernel_read+0x50d/0x9c0 fs/read_write.c:532
integrity_kernel_read+0x89/0xd0 security/integrity/iint.c:28
ima_calc_file_hash_tfm security/integrity/ima/ima_crypto.c:222 [inline]
ima_calc_file_hash+0x452/0x870 security/integrity/ima/ima_crypto.c:280
ima_collect_measurement+0x523/0x9d0 security/integrity/ima/ima_api.c:300
process_measurement+0x12d9/0x1c90 security/integrity/ima/ima_main.c:425
ima_file_check+0xe1/0x130 security/integrity/ima/ima_main.c:685
security_file_post_open+0xb3/0x260 security/security.c:2755
do_open fs/namei.c:4701 [inline]
path_openat+0x2e88/0x38a0 fs/namei.c:4858
do_file_open+0x23e/0x4a0 fs/namei.c:4887
file_open_name+0x162/0x1c0 fs/open.c:1322
__do_sys_swapon mm/swapfile.c:3467 [inline]
__se_sys_swapon+0x856/0x2010 mm/swapfile.c:3432
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x15f/0xf80 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f884264c7d7
Code: 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 b8 a7 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffe6306a658 EFLAGS: 00000246 ORIG_RAX: 00000000000000a7
RAX: ffffffffffffffda RBX: 0000000000000008 RCX: 00007f884264c7d7
RDX: 0000000000000000 RSI: 0000000000008000 RDI: 00007f88426e2e5b
RBP: 00007f88426e2e5b R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000008 R11: 0000000000000246 R12: 00007f88428963e0
R13: 00007f88426fdd26 R14: 0000000000200000 R15: 00007f88428963a0
</TASK>
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: Matt Bobrowski @ 2026-04-27 10:11 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: David Windsor, Alexander Viro, Christian Brauner,
Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, KP Singh, Paul Moore, James Morris,
Serge E. Hallyn, Song Liu, Jan Kara, John Fastabend,
Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel,
linux-kernel, bpf, linux-security-module
In-Reply-To: <CAP01T74WBjcMXbWYeaWB-oTk-bUzoxi_EEALDpT1hFUEOovnpw@mail.gmail.com>
On Mon, Apr 27, 2026 at 05:32:47AM +0200, Kumar Kartikeya Dwivedi wrote:
> On Mon, 27 Apr 2026 at 05:24, David Windsor <dwindsor@gmail.com> wrote:
> >
> > On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi
> > <memxor@gmail.com> wrote:
> > >
> > > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote:
> > > >
> > > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set
> > > > xattrs via inode_init_security hook using lsm_get_xattr_slot().
> > > >
> > > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF
> > > > programs cannot do: hook arguments are not directly writable from BPF.
> > > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name,
> > > > value), and the verifier transparently rewrites each call into
> > > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and
> > > > xattr_count are extracted from the hook context, which the verifier
> > > > spills to the stack at program entry since R1 is clobbered during normal
> > > > execution.
> > > >
> > > > A previous attempt [1] required a kmalloc string output protocol for
> > > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to
> > > > provide xattrs for inode_init_security hook") [2], the xattr name is no
> > > > longer allocated; it is a static constant. We take advantage of this by
> > > > passing the name directly. Because we rely on the hook-specific ctx
> > > > layout, the kfunc is restricted to lsm/inode_init_security.
> > > >
> > > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1]
> > > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2]
> > > > Suggested-by: Song Liu <song@kernel.org>
> > > > Signed-off-by: David Windsor <dwindsor@gmail.com>
> > > > ---
> > >
> > > The explanation and code make no sense to me. Why not pass xattrs and
> > > xattr_count directly as arguments, even if you choose to restrict the
> > > kfunc to a specific hook? Why does the verifier core need the hack to
> > > spill the context and extract the two arguments?
> > >
> >
> > xattr_count is an output parameter; we cannot currently write to it in
> > bpf as there is no verifier support for writing to int *. xattrs and
> > xattr_count can be fixed up by the verifier, so we only require the
> > user to pass the name and value.
>
> Sure, but the kfunc can. Did you try passing them in directly?
> If that doesn't work for some reason, we should fix it instead.
Hm, perhaps this fixup approach might be the simplest in order to
assure the needed safety? Allowing this new BPF kfunc to take xattr
and xattr_count arguments directly from the BPF LSM program could
possibly result in lsm_get_xattr_slot() to go off the rails, no?
Unless, what you're proposing here is to also add some provenance-like
tracking such that the BPF verifier assures that only the BPF LSM
program context arguments end up being supplied to it (if this is
something that doesn't already exist)?.
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: Matt Bobrowski @ 2026-04-27 9:48 UTC (permalink / raw)
To: David Windsor
Cc: Alexander Viro, Christian Brauner, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, KP Singh, Paul Moore, James Morris,
Serge E. Hallyn, Song Liu, Jan Kara, John Fastabend,
Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel,
linux-kernel, bpf, linux-security-module
In-Reply-To: <20260427001602.38353-2-dwindsor@gmail.com>
On Sun, Apr 26, 2026 at 08:15:57PM -0400, David Windsor wrote:
> Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set
> xattrs via inode_init_security hook using lsm_get_xattr_slot().
>
> lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF
> programs cannot do: hook arguments are not directly writable from BPF.
> To hide this, the BPF-facing API is just bpf_init_inode_xattr(name,
> value), and the verifier transparently rewrites each call into
> bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and
> xattr_count are extracted from the hook context, which the verifier
> spills to the stack at program entry since R1 is clobbered during normal
> execution.
>
> A previous attempt [1] required a kmalloc string output protocol for
> the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to
> provide xattrs for inode_init_security hook") [2], the xattr name is no
> longer allocated; it is a static constant. We take advantage of this by
> passing the name directly. Because we rely on the hook-specific ctx
> layout, the kfunc is restricted to lsm/inode_init_security.
>
> Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1]
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2]
> Suggested-by: Song Liu <song@kernel.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>
> ---
> fs/bpf_fs_kfuncs.c | 80 +++++++++++++++++++++++++++++++++++-
> include/linux/bpf_verifier.h | 3 ++
> kernel/bpf/fixups.c | 20 +++++++++
> kernel/bpf/verifier.c | 54 ++++++++++++++++++++++++
> security/bpf/hooks.c | 3 ++
> 5 files changed, 159 insertions(+), 1 deletion(-)
>
> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> index 9d27be058494..5a5951006a3f 100644
> --- a/fs/bpf_fs_kfuncs.c
> +++ b/fs/bpf_fs_kfuncs.c
> @@ -10,6 +10,7 @@
> #include <linux/fsnotify.h>
> #include <linux/file.h>
> #include <linux/kernfs.h>
> +#include <linux/lsm_hooks.h>
> #include <linux/mm.h>
> #include <linux/xattr.h>
>
> @@ -353,6 +354,68 @@ __bpf_kfunc int bpf_cgroup_read_xattr(struct cgroup *cgroup, const char *name__s
> }
> #endif /* CONFIG_CGROUPS */
>
> +/* Called from the verifier fixup of bpf_init_inode_xattr(). */
> +__bpf_kfunc int bpf_init_inode_xattr_impl(struct xattr *xattrs, int *xattr_count,
> + const char *name__str,
> + const struct bpf_dynptr *value_p)
> +{
> + struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p;
> + size_t name_len;
> + void *xattr_value;
> + struct xattr *xattr;
> + const void *value;
> + u32 value_len;
> +
> + if (!xattrs || !xattr_count || !name__str)
> + return -EINVAL;
> +
> + name_len = strlen(name__str);
> + if (name_len == 0 || name_len > XATTR_NAME_MAX)
> + return -EINVAL;
> +
> + value_len = __bpf_dynptr_size(value_ptr);
> + if (value_len == 0 || value_len > XATTR_SIZE_MAX)
> + return -EINVAL;
> +
> + value = __bpf_dynptr_data(value_ptr, value_len);
> + if (!value)
> + return -EINVAL;
> +
> + xattr_value = kmemdup(value, value_len, GFP_ATOMIC);
> + if (!xattr_value)
> + return -ENOMEM;
> +
> + xattr = lsm_get_xattr_slot(xattrs, xattr_count);
> + if (!xattr) {
> + kfree(xattr_value);
> + return -ENOSPC;
> + }
I think you should also include the following check:
if (!match_security_bpf_prefix(name__str))
return -EPERM;
This will ensure that some namespace isolation is provided and make
the behavior of this initialization based BPF kfunc consistent with
the pre-existing runtime xattr-related modification BPF kfuncs (e.g.,
bpf_set_dentry_xattr()).
> + xattr->name = name__str;
> + xattr->value = xattr_value;
> + xattr->value_len = value_len;
> +
> + return 0;
> +}
> +
> +/**
> + * bpf_init_inode_xattr - set an xattr on a new inode from inode_init_security
> + * @name__str: xattr name (e.g., "bpf.file_label")
> + * @value_p: dynptr containing the xattr value
> + *
> + * Only callable from lsm/inode_init_security programs. The verifier rewrites
> + * calls to bpf_init_inode_xattr_impl() with xattrs/xattr_count extracted from
> + * the hook context.
> + *
> + * Return: 0 on success, negative error on failure.
> + */
> +__bpf_kfunc int bpf_init_inode_xattr(const char *name__str,
> + const struct bpf_dynptr *value_p)
> +{
> + WARN_ONCE(1, "%s called without verifier fixup\n", __func__);
> + return -EFAULT;
> +}
> +
> __bpf_kfunc_end_defs();
>
> BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
> @@ -363,13 +426,28 @@ BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE)
> BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE)
> BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE)
> BTF_ID_FLAGS(func, bpf_remove_dentry_xattr, KF_SLEEPABLE)
> +BTF_ID_FLAGS(func, bpf_init_inode_xattr)
> +BTF_ID_FLAGS(func, bpf_init_inode_xattr_impl)
> BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)
>
> +BTF_ID_LIST(bpf_lsm_inode_init_security_btf_ids)
> +BTF_ID(func, bpf_lsm_inode_init_security)
> +
> +BTF_ID_LIST(bpf_init_inode_xattr_btf_ids)
> +BTF_ID(func, bpf_init_inode_xattr)
> +BTF_ID(func, bpf_init_inode_xattr_impl)
> +
> static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id)
> {
> if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) ||
> - prog->type == BPF_PROG_TYPE_LSM)
> + prog->type == BPF_PROG_TYPE_LSM) {
> + /* bpf_init_inode_xattr[_impl] only attach to inode_init_security. */
> + if ((kfunc_id == bpf_init_inode_xattr_btf_ids[0] ||
> + kfunc_id == bpf_init_inode_xattr_btf_ids[1]) &&
> + prog->aux->attach_btf_id != bpf_lsm_inode_init_security_btf_ids[0])
> + return -EACCES;
> return 0;
> + }
> return -EACCES;
> }
>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 101ca6cc5424..e73bb2222c3d 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -682,6 +682,7 @@ struct bpf_insn_aux_data {
> */
> u8 fastcall_spills_num:3;
> u8 arg_prog:4;
> + u8 init_inode_xattr_fixup:1;
>
> /* below fields are initialized once */
> unsigned int orig_idx; /* original instruction index */
> @@ -903,6 +904,8 @@ struct bpf_verifier_env {
> bool bypass_spec_v4;
> bool seen_direct_write;
> bool seen_exception;
> + bool needs_ctx_spill;
> + s16 ctx_stack_off;
> struct bpf_insn_aux_data *insn_aux_data; /* array of per-insn state */
> const struct bpf_line_info *prev_linfo;
> struct bpf_verifier_log log;
> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index fba9e8c00878..18d612a9fe29 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -725,6 +725,26 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
> }
> }
>
> + if (env->needs_ctx_spill) {
> + if (epilogue_cnt) {
> + /* gen_epilogue already saved ctx to the stack */
> + env->ctx_stack_off = -(s16)subprogs[0].stack_depth;
> + } else {
> + cnt = 0;
> + subprogs[0].stack_depth += 8;
> + env->ctx_stack_off = -(s16)subprogs[0].stack_depth;
> + insn_buf[cnt++] = BPF_STX_MEM(BPF_DW, BPF_REG_FP,
> + BPF_REG_1,
> + env->ctx_stack_off);
> + insn_buf[cnt++] = env->prog->insnsi[0];
> + new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt);
> + if (!new_prog)
> + return -ENOMEM;
> + env->prog = new_prog;
> + delta += cnt - 1;
> + }
> + }
> +
> if (ops->gen_prologue || env->seen_direct_write) {
> if (!ops->gen_prologue) {
> verifier_bug(env, "gen_prologue is null");
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 03f9e16c2abe..af5753ffb16b 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -10794,6 +10794,8 @@ enum special_kfunc_type {
> KF_bpf_arena_alloc_pages,
> KF_bpf_arena_free_pages,
> KF_bpf_arena_reserve_pages,
> + KF_bpf_init_inode_xattr,
> + KF_bpf_init_inode_xattr_impl,
> KF_bpf_session_is_return,
> KF_bpf_stream_vprintk,
> KF_bpf_stream_print_stack,
> @@ -10882,6 +10884,13 @@ BTF_ID(func, bpf_task_work_schedule_resume)
> BTF_ID(func, bpf_arena_alloc_pages)
> BTF_ID(func, bpf_arena_free_pages)
> BTF_ID(func, bpf_arena_reserve_pages)
> +#ifdef CONFIG_BPF_LSM
> +BTF_ID(func, bpf_init_inode_xattr)
> +BTF_ID(func, bpf_init_inode_xattr_impl)
> +#else
> +BTF_ID_UNUSED
> +BTF_ID_UNUSED
> +#endif
> BTF_ID(func, bpf_session_is_return)
> BTF_ID(func, bpf_stream_vprintk)
> BTF_ID(func, bpf_stream_print_stack)
> @@ -12701,6 +12710,24 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> if (err < 0)
> return err;
>
> + if (meta.func_id == special_kfunc_list[KF_bpf_init_inode_xattr_impl]) {
> + verbose(env, "bpf_init_inode_xattr_impl is not callable directly\n");
> + return -EACCES;
> + }
> +
> + if (meta.func_id == special_kfunc_list[KF_bpf_init_inode_xattr]) {
> + if (env->cur_state->curframe != 0) {
> + verbose(env, "bpf_init_inode_xattr cannot be called from subprograms\n");
> + return -EINVAL;
> + }
> + env->needs_ctx_spill = true;
> + insn_aux->init_inode_xattr_fixup = true;
> + err = bpf_add_kfunc_call(env,
> + special_kfunc_list[KF_bpf_init_inode_xattr_impl], 0);
> + if (err < 0)
> + return err;
> + }
> +
> if (is_bpf_rbtree_add_kfunc(meta.func_id)) {
> err = push_callback_call(env, insn, insn_idx, meta.subprogno,
> set_rbtree_add_callback_state);
> @@ -19272,6 +19299,33 @@ int bpf_fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> insn_buf[4] = BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1);
> insn_buf[5] = BPF_ALU64_IMM(BPF_NEG, BPF_REG_0, 0);
> *cnt = 6;
> + } else if (env->insn_aux_data[insn_idx].init_inode_xattr_fixup) {
> + struct bpf_kfunc_desc *impl_desc;
> +
> + impl_desc = find_kfunc_desc(env->prog,
> + special_kfunc_list[KF_bpf_init_inode_xattr_impl], 0);
> + if (!impl_desc) {
> + verifier_bug(env, "bpf_init_inode_xattr_impl desc not found");
> + return -EFAULT;
> + }
> +
> + /* Rewrite bpf_init_inode_xattr(name, value) to inject xattrs and
> + * xattr_count loaded from the saved inode_init_security ctx.
> + */
> + insn_buf[0] = BPF_MOV64_REG(BPF_REG_3, BPF_REG_1);
> + insn_buf[1] = BPF_MOV64_REG(BPF_REG_4, BPF_REG_2);
> + insn_buf[2] = BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_FP,
> + env->ctx_stack_off);
> + insn_buf[3] = BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_2,
> + 3 * sizeof(u64));
> + insn_buf[4] = BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_2,
> + 4 * sizeof(u64));
> + insn_buf[5] = *insn;
> + if (!bpf_jit_supports_far_kfunc_call())
> + insn_buf[5].imm = BPF_CALL_IMM(impl_desc->addr);
> + else
> + insn_buf[5].imm = impl_desc->func_id;
> + *cnt = 6;
> }
>
> if (env->insn_aux_data[insn_idx].arg_prog) {
> diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
> index 40efde233f3a..1e61baa821bd 100644
> --- a/security/bpf/hooks.c
> +++ b/security/bpf/hooks.c
> @@ -28,8 +28,11 @@ static int __init bpf_lsm_init(void)
> return 0;
> }
>
> +#define BPF_LSM_INODE_INIT_XATTRS 1
> +
> struct lsm_blob_sizes bpf_lsm_blob_sizes __ro_after_init = {
> .lbs_inode = sizeof(struct bpf_storage_blob),
> + .lbs_xattr_count = BPF_LSM_INODE_INIT_XATTRS,
> };
>
> DEFINE_LSM(bpf) = {
> --
> 2.53.0
>
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: David Windsor @ 2026-04-27 3:42 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: Alexander Viro, Christian Brauner, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh,
Matt Bobrowski, Paul Moore, James Morris, Serge E. Hallyn,
Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau,
Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf,
linux-security-module
In-Reply-To: <CAP01T74WBjcMXbWYeaWB-oTk-bUzoxi_EEALDpT1hFUEOovnpw@mail.gmail.com>
On Sun, Apr 26, 2026 at 11:33 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Mon, 27 Apr 2026 at 05:24, David Windsor <dwindsor@gmail.com> wrote:
> >
> > On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi
> > <memxor@gmail.com> wrote:
> > >
> > > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote:
> > > >
> > > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set
> > > > xattrs via inode_init_security hook using lsm_get_xattr_slot().
> > > >
> > > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF
> > > > programs cannot do: hook arguments are not directly writable from BPF.
> > > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name,
> > > > value), and the verifier transparently rewrites each call into
> > > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and
> > > > xattr_count are extracted from the hook context, which the verifier
> > > > spills to the stack at program entry since R1 is clobbered during normal
> > > > execution.
> > > >
> > > > A previous attempt [1] required a kmalloc string output protocol for
> > > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to
> > > > provide xattrs for inode_init_security hook") [2], the xattr name is no
> > > > longer allocated; it is a static constant. We take advantage of this by
> > > > passing the name directly. Because we rely on the hook-specific ctx
> > > > layout, the kfunc is restricted to lsm/inode_init_security.
> > > >
> > > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1]
> > > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2]
> > > > Suggested-by: Song Liu <song@kernel.org>
> > > > Signed-off-by: David Windsor <dwindsor@gmail.com>
> > > > ---
> > >
> > > The explanation and code make no sense to me. Why not pass xattrs and
> > > xattr_count directly as arguments, even if you choose to restrict the
> > > kfunc to a specific hook? Why does the verifier core need the hack to
> > > spill the context and extract the two arguments?
> > >
> >
> > xattr_count is an output parameter; we cannot currently write to it in
> > bpf as there is no verifier support for writing to int *. xattrs and
> > xattr_count can be fixed up by the verifier, so we only require the
> > user to pass the name and value.
>
> Sure, but the kfunc can. Did you try passing them in directly?
> If that doesn't work for some reason, we should fix it instead.
>
Aha yes, I did try that and I believe the verifier rejected it because
btf_ctx_access converts the pointer to a scalar. Will confirm the
exact error tomorrow.
> >
> > > pw-bot: cr
> > >
> > > > [...]
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: Kumar Kartikeya Dwivedi @ 2026-04-27 3:32 UTC (permalink / raw)
To: David Windsor
Cc: Alexander Viro, Christian Brauner, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh,
Matt Bobrowski, Paul Moore, James Morris, Serge E. Hallyn,
Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau,
Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf,
linux-security-module
In-Reply-To: <CAEXv5_heSFK5rZYtaJG70Xgv92Um6Dbq8b95PtnpKy7XFwpKUw@mail.gmail.com>
On Mon, 27 Apr 2026 at 05:24, David Windsor <dwindsor@gmail.com> wrote:
>
> On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi
> <memxor@gmail.com> wrote:
> >
> > On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote:
> > >
> > > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set
> > > xattrs via inode_init_security hook using lsm_get_xattr_slot().
> > >
> > > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF
> > > programs cannot do: hook arguments are not directly writable from BPF.
> > > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name,
> > > value), and the verifier transparently rewrites each call into
> > > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and
> > > xattr_count are extracted from the hook context, which the verifier
> > > spills to the stack at program entry since R1 is clobbered during normal
> > > execution.
> > >
> > > A previous attempt [1] required a kmalloc string output protocol for
> > > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to
> > > provide xattrs for inode_init_security hook") [2], the xattr name is no
> > > longer allocated; it is a static constant. We take advantage of this by
> > > passing the name directly. Because we rely on the hook-specific ctx
> > > layout, the kfunc is restricted to lsm/inode_init_security.
> > >
> > > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1]
> > > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2]
> > > Suggested-by: Song Liu <song@kernel.org>
> > > Signed-off-by: David Windsor <dwindsor@gmail.com>
> > > ---
> >
> > The explanation and code make no sense to me. Why not pass xattrs and
> > xattr_count directly as arguments, even if you choose to restrict the
> > kfunc to a specific hook? Why does the verifier core need the hack to
> > spill the context and extract the two arguments?
> >
>
> xattr_count is an output parameter; we cannot currently write to it in
> bpf as there is no verifier support for writing to int *. xattrs and
> xattr_count can be fixed up by the verifier, so we only require the
> user to pass the name and value.
Sure, but the kfunc can. Did you try passing them in directly?
If that doesn't work for some reason, we should fix it instead.
>
> > pw-bot: cr
> >
> > > [...]
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: David Windsor @ 2026-04-27 3:23 UTC (permalink / raw)
To: Kumar Kartikeya Dwivedi
Cc: Alexander Viro, Christian Brauner, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh,
Matt Bobrowski, Paul Moore, James Morris, Serge E. Hallyn,
Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau,
Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf,
linux-security-module
In-Reply-To: <CAP01T74iuSDJeL6g=5yLdYGb-VcESK1SYY3R1S1r-CtQEsW+oQ@mail.gmail.com>
On Sun, Apr 26, 2026 at 10:57 PM Kumar Kartikeya Dwivedi
<memxor@gmail.com> wrote:
>
> On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote:
> >
> > Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set
> > xattrs via inode_init_security hook using lsm_get_xattr_slot().
> >
> > lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF
> > programs cannot do: hook arguments are not directly writable from BPF.
> > To hide this, the BPF-facing API is just bpf_init_inode_xattr(name,
> > value), and the verifier transparently rewrites each call into
> > bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and
> > xattr_count are extracted from the hook context, which the verifier
> > spills to the stack at program entry since R1 is clobbered during normal
> > execution.
> >
> > A previous attempt [1] required a kmalloc string output protocol for
> > the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to
> > provide xattrs for inode_init_security hook") [2], the xattr name is no
> > longer allocated; it is a static constant. We take advantage of this by
> > passing the name directly. Because we rely on the hook-specific ctx
> > layout, the kfunc is restricted to lsm/inode_init_security.
> >
> > Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1]
> > Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2]
> > Suggested-by: Song Liu <song@kernel.org>
> > Signed-off-by: David Windsor <dwindsor@gmail.com>
> > ---
>
> The explanation and code make no sense to me. Why not pass xattrs and
> xattr_count directly as arguments, even if you choose to restrict the
> kfunc to a specific hook? Why does the verifier core need the hack to
> spill the context and extract the two arguments?
>
xattr_count is an output parameter; we cannot currently write to it in
bpf as there is no verifier support for writing to int *. xattrs and
xattr_count can be fixed up by the verifier, so we only require the
user to pass the name and value.
> pw-bot: cr
>
> > [...]
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: Kumar Kartikeya Dwivedi @ 2026-04-27 2:56 UTC (permalink / raw)
To: David Windsor
Cc: Alexander Viro, Christian Brauner, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, KP Singh,
Matt Bobrowski, Paul Moore, James Morris, Serge E. Hallyn,
Song Liu, Jan Kara, John Fastabend, Martin KaFai Lau,
Yonghong Song, Jiri Olsa, linux-fsdevel, linux-kernel, bpf,
linux-security-module
In-Reply-To: <20260427001602.38353-2-dwindsor@gmail.com>
On Mon, 27 Apr 2026 at 02:16, David Windsor <dwindsor@gmail.com> wrote:
>
> Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set
> xattrs via inode_init_security hook using lsm_get_xattr_slot().
>
> lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF
> programs cannot do: hook arguments are not directly writable from BPF.
> To hide this, the BPF-facing API is just bpf_init_inode_xattr(name,
> value), and the verifier transparently rewrites each call into
> bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and
> xattr_count are extracted from the hook context, which the verifier
> spills to the stack at program entry since R1 is clobbered during normal
> execution.
>
> A previous attempt [1] required a kmalloc string output protocol for
> the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to
> provide xattrs for inode_init_security hook") [2], the xattr name is no
> longer allocated; it is a static constant. We take advantage of this by
> passing the name directly. Because we rely on the hook-specific ctx
> layout, the kfunc is restricted to lsm/inode_init_security.
>
> Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1]
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2]
> Suggested-by: Song Liu <song@kernel.org>
> Signed-off-by: David Windsor <dwindsor@gmail.com>
> ---
The explanation and code make no sense to me. Why not pass xattrs and
xattr_count directly as arguments, even if you choose to restrict the
kfunc to a specific hook? Why does the verifier core need the hack to
spill the context and extract the two arguments?
pw-bot: cr
> [...]
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: bot+bpf-ci @ 2026-04-27 0:51 UTC (permalink / raw)
To: dwindsor, viro, brauner, ast, daniel, andrii, eddyz87, memxor,
kpsingh, mattbobrowski, paul, jmorris, serge
Cc: dwindsor, song, jack, john.fastabend, martin.lau, yonghong.song,
jolsa, linux-fsdevel, linux-kernel, bpf, linux-security-module,
ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm,
ihor.solodrai
In-Reply-To: <20260427001602.38353-2-dwindsor@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1536 bytes --]
> diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
> --- a/fs/bpf_fs_kfuncs.c
> +++ b/fs/bpf_fs_kfuncs.c
[ ... ]
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -19272,6 +19299,33 @@ struct bpf_insn addr[2] = { BPF_LD_IMM64(BPF_REG_2, (long)kptr_struct_meta) };
> insn_buf[4] = BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1);
> insn_buf[5] = BPF_ALU64_IMM(BPF_NEG, BPF_REG_0, 0);
> *cnt = 6;
> + } else if (env->insn_aux_data[insn_idx].init_inode_xattr_fixup) {
> + struct bpf_kfunc_desc *impl_desc;
> +
> + impl_desc = find_kfunc_desc(env->prog,
> + special_kfunc_list[KF_bpf_init_inode_xattr_impl], 0);
> + if (!impl_desc) {
> + verifier_bug(env, "bpf_init_inode_xattr_impl desc not found");
> + return -EFAULT;
> + }
> +
> + /* Rewrite bpf_init_inode_xattr(name, value) to inject xattrs and
> + * xattr_count loaded from the saved inode_init_security ctx.
> + */
This isn't a bug, but the comment style needs adjustment. In
kernel/bpf/verifier.c, multi-line comments should have the opening /*
on its own line:
/*
* Rewrite bpf_init_inode_xattr(name, value) to inject xattrs
* and xattr_count loaded from the saved inode_init_security
* ctx.
*/
[ ... ]
---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/24970971112
^ permalink raw reply
* [PATCH bpf-next 1/2] bpf: add bpf_init_inode_xattr kfunc for atomic inode labeling
From: David Windsor @ 2026-04-27 0:15 UTC (permalink / raw)
To: Alexander Viro, Christian Brauner, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman,
Kumar Kartikeya Dwivedi, KP Singh, Matt Bobrowski, Paul Moore,
James Morris, Serge E. Hallyn
Cc: David Windsor, Song Liu, Jan Kara, John Fastabend,
Martin KaFai Lau, Yonghong Song, Jiri Olsa, linux-fsdevel,
linux-kernel, bpf, linux-security-module
In-Reply-To: <20260427001602.38353-1-dwindsor@gmail.com>
Add bpf_init_inode_xattr() kfunc for BPF LSM programs to atomically set
xattrs via inode_init_security hook using lsm_get_xattr_slot().
lsm_get_xattr_slot() claims a slot by writing to xattr_count, which BPF
programs cannot do: hook arguments are not directly writable from BPF.
To hide this, the BPF-facing API is just bpf_init_inode_xattr(name,
value), and the verifier transparently rewrites each call into
bpf_init_inode_xattr_impl(xattrs, xattr_count, name, value). xattrs and
xattr_count are extracted from the hook context, which the verifier
spills to the stack at program entry since R1 is clobbered during normal
execution.
A previous attempt [1] required a kmalloc string output protocol for
the xattr name. Since commit 6bcdfd2cac55 ("security: Allow all LSMs to
provide xattrs for inode_init_security hook") [2], the xattr name is no
longer allocated; it is a static constant. We take advantage of this by
passing the name directly. Because we rely on the hook-specific ctx
layout, the kfunc is restricted to lsm/inode_init_security.
Link: https://kernsec.org/pipermail/linux-security-module-archive/2022-October/034878.html [1]
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=6bcdfd2cac55 [2]
Suggested-by: Song Liu <song@kernel.org>
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
fs/bpf_fs_kfuncs.c | 80 +++++++++++++++++++++++++++++++++++-
include/linux/bpf_verifier.h | 3 ++
kernel/bpf/fixups.c | 20 +++++++++
kernel/bpf/verifier.c | 54 ++++++++++++++++++++++++
security/bpf/hooks.c | 3 ++
5 files changed, 159 insertions(+), 1 deletion(-)
diff --git a/fs/bpf_fs_kfuncs.c b/fs/bpf_fs_kfuncs.c
index 9d27be058494..5a5951006a3f 100644
--- a/fs/bpf_fs_kfuncs.c
+++ b/fs/bpf_fs_kfuncs.c
@@ -10,6 +10,7 @@
#include <linux/fsnotify.h>
#include <linux/file.h>
#include <linux/kernfs.h>
+#include <linux/lsm_hooks.h>
#include <linux/mm.h>
#include <linux/xattr.h>
@@ -353,6 +354,68 @@ __bpf_kfunc int bpf_cgroup_read_xattr(struct cgroup *cgroup, const char *name__s
}
#endif /* CONFIG_CGROUPS */
+/* Called from the verifier fixup of bpf_init_inode_xattr(). */
+__bpf_kfunc int bpf_init_inode_xattr_impl(struct xattr *xattrs, int *xattr_count,
+ const char *name__str,
+ const struct bpf_dynptr *value_p)
+{
+ struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p;
+ size_t name_len;
+ void *xattr_value;
+ struct xattr *xattr;
+ const void *value;
+ u32 value_len;
+
+ if (!xattrs || !xattr_count || !name__str)
+ return -EINVAL;
+
+ name_len = strlen(name__str);
+ if (name_len == 0 || name_len > XATTR_NAME_MAX)
+ return -EINVAL;
+
+ value_len = __bpf_dynptr_size(value_ptr);
+ if (value_len == 0 || value_len > XATTR_SIZE_MAX)
+ return -EINVAL;
+
+ value = __bpf_dynptr_data(value_ptr, value_len);
+ if (!value)
+ return -EINVAL;
+
+ xattr_value = kmemdup(value, value_len, GFP_ATOMIC);
+ if (!xattr_value)
+ return -ENOMEM;
+
+ xattr = lsm_get_xattr_slot(xattrs, xattr_count);
+ if (!xattr) {
+ kfree(xattr_value);
+ return -ENOSPC;
+ }
+
+ xattr->name = name__str;
+ xattr->value = xattr_value;
+ xattr->value_len = value_len;
+
+ return 0;
+}
+
+/**
+ * bpf_init_inode_xattr - set an xattr on a new inode from inode_init_security
+ * @name__str: xattr name (e.g., "bpf.file_label")
+ * @value_p: dynptr containing the xattr value
+ *
+ * Only callable from lsm/inode_init_security programs. The verifier rewrites
+ * calls to bpf_init_inode_xattr_impl() with xattrs/xattr_count extracted from
+ * the hook context.
+ *
+ * Return: 0 on success, negative error on failure.
+ */
+__bpf_kfunc int bpf_init_inode_xattr(const char *name__str,
+ const struct bpf_dynptr *value_p)
+{
+ WARN_ONCE(1, "%s called without verifier fixup\n", __func__);
+ return -EFAULT;
+}
+
__bpf_kfunc_end_defs();
BTF_KFUNCS_START(bpf_fs_kfunc_set_ids)
@@ -363,13 +426,28 @@ BTF_ID_FLAGS(func, bpf_get_dentry_xattr, KF_SLEEPABLE)
BTF_ID_FLAGS(func, bpf_get_file_xattr, KF_SLEEPABLE)
BTF_ID_FLAGS(func, bpf_set_dentry_xattr, KF_SLEEPABLE)
BTF_ID_FLAGS(func, bpf_remove_dentry_xattr, KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_init_inode_xattr)
+BTF_ID_FLAGS(func, bpf_init_inode_xattr_impl)
BTF_KFUNCS_END(bpf_fs_kfunc_set_ids)
+BTF_ID_LIST(bpf_lsm_inode_init_security_btf_ids)
+BTF_ID(func, bpf_lsm_inode_init_security)
+
+BTF_ID_LIST(bpf_init_inode_xattr_btf_ids)
+BTF_ID(func, bpf_init_inode_xattr)
+BTF_ID(func, bpf_init_inode_xattr_impl)
+
static int bpf_fs_kfuncs_filter(const struct bpf_prog *prog, u32 kfunc_id)
{
if (!btf_id_set8_contains(&bpf_fs_kfunc_set_ids, kfunc_id) ||
- prog->type == BPF_PROG_TYPE_LSM)
+ prog->type == BPF_PROG_TYPE_LSM) {
+ /* bpf_init_inode_xattr[_impl] only attach to inode_init_security. */
+ if ((kfunc_id == bpf_init_inode_xattr_btf_ids[0] ||
+ kfunc_id == bpf_init_inode_xattr_btf_ids[1]) &&
+ prog->aux->attach_btf_id != bpf_lsm_inode_init_security_btf_ids[0])
+ return -EACCES;
return 0;
+ }
return -EACCES;
}
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 101ca6cc5424..e73bb2222c3d 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -682,6 +682,7 @@ struct bpf_insn_aux_data {
*/
u8 fastcall_spills_num:3;
u8 arg_prog:4;
+ u8 init_inode_xattr_fixup:1;
/* below fields are initialized once */
unsigned int orig_idx; /* original instruction index */
@@ -903,6 +904,8 @@ struct bpf_verifier_env {
bool bypass_spec_v4;
bool seen_direct_write;
bool seen_exception;
+ bool needs_ctx_spill;
+ s16 ctx_stack_off;
struct bpf_insn_aux_data *insn_aux_data; /* array of per-insn state */
const struct bpf_line_info *prev_linfo;
struct bpf_verifier_log log;
diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index fba9e8c00878..18d612a9fe29 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -725,6 +725,26 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
}
}
+ if (env->needs_ctx_spill) {
+ if (epilogue_cnt) {
+ /* gen_epilogue already saved ctx to the stack */
+ env->ctx_stack_off = -(s16)subprogs[0].stack_depth;
+ } else {
+ cnt = 0;
+ subprogs[0].stack_depth += 8;
+ env->ctx_stack_off = -(s16)subprogs[0].stack_depth;
+ insn_buf[cnt++] = BPF_STX_MEM(BPF_DW, BPF_REG_FP,
+ BPF_REG_1,
+ env->ctx_stack_off);
+ insn_buf[cnt++] = env->prog->insnsi[0];
+ new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt);
+ if (!new_prog)
+ return -ENOMEM;
+ env->prog = new_prog;
+ delta += cnt - 1;
+ }
+ }
+
if (ops->gen_prologue || env->seen_direct_write) {
if (!ops->gen_prologue) {
verifier_bug(env, "gen_prologue is null");
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 03f9e16c2abe..af5753ffb16b 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10794,6 +10794,8 @@ enum special_kfunc_type {
KF_bpf_arena_alloc_pages,
KF_bpf_arena_free_pages,
KF_bpf_arena_reserve_pages,
+ KF_bpf_init_inode_xattr,
+ KF_bpf_init_inode_xattr_impl,
KF_bpf_session_is_return,
KF_bpf_stream_vprintk,
KF_bpf_stream_print_stack,
@@ -10882,6 +10884,13 @@ BTF_ID(func, bpf_task_work_schedule_resume)
BTF_ID(func, bpf_arena_alloc_pages)
BTF_ID(func, bpf_arena_free_pages)
BTF_ID(func, bpf_arena_reserve_pages)
+#ifdef CONFIG_BPF_LSM
+BTF_ID(func, bpf_init_inode_xattr)
+BTF_ID(func, bpf_init_inode_xattr_impl)
+#else
+BTF_ID_UNUSED
+BTF_ID_UNUSED
+#endif
BTF_ID(func, bpf_session_is_return)
BTF_ID(func, bpf_stream_vprintk)
BTF_ID(func, bpf_stream_print_stack)
@@ -12701,6 +12710,24 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
if (err < 0)
return err;
+ if (meta.func_id == special_kfunc_list[KF_bpf_init_inode_xattr_impl]) {
+ verbose(env, "bpf_init_inode_xattr_impl is not callable directly\n");
+ return -EACCES;
+ }
+
+ if (meta.func_id == special_kfunc_list[KF_bpf_init_inode_xattr]) {
+ if (env->cur_state->curframe != 0) {
+ verbose(env, "bpf_init_inode_xattr cannot be called from subprograms\n");
+ return -EINVAL;
+ }
+ env->needs_ctx_spill = true;
+ insn_aux->init_inode_xattr_fixup = true;
+ err = bpf_add_kfunc_call(env,
+ special_kfunc_list[KF_bpf_init_inode_xattr_impl], 0);
+ if (err < 0)
+ return err;
+ }
+
if (is_bpf_rbtree_add_kfunc(meta.func_id)) {
err = push_callback_call(env, insn, insn_idx, meta.subprogno,
set_rbtree_add_callback_state);
@@ -19272,6 +19299,33 @@ int bpf_fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
insn_buf[4] = BPF_ALU64_REG(BPF_SUB, BPF_REG_0, BPF_REG_1);
insn_buf[5] = BPF_ALU64_IMM(BPF_NEG, BPF_REG_0, 0);
*cnt = 6;
+ } else if (env->insn_aux_data[insn_idx].init_inode_xattr_fixup) {
+ struct bpf_kfunc_desc *impl_desc;
+
+ impl_desc = find_kfunc_desc(env->prog,
+ special_kfunc_list[KF_bpf_init_inode_xattr_impl], 0);
+ if (!impl_desc) {
+ verifier_bug(env, "bpf_init_inode_xattr_impl desc not found");
+ return -EFAULT;
+ }
+
+ /* Rewrite bpf_init_inode_xattr(name, value) to inject xattrs and
+ * xattr_count loaded from the saved inode_init_security ctx.
+ */
+ insn_buf[0] = BPF_MOV64_REG(BPF_REG_3, BPF_REG_1);
+ insn_buf[1] = BPF_MOV64_REG(BPF_REG_4, BPF_REG_2);
+ insn_buf[2] = BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_FP,
+ env->ctx_stack_off);
+ insn_buf[3] = BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_2,
+ 3 * sizeof(u64));
+ insn_buf[4] = BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_2,
+ 4 * sizeof(u64));
+ insn_buf[5] = *insn;
+ if (!bpf_jit_supports_far_kfunc_call())
+ insn_buf[5].imm = BPF_CALL_IMM(impl_desc->addr);
+ else
+ insn_buf[5].imm = impl_desc->func_id;
+ *cnt = 6;
}
if (env->insn_aux_data[insn_idx].arg_prog) {
diff --git a/security/bpf/hooks.c b/security/bpf/hooks.c
index 40efde233f3a..1e61baa821bd 100644
--- a/security/bpf/hooks.c
+++ b/security/bpf/hooks.c
@@ -28,8 +28,11 @@ static int __init bpf_lsm_init(void)
return 0;
}
+#define BPF_LSM_INODE_INIT_XATTRS 1
+
struct lsm_blob_sizes bpf_lsm_blob_sizes __ro_after_init = {
.lbs_inode = sizeof(struct bpf_storage_blob),
+ .lbs_xattr_count = BPF_LSM_INODE_INIT_XATTRS,
};
DEFINE_LSM(bpf) = {
--
2.53.0
^ permalink raw reply related
* [PATCH] selinux: don't reserve xattr slot when we won't fill it
From: David Windsor @ 2026-04-26 23:23 UTC (permalink / raw)
To: Paul Moore, Stephen Smalley
Cc: Ondrej Mosnacek, selinux, linux-security-module, linux-kernel
Move lsm_get_xattr_slot() below the SBLABEL_MNT check so we don't leave
a NULL-named slot in the array when returning -EOPNOTSUPP; filesystem
initxattrs() callbacks stop iterating at the first NULL ->name, silently
dropping xattrs installed by later LSMs.
Signed-off-by: David Windsor <dwindsor@gmail.com>
---
security/selinux/hooks.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 97801966bf32..4ff118a9395f 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -2966,7 +2966,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
{
const struct cred_security_struct *crsec = selinux_cred(current_cred());
struct superblock_security_struct *sbsec;
- struct xattr *xattr = lsm_get_xattr_slot(xattrs, xattr_count);
+ struct xattr *xattr;
u32 newsid, clen;
u16 newsclass;
int rc;
@@ -2992,6 +2992,7 @@ static int selinux_inode_init_security(struct inode *inode, struct inode *dir,
!(sbsec->flags & SBLABEL_MNT))
return -EOPNOTSUPP;
+ xattr = lsm_get_xattr_slot(xattrs, xattr_count);
if (xattr) {
rc = security_sid_to_context_force(newsid,
&context, &clen);
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v2 0/4] Firmware LSM hook
From: Jason Gunthorpe @ 2026-04-26 13:42 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Paul Moore, Roberto Sassu, KP Singh, Matt Bobrowski,
Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
Saeed Mahameed, Itay Avraham, Dave Jiang, Jonathan Cameron, bpf,
linux-kernel, linux-kselftest, linux-rdma, Chiara Meiohas,
Maher Sanalla, linux-security-module
In-Reply-To: <20260426103957.GH172828@unreal>
On Sun, Apr 26, 2026 at 01:39:57PM +0300, Leon Romanovsky wrote:
> On Fri, Apr 24, 2026 at 11:19:21AM -0300, Jason Gunthorpe wrote:
> > On Thu, Apr 23, 2026 at 05:09:50PM +0300, Leon Romanovsky wrote:
> >
> > > > > Leon mentioned that different firmware revisions would have different
> > > > > parameters for a given opcode, and that one would need to inspect
> > > > > those parameters to properly filter the command. Is that not true, or
> > > > > am I misreading or misunderstanding Leon's comments?
> > > >
> > > > They are ABI stable, so there will be rules about future changes that
> > > > old software can follow to ignore or reject future things it doesn't
> > > > understand.
> > >
> > > It is wishful thinking and applicable only to mlx5 devices. No one
> > > promises that other devices follow same ABI rules.
> >
> > Well, I will definately kick them out of fwctl if they don't.
>
> It is easy to say but harder to follow. The kernel includes many devices that
> exist only in specific hyperscale environments, where the update cycle is
> tightly controlled. They easily can break FW backward compatibility.
Well Linus's rule applies here, if it doesn't bother anyone it didn't
break..
Jason
^ permalink raw reply
* Re: [PATCH v2 0/4] Firmware LSM hook
From: Leon Romanovsky @ 2026-04-26 10:39 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: Paul Moore, Roberto Sassu, KP Singh, Matt Bobrowski,
Alexei Starovoitov, Daniel Borkmann, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, Stanislav Fomichev, Hao Luo, Jiri Olsa, Shuah Khan,
Saeed Mahameed, Itay Avraham, Dave Jiang, Jonathan Cameron, bpf,
linux-kernel, linux-kselftest, linux-rdma, Chiara Meiohas,
Maher Sanalla, linux-security-module
In-Reply-To: <20260424141921.GA3611611@ziepe.ca>
On Fri, Apr 24, 2026 at 11:19:21AM -0300, Jason Gunthorpe wrote:
> On Thu, Apr 23, 2026 at 05:09:50PM +0300, Leon Romanovsky wrote:
>
> > > > Leon mentioned that different firmware revisions would have different
> > > > parameters for a given opcode, and that one would need to inspect
> > > > those parameters to properly filter the command. Is that not true, or
> > > > am I misreading or misunderstanding Leon's comments?
> > >
> > > They are ABI stable, so there will be rules about future changes that
> > > old software can follow to ignore or reject future things it doesn't
> > > understand.
> >
> > It is wishful thinking and applicable only to mlx5 devices. No one
> > promises that other devices follow same ABI rules.
>
> Well, I will definately kick them out of fwctl if they don't.
It is easy to say but harder to follow. The kernel includes many devices that
exist only in specific hyperscale environments, where the update cycle is
tightly controlled. They easily can break FW backward compatibility.
Thanks
>
> Jason
^ permalink raw reply
* Re: [PATCH RFC 3/3] LSM: Reserve use of secmarks
From: Casey Schaufler @ 2026-04-25 19:03 UTC (permalink / raw)
To: Paul Moore, linux-security-module
Cc: jmorris, serge, keescook, john.johansen, penguin-kernel,
stephen.smalley.work, selinux, Casey Schaufler
In-Reply-To: <8fa09781ac340398fb2b914bf29d9dcb@paul-moore.com>
On 4/23/2026 6:19 PM, Paul Moore wrote:
> On Feb 25, 2026 Casey Schaufler <casey@schaufler-ca.com> wrote:
>> Use of "exclusive" LSM hooks are limited to the first LSM registering
>> them. These hooks include those use to process network secmarks.
>> The hooks required to process secmarks are flagged with LSM_FLAG_EXCLUSIVE
>> in their definitions. This includes secid_to_secctx and secctx_to_secid,
>> which are used by netfilter.
>>
>> The various LSMs that use secmarks are updated to detect whether
>> they are providing exclusive hooks, and to eschew the use of secmarks
>> if they are not.
>>
>> SELinux has a peculiar behavior in that it may decide that it
>> must have network controls, but only after policy is loaded.
>> This patch includes a warning should policy capability alwaysnetwork
>> be set when another LSM holds the exclusive hooks. It has been
>> suggested that SELinux would consider this a fatal condition, in
>> which case a panic might be a favored, if draconian, option.
>>
>> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
>> ---
>> include/linux/lsm_hook_defs.h | 12 +++++------
>> include/linux/security.h | 1 +
>> security/apparmor/lsm.c | 24 ++++++++++++++++------
>> security/security.c | 15 ++++++++++++++
>> security/selinux/hooks.c | 35 ++++++++++++++++++++++++--------
>> security/selinux/ss/services.c | 3 +++
>> security/smack/smack_lsm.c | 6 ++++--
>> security/smack/smack_netfilter.c | 6 ++++++
>> 8 files changed, 80 insertions(+), 22 deletions(-)
> ..
>
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index e3c137a1b30a..638436b9b748 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -326,6 +326,7 @@ int unregister_blocking_lsm_notifier(struct notifier_block *nb);
>> extern int security_init(void);
>> extern int early_security_init(void);
>> extern u64 lsm_name_to_attr(const char *name);
>> +extern u32 lsm_secmark_from_skb(struct sk_buff *skb, const u64 lsmid);
>>
>> /* Security operations */
>> int security_binder_set_context_mgr(const struct cred *mgr);
>> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
>> index a87cd60ed206..2ce0d9a73973 100644
>> --- a/security/apparmor/lsm.c
>> +++ b/security/apparmor/lsm.c
>> @@ -1511,9 +1511,11 @@ static int apparmor_socket_shutdown(struct socket *sock, int how)
>> static int apparmor_socket_sock_rcv_skb(struct sock *sk, struct sk_buff *skb)
>> {
>> struct aa_sk_ctx *ctx = aa_sock(sk);
>> + u32 secmark;
>> int error;
>>
>> - if (!skb->secmark)
>> + secmark = lsm_secmark_from_skb(skb, LSM_ID_APPARMOR);
>> + if (!secmark)
>> return 0;
>>
>> /*
> ..
>
>> diff --git a/security/security.c b/security/security.c
>> index 25e7cfc96f20..754b16004677 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -4509,6 +4509,21 @@ void security_inet_conn_established(struct sock *sk,
>> }
>> EXPORT_SYMBOL(security_inet_conn_established);
>>
>> +/**
>> + * lsm_secmark_from_skb - secid for the specified LSM from the packet
>> + * @skb: the packet
>> + * @lsm: which LSM is asking
>> + *
>> + * If the specified LSM has use of the secmark, return its value.
>> + * Otherwise, return the invalid secid 0.
>> + */
>> +u32 lsm_secmark_from_skb(struct sk_buff *skb, const u64 lsmid)
>> +{
>> + if (lsmid == lsm_exclusive_hooks)
>> + return skb->secmark;
>> + return 0;
>> +}
> Ooof. Not a fan. A better way to handle this would be to like I
> mentioned in my comments on patch 2/3: have the LSM detect that it lost
> the LSM_FLAG_EXCLUSIVE battle at callback registration time and do
> whatever adjustments are necessary at init time. In a number of cases I
> believe that simply not registering the callback will be sufficient.
In no case (AppArmor, Smack, SELinux) will that be sufficient. All
access skb->secmark directly. The hooks that use the secmark also use
other mechanisms (e.g. netlabel) to obtain security attributes.
> If the only way to solve this is via runtime checks like this,
> unfortunately my answer is going to be "no".
OK. I hear you.
> .. and of course the proper way to solve this for secmark is to just
> do the idr/xarray conversion for secmarks so every LSM can have their
> own secmark. Yes, it does require some work, but to be honest I think
> you've spent more time avoiding that work then it would be to just do
> it. I'm growing increasing inclined to simply state that this is the
> only solution I'm going to accept.
Thank you for the clarity. If you happen to have a preferred example of
how idr/xarray is used I expect that we can reduce the review cycle of
such an implementation.
... Or, if there's someone out there who's itching to get involved and
eager to tackle a moderately complicated integration effort, I'd happily
outline the pain points.
>
> --
> paul-moore.com
With that I think this RFC has been useful in that the LSM define flag
scheme might be useful in the future, but doesn't address the task at
hand.
^ permalink raw reply
* Re: [RFC PATCH v3 4/4] Revert "firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall"
From: Jarkko Sakkinen @ 2026-04-25 14:19 UTC (permalink / raw)
To: Jonathan McDowell
Cc: linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, paul, jmorris, serge, zohar,
roberto.sassu, dmitry.kasatkin, eric.snowberg, jgg, sudeep.holla,
maz, oupton, joey.gouly, suzuki.poulose, yuzenghui,
catalin.marinas, will, noodles, sebastianene, Yeoreum Yun
In-Reply-To: <2e7b4dc552b45ddf14cc43bc449cbebb4ade0027.1777036497.git.noodles@meta.com>
On Fri, Apr 24, 2026 at 02:24:42PM +0100, Jonathan McDowell wrote:
> From: Yeoreum Yun <yeoreum.yun@arm.com>
>
> This reverts commit 0e0546eabcd6c19765a8dbf5b5db3723e7b0ea75, which was
> added to address ordering issues with the IMA LSM initialisation where
> the TPM would not be fully ready by the time IMA wanted it. This has
> been resolved within IMA by retrying setup during late_initcall_sync if
> the TPM is not available at first.
>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> Signed-off-by: Jonathan McDowell <noodles@meta.com>
> ---
> drivers/firmware/arm_ffa/driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index f2f94d4d533e..01547c5c0e38 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -2106,7 +2106,7 @@ static int __init ffa_init(void)
> kfree(drv_info);
> return ret;
> }
> -rootfs_initcall(ffa_init);
> +module_init(ffa_init);
>
> static void __exit ffa_exit(void)
> {
> --
> 2.53.0
>
LGTM (for both tpm patches).
However, I'll hold on any further comments/tags up until I've sorted 7.1 PRs
(just so that I have full focus).
BR, Jarkko
^ permalink raw reply
* Re: [RFC PATCH v3 2/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Jonathan McDowell @ 2026-04-25 9:10 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, paul, jmorris, serge, roberto.sassu,
dmitry.kasatkin, eric.snowberg, jarkko, jgg, sudeep.holla, maz,
oupton, joey.gouly, suzuki.poulose, yuzenghui, catalin.marinas,
will, noodles, sebastianene, Yeoreum Yun
In-Reply-To: <43ff6ca37df45ed53061dad46e9d31a5118e5714.camel@linux.ibm.com>
On Fri, Apr 24, 2026 at 04:25:31PM -0400, Mimi Zohar wrote:
>Thanks, Jonathan!
>
>On Fri, 2026-04-24 at 14:24 +0100, Jonathan McDowell wrote:
>> -static int __init init_ima(void)
>> +static int __init init_ima(bool late)
>> {
>> int error;
>>
>> @@ -1247,10 +1247,26 @@ static int __init init_ima(void)
>> return 0;
>> }
>>
>> + /*
>> + * If we found the TPM during our first attempt, or we know there's no
>> + * TPM, nothing further to do
>> + */
>
>Perhaps it's just me, but the comment wording is a bit off. Could I change it
>to: If we either found the TPM or knew there's no TPM during our first attempt,
>nothing futher to do.
No objections to that updated wording from me.
>Otherwise the patch looks good.
>
>Mimi
>
>
>> + if (late && (ima_tpm_chip || !IS_ENABLED(CONFIG_TCG_TPM)))
>> + return 0;
>> +
>> + ima_tpm_chip = tpm_default_chip();
>> + if (!ima_tpm_chip && !late && IS_ENABLED(CONFIG_TCG_TPM)) {
>> + pr_debug("TPM not available, will try later\n");
>> + return -EPROBE_DEFER;
>> + }
>> +
>> + if (!ima_tpm_chip)
>> + pr_info("No TPM chip found, activating TPM-bypass!\n");
>> +
J.
--
Revd Jonathan McDowell, ULC | Run like hell!
^ permalink raw reply
* Re: [RFC PATCH v2 1/4] security: ima: call ima_init() again at late_initcall_sync for defered TPM
From: Yeoreum Yun @ 2026-04-25 4:53 UTC (permalink / raw)
To: Mimi Zohar
Cc: Paul Moore, roberto.sassu, Jonathan McDowell,
linux-security-module, linux-kernel, linux-integrity,
linux-arm-kernel, kvmarm, jmorris, serge, dmitry.kasatkin,
eric.snowberg, jarkko, jgg, sudeep.holla, maz, oupton, joey.gouly,
suzuki.poulose, yuzenghui, catalin.marinas, will, noodles,
sebastianene
In-Reply-To: <014cf39aa8d6a0bcfa1a95c069675977ac67b843.camel@linux.ibm.com>
[...]
> > I understand the need to ensure that the TPM is available, but if it
> > isn't safe to wait to initialize IMA at late_initcall_sync() then it
> > would seem like this is a bad option and we need another mechanism to
> > synchronize IMA with TPM devices. If it is safe to initalize IMA in
> > late_initcall_sync(), just do that and be done with it.
>
> Within the same initcall level there is no way of ordering the initialization.
> Yeorum attempted to address the ordering issue in commit 0e0546eabcd6
> ("firmware: arm_ffa: Change initcall level of ffa_init() to rootfs_initcall"),
> which is being reverted in this patch set.
>
> Ordering within an initcall level needs to be fixed, but for now retrying at
> late_initcall_sync works for some, hopefully most, cases.
Ordering within an initcall level is not good idea.
Though it came to my mind first long ago,
It also goes against existing mechanisms like deferred probe, and
can cause the driver model’s dependency handling to harden
in the wrong direction. So this I think it wouldn't be an option.
--
Sincerely,
Yeoreum Yun
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox