Linux Security Modules development
 help / color / mirror / Atom feed
* [PATCH 5.15.y] apparmor: fix memory leak in verify_header
From: Li hongliang @ 2026-04-13  5:49 UTC (permalink / raw)
  To: massimiliano.pellizzer
  Cc: john.johansen, paul, jmorris, serge, apparmor,
	linux-security-module, qsa, carnil, georgia.garcia, cengiz.can

From: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com>

[ Upstream commit e38c55d9f834e5b848bfed0f5c586aaf45acb825 ]

The function sets `*ns = NULL` on every call, leaking the namespace
string allocated in previous iterations when multiple profiles are
unpacked. This also breaks namespace consistency checking since *ns
is always NULL when the comparison is made.

Remove the incorrect assignment.
The caller (aa_unpack) initializes *ns to NULL once before the loop,
which is sufficient.

Fixes: dd51c8485763 ("apparmor: provide base for multiple profiles to be replaced at once")
Reported-by: Qualys Security Advisory <qsa@qualys.com>
Tested-by: Salvatore Bonaccorso <carnil@debian.org>
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Reviewed-by: Cengiz Can <cengiz.can@canonical.com>
Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Li hongliang <1468888505@139.com>
---
 security/apparmor/policy_unpack.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 851fd6212831..3bbd28603c8c 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -959,7 +959,6 @@ static int verify_header(struct aa_ext *e, int required, const char **ns)
 {
 	int error = -EPROTONOSUPPORT;
 	const char *name = NULL;
-	*ns = NULL;
 
 	/* get the interface version */
 	if (!unpack_u32(e, &e->version, "version")) {
-- 
2.34.1



^ permalink raw reply related

* [PATCH 5.10.y] apparmor: fix memory leak in verify_header
From: Li hongliang @ 2026-04-13  5:49 UTC (permalink / raw)
  To: massimiliano.pellizzer
  Cc: john.johansen, paul, jmorris, serge, apparmor,
	linux-security-module, qsa, carnil, georgia.garcia, cengiz.can

From: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com>

[ Upstream commit e38c55d9f834e5b848bfed0f5c586aaf45acb825 ]

The function sets `*ns = NULL` on every call, leaking the namespace
string allocated in previous iterations when multiple profiles are
unpacked. This also breaks namespace consistency checking since *ns
is always NULL when the comparison is made.

Remove the incorrect assignment.
The caller (aa_unpack) initializes *ns to NULL once before the loop,
which is sufficient.

Fixes: dd51c8485763 ("apparmor: provide base for multiple profiles to be replaced at once")
Reported-by: Qualys Security Advisory <qsa@qualys.com>
Tested-by: Salvatore Bonaccorso <carnil@debian.org>
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Reviewed-by: Cengiz Can <cengiz.can@canonical.com>
Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Li hongliang <1468888505@139.com>
---
 security/apparmor/policy_unpack.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 93fcafdaa548..5ea8c14f5eac 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -959,7 +959,6 @@ static int verify_header(struct aa_ext *e, int required, const char **ns)
 {
 	int error = -EPROTONOSUPPORT;
 	const char *name = NULL;
-	*ns = NULL;
 
 	/* get the interface version */
 	if (!unpack_u32(e, &e->version, "version")) {
-- 
2.34.1



^ permalink raw reply related

* [PATCH 6.1.y] apparmor: fix memory leak in verify_header
From: Li hongliang @ 2026-04-13  5:48 UTC (permalink / raw)
  To: massimiliano.pellizzer
  Cc: john.johansen, paul, jmorris, serge, apparmor,
	linux-security-module, qsa, carnil, georgia.garcia, cengiz.can

From: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com>

[ Upstream commit e38c55d9f834e5b848bfed0f5c586aaf45acb825 ]

The function sets `*ns = NULL` on every call, leaking the namespace
string allocated in previous iterations when multiple profiles are
unpacked. This also breaks namespace consistency checking since *ns
is always NULL when the comparison is made.

Remove the incorrect assignment.
The caller (aa_unpack) initializes *ns to NULL once before the loop,
which is sufficient.

Fixes: dd51c8485763 ("apparmor: provide base for multiple profiles to be replaced at once")
Reported-by: Qualys Security Advisory <qsa@qualys.com>
Tested-by: Salvatore Bonaccorso <carnil@debian.org>
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Reviewed-by: Cengiz Can <cengiz.can@canonical.com>
Signed-off-by: Massimiliano Pellizzer <massimiliano.pellizzer@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Li hongliang <1468888505@139.com>
---
 security/apparmor/policy_unpack.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/security/apparmor/policy_unpack.c b/security/apparmor/policy_unpack.c
index 17601235ff98..22cc968a01fc 100644
--- a/security/apparmor/policy_unpack.c
+++ b/security/apparmor/policy_unpack.c
@@ -942,7 +942,6 @@ static int verify_header(struct aa_ext *e, int required, const char **ns)
 {
 	int error = -EPROTONOSUPPORT;
 	const char *name = NULL;
-	*ns = NULL;
 
 	/* get the interface version */
 	if (!aa_unpack_u32(e, &e->version, "version")) {
-- 
2.34.1



^ permalink raw reply related

* Re: LSM: Whiteout chardev creation sidesteps mknod hook
From: Miklos Szeredi @ 2026-04-13 10:18 UTC (permalink / raw)
  To: Günther Noack
  Cc: Christian Brauner, Serge Hallyn, Amir Goldstein,
	Mickaël Salaün, Paul Moore, linux-security-module
In-Reply-To: <adoIGHwgMJSuRfE5@google.com>

On Sat, 11 Apr 2026 at 10:36, Günther Noack <gnoack@google.com> wrote:

> I also don't currently see how an attacker would abuse this, but I still see
> this as a violation of Landlock's security model if we can create a policy that
> denies the creation of character device directory entries, and then we still
> have a way to make them appear there where we previously had a different file.

Look: a whiteout is a whiteout, NOT a character device.  Don't let the
fact that it's represented by "c 0 0" fool you, this is a completely
different beast.  See commit a3c751a50fe6 ("vfs: allow unprivileged
whiteout creation").

Does this beast need special handling by LSMs?  I have no idea, but
treating them the same as char devs sounds like a bad idea.

Thanks,
Miklos

^ permalink raw reply

* Re: [PATCH] trusted-keys: move pr_fmt out of trusted-type.h
From: Marco Felsch @ 2026-04-13 11:01 UTC (permalink / raw)
  To: Josh Snyder
  Cc: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
	Ahmad Fatoum, Pengutronix Kernel Team, Paul Moore, James Morris,
	Serge E. Hallyn, David Gstir, sigma star Kernel Team,
	Srish Srinivasan, Nayna Jain, Sumit Garg, linux-security-module,
	linux-integrity, keyrings, linux-kernel
In-Reply-To: <20260411-trusted-key-header-v1-1-407c2cd954db@code406.com>

Hi Josh,

On 26-04-11, Josh Snyder wrote:
> Defining pr_fmt in a widely-included header leaks the "trusted_key: "
> prefix into every translation unit that transitively includes
> <keys/trusted-type.h>. dm-crypt, for example, ends up printing
> 
>     trusted_key: device-mapper: crypt: dm-10: INTEGRITY AEAD ERROR ...
> 
> dm-crypt began including <keys/trusted-type.h> in commit 363880c4eb36
> ("dm crypt: support using trusted keys"), which predates the pr_fmt
> addition, so the regression has been live from the moment the header
> gained its own pr_fmt definition.
> 
> Move the pr_fmt definition into the trusted-keys source files that
> actually want the prefix.
> 
> Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
> Assisted-by: Claude:claude-opus-4-6
> Signed-off-by: Josh Snyder <josh@code406.com>
> ---
>  include/keys/trusted-type.h               | 6 ------
>  security/keys/trusted-keys/trusted_caam.c | 2 ++
>  security/keys/trusted-keys/trusted_core.c | 2 ++
>  security/keys/trusted-keys/trusted_dcp.c  | 2 ++
>  security/keys/trusted-keys/trusted_pkwm.c | 2 ++
>  security/keys/trusted-keys/trusted_tpm1.c | 2 ++
>  security/keys/trusted-keys/trusted_tpm2.c | 2 ++
>  7 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index 03527162613f7..54da1f174aeab 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -11,12 +11,6 @@
>  #include <linux/rcupdate.h>
>  #include <linux/tpm.h>
>  
> -#ifdef pr_fmt
> -#undef pr_fmt
> -#endif
> -
> -#define pr_fmt(fmt) "trusted_key: " fmt
> -
>  #define MIN_KEY_SIZE			32
>  #define MAX_KEY_SIZE			128
>  #if IS_ENABLED(CONFIG_TRUSTED_KEYS_PKWM)
> diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
> index 601943ce0d60f..a31fd89c0e5c5 100644
> --- a/security/keys/trusted-keys/trusted_caam.c
> +++ b/security/keys/trusted-keys/trusted_caam.c
> @@ -4,6 +4,8 @@
>   * Copyright 2025 NXP
>   */
>  
> +#define pr_fmt(fmt) "trusted_key: " fmt

Can we adapt this patch further to include the trusted-key type as well?
E.g. 'trusted_key-caam'.

Regards,
  Marco

^ permalink raw reply

* Re: [PATCH] trusted-keys: move pr_fmt out of trusted-type.h
From: Ahmad Fatoum @ 2026-04-13 11:03 UTC (permalink / raw)
  To: Marco Felsch, Josh Snyder
  Cc: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
	Pengutronix Kernel Team, Paul Moore, James Morris,
	Serge E. Hallyn, David Gstir, sigma star Kernel Team,
	Srish Srinivasan, Nayna Jain, Sumit Garg, linux-security-module,
	linux-integrity, keyrings, linux-kernel
In-Reply-To: <cie3zqy5phlopdrxsxpniujwr6i3cpdkfrwjvth3a7ypwjx3ee@hqjl67jnfdch>

Hi,

On 4/13/26 1:01 PM, Marco Felsch wrote:
> Hi Josh,
> 
> On 26-04-11, Josh Snyder wrote:
>> Defining pr_fmt in a widely-included header leaks the "trusted_key: "
>> prefix into every translation unit that transitively includes
>> <keys/trusted-type.h>. dm-crypt, for example, ends up printing
>>
>>     trusted_key: device-mapper: crypt: dm-10: INTEGRITY AEAD ERROR ...
>>
>> dm-crypt began including <keys/trusted-type.h> in commit 363880c4eb36
>> ("dm crypt: support using trusted keys"), which predates the pr_fmt
>> addition, so the regression has been live from the moment the header
>> gained its own pr_fmt definition.
>>
>> Move the pr_fmt definition into the trusted-keys source files that
>> actually want the prefix.
>>
>> Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
>> Assisted-by: Claude:claude-opus-4-6
>> Signed-off-by: Josh Snyder <josh@code406.com>
>> ---
>>  include/keys/trusted-type.h               | 6 ------
>>  security/keys/trusted-keys/trusted_caam.c | 2 ++
>>  security/keys/trusted-keys/trusted_core.c | 2 ++
>>  security/keys/trusted-keys/trusted_dcp.c  | 2 ++
>>  security/keys/trusted-keys/trusted_pkwm.c | 2 ++
>>  security/keys/trusted-keys/trusted_tpm1.c | 2 ++
>>  security/keys/trusted-keys/trusted_tpm2.c | 2 ++
>>  7 files changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
>> index 03527162613f7..54da1f174aeab 100644
>> --- a/include/keys/trusted-type.h
>> +++ b/include/keys/trusted-type.h
>> @@ -11,12 +11,6 @@
>>  #include <linux/rcupdate.h>
>>  #include <linux/tpm.h>
>>  
>> -#ifdef pr_fmt
>> -#undef pr_fmt
>> -#endif
>> -
>> -#define pr_fmt(fmt) "trusted_key: " fmt
>> -
>>  #define MIN_KEY_SIZE			32
>>  #define MAX_KEY_SIZE			128
>>  #if IS_ENABLED(CONFIG_TRUSTED_KEYS_PKWM)
>> diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
>> index 601943ce0d60f..a31fd89c0e5c5 100644
>> --- a/security/keys/trusted-keys/trusted_caam.c
>> +++ b/security/keys/trusted-keys/trusted_caam.c
>> @@ -4,6 +4,8 @@
>>   * Copyright 2025 NXP
>>   */
>>  
>> +#define pr_fmt(fmt) "trusted_key: " fmt
> 
> Can we adapt this patch further to include the trusted-key type as well?
> E.g. 'trusted_key-caam'.

Agreed, if we move it into the individual files, we can use the occasion
to make it a bit more descriptive.

I would suggest "trusted_key: caam: ", so the prefix stays the same.

Cheers,
Ahmad

> 
> Regards,
>   Marco
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |


^ permalink raw reply

* Re: LSM: Whiteout chardev creation sidesteps mknod hook
From: Günther Noack @ 2026-04-13 12:23 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Christian Brauner, Serge Hallyn, Amir Goldstein,
	Mickaël Salaün, Paul Moore, linux-security-module
In-Reply-To: <CAJfpegv6GDFZjdGrQ=0Jahvz5mSgfJr+GvjVwws=SFX-yirpSg@mail.gmail.com>

On Mon, Apr 13, 2026 at 12:18:08PM +0200, Miklos Szeredi wrote:
> On Sat, 11 Apr 2026 at 10:36, Günther Noack <gnoack@google.com> wrote:
> > I also don't currently see how an attacker would abuse this, but I still see
> > this as a violation of Landlock's security model if we can create a policy that
> > denies the creation of character device directory entries, and then we still
> > have a way to make them appear there where we previously had a different file.
> 
> Look: a whiteout is a whiteout, NOT a character device.  Don't let the
> fact that it's represented by "c 0 0" fool you, this is a completely
> different beast.  See commit a3c751a50fe6 ("vfs: allow unprivileged
> whiteout creation").
> 
> Does this beast need special handling by LSMs?  I have no idea, but
> treating them the same as char devs sounds like a bad idea.

Thanks for the pointer to that commit.  I was under the impression
that creation of the whiteout objects required CAP_MKNOD, but it seems
you have dropped that requirement in that commit.

(FWIW, I was mislead by the rename(2) man page[1], which is apparently
not up to date and where it explicitly says:

    RENAME_WHITEOUT requires the same privileges as creating a
    device node (i.e., the CAP_MKNOD capability).

So with that assumption, it seemed natural that this should have
extended equivalently into a Landlock policy.)

So if the "c 0 0" whiteout device is indeed a different kind of file,
maybe we would need to treat it with a separate Landlock access right
after all then.  I'll ponder it.

FWIW, besides introducing a new LANDLOCK_ACCESS_FS_MAKE_WHITEOUT
access right and adding more special cases to the Landlock API,
another possible option might be to just forbid creating whiteout
objects altogether, when under a Landlock policy.  As the man page
also notes, "This operation makes sense only for overlay/union
filesystem implementations", and since these likely can't use Landlock
anyway due to mounting, I think there would be no use case left where
anyone would want to perform such an operation within a Landlock
domain -- I don't think this would break anyone.  Mickaël, do you have
an opinion on that idea?

—Günther

P.S. Initial patch set from Saturday is at [2], but this still uses
the LANDLOCK_ACCESS_FS_MAKE_CHAR right.

[1] https://man7.org/linux/man-pages/man2/rename.2.html
[2] https://lore.kernel.org/all/20260411090944.3131168-2-gnoack@google.com/

^ permalink raw reply

* Re: landlock: Add support for chmod and chown system calls families
From: Günther Noack @ 2026-04-13 12:36 UTC (permalink / raw)
  To: Jeffrey Bencteux
  Cc: mic, paul, jmorris, serge, linux-security-module, xiujianfeng
In-Reply-To: <20260412095233.34306-1-jeff@bencteux.fr>

Hello Jeffrey,

On Sun, Apr 12, 2026 at 11:50:39AM +0200, Jeffrey Bencteux wrote:
> This patch serie add support for chmod and chown system calls families
> in Landlock.
> 
> These system calls could be used when exploiting applications. Two new
> flags are added for struct landlock_ruleset_attr:
> 
> * LANDLOCK_ACCESS_FS_CHMOD
> * LANDLOCK_ACCESS_FS_CHOWN
> 
> Restriction is limited to files as the security.c hooks for both
> system calls seem to only applies to files. More digging is needed
> before being able to restrict calls to chmod and chown on directories.
> 
> It adds basic tests for both family operations, one for when it is
> allowed, one for when it is not.
> 
> First patch also fixes a bug I encountered when writing the tests.

Thanks for the initial patch!

Before you start your investigation completely from scratch,
did you see the prior work on this topic?

* https://github.com/landlock-lsm/linux/issues/11
* https://lore.kernel.org/all/20220822114701.26975-1-xiujianfeng@huawei.com/

That specific patchset was unfortunately abandoned at the time, but I
suspect that some of the discussion still applies for your patchset as
well?

In my understanding, it was in the end blocked on a LSM hook change.
(If this is needed, a common approach for doing that hook change is to
add it to the same patch series as one of the earliest commits.)

—Günther

^ permalink raw reply

* Re: [RFC PATCH 00/20] BPF interface for applying Landlock rulesets
From: Justin Suess @ 2026-04-13 15:06 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: andrii, ast, bpf, brauner, daniel, eddyz87, fred, gnoack, jack,
	jmorris, john.fastabend, kees, kpsingh, linux-fsdevel,
	linux-kernel, linux-security-module, m, martin.lau, paul
In-Reply-To: <20260408.ainu5Chohnge@digikod.net>

On Wed, Apr 08, 2026 at 09:21:11PM +0200, Mickaël Salaün wrote:
> On Wed, Apr 08, 2026 at 01:10:28PM -0400, Justin Suess wrote:
> > 
> > Add a flag LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS, which executes
> > task_set_no_new_privs on the current credentials, but only if
> > the process lacks the CAP_SYS_ADMIN capability.
> > 
> > While this operation is redundant for code running from userspace
> > (indeed callers may achieve the same logic by calling
> > prctl w/ PR_SET_NO_NEW_PRIVS), this flag enables callers without access
> > to the syscall abi (defined in subsequent patches) to restrict processes
> > from gaining additional capabilities. This is important to ensure that
> > consumers can meet the task_no_new_privs || CAP_SYS_ADMIN invariant
> > enforced by Landlock without having syscall access.
> > 
> > This is done by hooking bprm_committing_creds along with a
> > landlock_cred_security flag to indicate that the next execution should
> > task_set_no_new_privs if the process doesn't possess CAP_SYS_ADMIN. This
> > is done to ensure that task_set_no_new_privs is being done past the
> > point of no return.
> > 
> > Cc: Mickaël Salaün <mic@digikod.net>
> > Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> > ---
> > 
> > On Wed, Apr 08, 2026 at 02:00:00 -0000, Mickaël Salaün wrote:
> > > > Points of Feedback
> > > > ===
> > > > 
> > > > First, the new set_nnp_on_point_of_no_return field in struct linux_binprm.
> > > > This field was needed to request that task_set_no_new_privs be set during an
> > > > execution, but only after the execution has proceeded beyond the point of no
> > > > return. I couldn't find a way to express this semantic without adding a new
> > > > bitfield to struct linux_binprm and a conditional in fs/exec.c. Please see
> > > > patch 2.
> > 
> > > What about using security_bprm_committing_creds()?
> > 
> > Good idea. Definitely cleaner.
> > 
> > Something like this? Then dropping the "execve: Add set_nnp_on_point_of_no_return"
> > commit.
> > 
> > This adds a bitfield to the landlock_cred_security struct to indicate that the flag
> > should be set on the next exec(s).
> > 
> >  include/uapi/linux/landlock.h | 14 ++++++++++++++
> >  security/landlock/cred.c      | 13 +++++++++++++
> >  security/landlock/cred.h      |  7 +++++++
> >  security/landlock/limits.h    |  2 +-
> >  security/landlock/ruleset.c   | 15 ++++++++++++---
> >  security/landlock/syscalls.c  |  5 +++++
> >  6 files changed, 52 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> > index f88fa1f68b77..edd9d9a7f60e 100644
> > --- a/include/uapi/linux/landlock.h
> > +++ b/include/uapi/linux/landlock.h
> > @@ -129,12 +129,26 @@ struct landlock_ruleset_attr {
> >   *
> >   *     If the calling thread is running with no_new_privs, this operation
> >   *     enables no_new_privs on the sibling threads as well.
> > + *
> > + * %LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS
> > + *    Sets no_new_privs on the calling thread before applying the Landlock domain.
> > + *    This flag is useful for convenience as well as for applying a ruleset from
> > + *    an outside context (e.g BPF). This flag only has an effect on when both
> > + *    no_new_privs isn't already set and the caller doesn't possess CAP_SYS_ADMIN.
> > + *
> > + *    This flag has slightly different behavior when used from BPF. Instead of
> > + *    setting no_new_privs on the current task, it sets a flag on the bprm so that
> > + *    no_new_privs is set on the task at exec point-of-no-return. This guarantees
> > + *    that the current execution is unaffected, and may escalate as usual until the
> > + *    next exec, but the resulting task cannot gain more privileges through later
> > + *    exec transitions.
> >   */
> >  /* clang-format off */
> >  #define LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF		(1U << 0)
> >  #define LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON			(1U << 1)
> >  #define LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF		(1U << 2)
> >  #define LANDLOCK_RESTRICT_SELF_TSYNC				(1U << 3)
> > +#define LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS			(1U << 4)
> >  /* clang-format on */
> >  
> >  /**
> > diff --git a/security/landlock/cred.c b/security/landlock/cred.c
> > index 0cb3edde4d18..bcc9b716916f 100644
> > --- a/security/landlock/cred.c
> > +++ b/security/landlock/cred.c
> > @@ -43,6 +43,18 @@ static void hook_cred_free(struct cred *const cred)
> >  		landlock_put_ruleset_deferred(dom);
> >  }
> >  
> > +static void hook_bprm_committing_creds(const struct linux_binprm *bprm)
> > +{
> > +	struct landlock_cred_security *const llcred = landlock_cred(bprm->cred);
> > +
> > +	if (llcred->set_nnp_on_committing_creds &&
> > +	    !ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN)) {
> 
> If asked by the caller, NNP must be set, whatever the capabilities of
> the task.
> 
> > +		task_set_no_new_privs(current);
> > +		/* Don't need to set it again for subsequent execution. */
> > +		llcred->set_nnp_on_committing_creds = false;
> > +	}
> 
> Thinking more about it, it would make more sense to add another flag to
> enforce restriction on the next exec.  This new cred bit would then be
> generic and enforce both NNP (if set) and the domain once we know the
> execution is ok.  That should also bring the required plumbing to
> create the domain at syscall (or kfunc) time and handle memory
> allocation issue there, but only enforce it at exec time with
> security_bprm_committing_creds() (without any possible error).
>

I did some more consideration as well over the weekend.

For no new privs post point of new return:

It still seems to me we can't have post point-of-no-return setting of
NNP from userspace without CAP_SYS_ADMIN for the security reason
listed previously. The BPF side may not need to be subject to that
restriction, since it's in a higher security boundary.

For ruleset enforcement post point of no return:

The post point-of-no-return enforcement of a ruleset from
userspace would be OK, as long as the existing task_no_new_privs ||
CAP_SYS_ADMIN invarient is enforced.

The way I'm thinking of implementing this is storing two pointers to
unmerged rulesets in struct landlock_cred_security. One for the BPF side
and one for the userspace side. If landlock_restrict_self is called with
LANDLOCK_RESTRICT_SELF_EXECTIME (proposed name for this flag), then the
domain would be copied and the pointer to the copy and stored there.

The BPF side would have a seperate pointer, and do the same copy and
store.

Repeated calls to landlock_restrict_self LANDLOCK_RESTRICT_SELF_EXECTIME
would put the reference (and hence free) on the stored unmerged domain,
then store the new one.

When we reach the security_bprm_committing_creds hook, we can merge the
domains in a deterministic order:

1. Existing domain (if any)
2. The domain stored from bpf_landlock_restrict_bprm (if any)
3. The domain stored from landlock_restrict_self w/
LANDLOCK_RESTRICT_SELF_EXECTIME (if any)

Then set the domain pointer to the newly merged domain.

Then we release the references on the stored domains and reset the
pointers to null.

Some implementation details:

1. LANDLOCK_RESTRICT_SELF_EXECTIME w/ bpf_landlock_restrict_binprm is
   redundant since the kfunc is designed to apply there anyway so we can return an error
   if it is explictly set when used with that kfunc. (Or always require
   it be set)
2. The existing LANDLOCK_RESTRICT_SELF_LOG_* flags would be set on the
   stored domain.
3. The TSYNC flags would be sort of misleading for either of these two
   flags and should be mutually exclusive with both of the NO_NEW_PRIVS
   and EXECTIME flags.
4. Common enforcement and merge path for bpf and userspace as you stated
   earlier 

I can make a separate series with one or both of these flags if you
wish once we hear about the preferred tree that this needs to be based
on. Or keep it as one (very large) series.

Justin

> > [...]

^ permalink raw reply

* Re: [PATCH] evm: zero-initialize the evm_xattrs read buffer
From: Roberto Sassu @ 2026-04-13 15:20 UTC (permalink / raw)
  To: Pengpeng Hou, Mimi Zohar, Roberto Sassu
  Cc: Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris,
	Serge Hallyn, linux-integrity, linux-security-module,
	linux-kernel
In-Reply-To: <20260407153002.2-evm-xattrs-pengpeng@iscas.ac.cn>

On Tue, 2026-04-07 at 14:09 +0800, Pengpeng Hou wrote:
> evm_read_xattrs() allocates size + 1 bytes, fills them from the list of
> enabled xattrs and then passes strlen(temp) to simple_read_from_buffer().
> When no configured xattrs are enabled, the fill loop stores nothing and
> temp[0] remains uninitialized, so strlen() reads beyond initialized
> memory.
> 
> Use kzalloc() so the empty-list case stays a valid empty C string.

Please also add the Fixes: tag with the relevant commit.

> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  security/integrity/evm/evm_secfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
> index acd840461902..03d376fa36c2 100644
> --- a/security/integrity/evm/evm_secfs.c
> +++ b/security/integrity/evm/evm_secfs.c
> @@ -145,7 +145,7 @@ static ssize_t evm_read_xattrs(struct file *filp, char __user *buf,
>  		size += strlen(xattr->name) + 1;
>  	}
>  
> -	temp = kmalloc(size + 1, GFP_KERNEL);
> +	temp = kzalloc(size + 1, GFP_KERNEL);

Yes, or just set temp[size] to the terminator so that we don't waste
computation. Can you also change sprintf() to snprintf()?

Thanks

Roberto

>  	if (!temp) {
>  		mutex_unlock(&xattr_list_mutex);
>  		return -ENOMEM;


^ permalink raw reply

* Re: [PATCH v2 0/4] Firmware LSM hook
From: Leon Romanovsky @ 2026-04-13 15:53 UTC (permalink / raw)
  To: Paul Moore
  Cc: 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,
	Jason Gunthorpe, 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: <CAHC9VhRnYXjg+vE9a8PeykbXk91is12zYLaO7EFdfZPKMxDfPA@mail.gmail.com>

On Sun, Apr 12, 2026 at 09:38:35PM -0400, Paul Moore wrote:
> On Sun, Apr 12, 2026 at 5:00 AM Leon Romanovsky <leon@kernel.org> wrote:
> > On Thu, Apr 09, 2026 at 05:04:24PM -0400, Paul Moore wrote:
> > > On Thu, Apr 9, 2026 at 8:45 AM Leon Romanovsky <leon@kernel.org> wrote:
> > > > On Thu, Apr 09, 2026 at 02:27:43PM +0200, Roberto Sassu wrote:
> > > > > On Thu, 2026-04-09 at 15:12 +0300, Leon Romanovsky wrote:
> > > > > > On Tue, Mar 31, 2026 at 08:56:32AM +0300, Leon Romanovsky wrote:
> 
> ...
> 
> > > > We implemented this approach in v1:
> > > > https://patch.msgid.link/20260309-fw-lsm-hook-v1-0-4a6422e63725@nvidia.com
> > > > and were advised to pursue a different direction.
> > >
> > > I'm assuming you are referring to my comments? If so, that isn't exactly what I said,
> > > I mentioned at least one other option besides
> > > going directly to BPF.  Ultimately, it is your choice to decide how
> > > you want to proceed, but to claim I advised you to avoid a LSM based
> > > solution isn't strictly correct.
> >
> > Yes, this matches how we understood your comments:
> > https://lore.kernel.org/all/20260311081955.GS12611@unreal/
> >
> > In the end, the goal is to build something practical and avoid adding
> > unnecessary complexity that brings no real benefit to users.
> >
> > > Regardless, looking at your v2 patchset, it looks like you've taken an
> > > unusual approach of using some of the LSM mechanisms, e.g. LSM_HOOK(),
> > > but not actually exposing a LSM hook with proper callbacks.
> > > Unfortunately, that's not something we want to support.  If you want
> > > to pursue an LSM based solution, complete with a security_XXX() hook,
> > > use of LSM_HOOK() macros, etc. then that's fine, I'm happy to work
> > > with you on that.
> >
> > The issue is that the sentence below was the reason we did not merge v1 with v2:
> > https://github.com/LinuxSecurityModule/kernel/blob/main/README.md#new-lsm-hooks
> > "pass through implementations, such as the BPF LSM, are not eligible for
> > LSM hook reference implementations."
> 
> I can expand on that in a minute, but I'd like to return to your use
> of the LSM_HOOK() macro and locating the hook within the BPF LSM as
> that is the most concerning issue from my perspective.  One should
> only use the LSM_HOOK() macro and locate code within bpf_lsm.c if that
> code is part of the BPF LSM, utilizing an LSM hook.  Since this
> patchset doesn't use an LSM hook or any part of the LSM framework, the
> implementation choices seem odd and are not something we want to
> support.  As mentioned in my prior reply, you could do something very
> similar though the use of a normal BPF hook similar to what was done
> with the update_socket_protocol() BPF hook.
> 
> There are multiple reasons why out-of-tree and pass through LSMs are
> not considered eligible for reference implementations of LSM hooks.  I
> think is most relevant to this patchset is that an out-of-tree hook
> implementation doesn't necessarily require a stable interface, and
> without a stable interface it is impossible to make a generic API at
> the LSM framework layer.  As you mentioned previously, each vendor and
> each firmware version brings the possibility of a new
> format/interface, and while that may not be a problem for out-of-tree
> code which is left to the user/admin to manage, it makes upstream
> development difficult.  I did mention at least one approach that might
> be a possibility to enable upstream, in-tree support of this, but you
> seem to prefer a BPF approach that doesn't require a well defined
> format.
> 
> > > However, if you've decided that your preferred
> > > option is to create a BPF hook you should avoid using things like
> > > LSM_HOOK() and locating your hook/code in bpf_lsm.c.
> >
> > We are not limited to LSM solution, the goal is to intercept commands
> > which are submitted to the FW and "security" bucket sounded right to us.
> 
> Yes, it does sound "security relevant", but without a well defined
> interface/format it is going to be difficult to write a generic LSM to
> have any level of granularity beyond a basic "load firmware"
> permission.
> 
> > > The good news is that there are plenty of other examples of BPF
> > > plugable code that you could use as an example, one such thing is the
> > > update_socket_protocol() BPF hook that was originally proposed as a
> > > LSM hook, but moved to a dedicated BPF hook as we generally want to
> > > avoid changing non-LSM kernel objects within the scope of the LSMs.
> > > While your proposed case is slightly different, I think the basic idea
> > > and mechanism should still be useful.
> > >
> > > https://lore.kernel.org/all/cover.1692147782.git.geliang.tang@suse.com
> >
> > Thanks
> 
> Good luck on whatever you choose, and while I'm guessing it is
> unlikely, if you do decide to pursue a LSM based solution please let
> us know and we can work with you to try and find ways to make it work.

Thanks a lot. We should know which direction we'll take in a week or two,
once Chiara wraps up her internal commitments and returns to this series.

I appreciate your help.

Thanks

> 
> -- 
> paul-moore.com
> 

^ permalink raw reply

* Re: [PATCH v2 0/4] Firmware LSM hook
From: Jason Gunthorpe @ 2026-04-13 16:42 UTC (permalink / raw)
  To: Paul Moore
  Cc: Leon Romanovsky, 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: <CAHC9VhRnYXjg+vE9a8PeykbXk91is12zYLaO7EFdfZPKMxDfPA@mail.gmail.com>

On Sun, Apr 12, 2026 at 09:38:35PM -0400, Paul Moore wrote:
> > We are not limited to LSM solution, the goal is to intercept commands
> > which are submitted to the FW and "security" bucket sounded right to us.
> 
> Yes, it does sound "security relevant", but without a well defined
> interface/format it is going to be difficult to write a generic LSM to
> have any level of granularity beyond a basic "load firmware"
> permission.

I think to step back a bit, what this is trying to achieve is very
similar to the iptables fwmark/secmark scheme.

secmark allows the user to specify programmable rules via iptables
which results in each packet being tagged with a SELinux context and
then the userspace policy can consume that and make security decision
based on that.

Google is showing me examples of this to permit only certain processes
to use certain network addresses.

So this is exactly the same high level idea. The transport of the
packet is different (firwmare cmd vs network) but otherwise it is all
the same basic problem. We need a user programmable classifier like
iptables. Once classified we want this to work with more than SELinux
only, we have a particular interest in the eBPF LSM. In any case the
userspace should be able to specify the security policy that applies
to the kernel classified data.

Following the fwmark example, if there was some programmable in-kernel
function to convert the cmd into a SELinux label would we be able to
enable SELinux following the SECMARK design?

Would there be an objection if that in-kernel function was using a
system-wide eBPF uploaded with some fwctl uAPI?

Finally, would there be an objection to enabling the same function in
eBPF by feeding it the entire command and have it classify and make a
security decision in a single eBPF program? Is there some other way to
enable eBPF? I see eBPF doesn't interwork with SECMARK today so there
isn't a ready example?

Jason

^ permalink raw reply

* Re: LSM: Whiteout chardev creation sidesteps mknod hook
From: Stephen Smalley @ 2026-04-13 17:08 UTC (permalink / raw)
  To: Günther Noack
  Cc: Miklos Szeredi, Christian Brauner, Serge Hallyn, Amir Goldstein,
	Mickaël Salaün, Paul Moore, linux-security-module
In-Reply-To: <adzgKVIhRj5t3GqM@google.com>

On Mon, Apr 13, 2026 at 8:24 AM Günther Noack <gnoack@google.com> wrote:
>
> On Mon, Apr 13, 2026 at 12:18:08PM +0200, Miklos Szeredi wrote:
> > On Sat, 11 Apr 2026 at 10:36, Günther Noack <gnoack@google.com> wrote:
> > > I also don't currently see how an attacker would abuse this, but I still see
> > > this as a violation of Landlock's security model if we can create a policy that
> > > denies the creation of character device directory entries, and then we still
> > > have a way to make them appear there where we previously had a different file.
> >
> > Look: a whiteout is a whiteout, NOT a character device.  Don't let the
> > fact that it's represented by "c 0 0" fool you, this is a completely
> > different beast.  See commit a3c751a50fe6 ("vfs: allow unprivileged
> > whiteout creation").
> >
> > Does this beast need special handling by LSMs?  I have no idea, but
> > treating them the same as char devs sounds like a bad idea.
>
> Thanks for the pointer to that commit.  I was under the impression
> that creation of the whiteout objects required CAP_MKNOD, but it seems
> you have dropped that requirement in that commit.
>
> (FWIW, I was mislead by the rename(2) man page[1], which is apparently
> not up to date and where it explicitly says:
>
>     RENAME_WHITEOUT requires the same privileges as creating a
>     device node (i.e., the CAP_MKNOD capability).
>
> So with that assumption, it seemed natural that this should have
> extended equivalently into a Landlock policy.)
>
> So if the "c 0 0" whiteout device is indeed a different kind of file,
> maybe we would need to treat it with a separate Landlock access right
> after all then.  I'll ponder it.
>
> FWIW, besides introducing a new LANDLOCK_ACCESS_FS_MAKE_WHITEOUT
> access right and adding more special cases to the Landlock API,
> another possible option might be to just forbid creating whiteout
> objects altogether, when under a Landlock policy.  As the man page
> also notes, "This operation makes sense only for overlay/union
> filesystem implementations", and since these likely can't use Landlock
> anyway due to mounting, I think there would be no use case left where
> anyone would want to perform such an operation within a Landlock
> domain -- I don't think this would break anyone.  Mickaël, do you have
> an opinion on that idea?

Just as a point of comparison, for SELinux, I see the following
permission checks occur upon renameat2(..., RENAME_WHITEOUT):
1. add_name, write, and search permissions to the destination dir
2. rename permission to the source file
3. if the dst file already exists, unlink permission to the dst file
4. remove_name, write, and search permissions to the source dir

And the following permission checks upon mknod(..., S_IFCHR, 0):
1. add_name, write, and search permissions to the destination dir
2. create permission to the new chr_file

So we are missing the create check for the whiteout file on
renameat2(); not sure if that's working as intended or a bug.

>
> —Günther
>
> P.S. Initial patch set from Saturday is at [2], but this still uses
> the LANDLOCK_ACCESS_FS_MAKE_CHAR right.
>
> [1] https://man7.org/linux/man-pages/man2/rename.2.html
> [2] https://lore.kernel.org/all/20260411090944.3131168-2-gnoack@google.com/

^ permalink raw reply

* Re: [RFC PATCH v4 00/19] Support socket access-control
From: Mikhail Ivanov @ 2026-04-13 17:11 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: gnoack, willemdebruijn.kernel, matthieu, linux-security-module,
	netdev, netfilter-devel, yusongping, artem.kuzin,
	konstantin.meskhidze
In-Reply-To: <20260408.icooCaighie2@digikod.net>

On 4/8/2026 1:26 PM, Mickaël Salaün wrote:
> Hi Mikhail,

Hi!

> 
> On Tue, Nov 18, 2025 at 09:46:20PM +0800, Mikhail Ivanov wrote:
>> Hello! This is v4 RFC patch dedicated to socket protocols restriction.
>>
>> It is based on the landlock's mic-next branch on top of Linux 6.16-rc2
>> kernel version.
>>
>> Objective
>> =========
>> Extend Landlock with a mechanism to restrict any set of protocols in
>> a sandboxed process.
>>
>> Closes: https://github.com/landlock-lsm/linux/issues/6
>>
>> Motivation
>> ==========
>> Landlock implements the `LANDLOCK_RULE_NET_PORT` rule type, which provides
>> fine-grained control of actions for a specific protocol. Any action or
>> protocol that is not supported by this rule can not be controlled. As a
>> result, protocols for which fine-grained control is not supported can be
>> used in a sandboxed system and lead to vulnerabilities or unexpected
>> behavior.
>>
>> Controlling the protocols used will allow to use only those that are
>> necessary for the system and/or which have fine-grained Landlock control
>> through others types of rules (e.g. TCP bind/connect control with
>> `LANDLOCK_RULE_NET_PORT`, UNIX bind control with
>> `LANDLOCK_RULE_PATH_BENEATH`).
>>
>> Consider following examples:
>> * Server may want to use only TCP sockets for which there is fine-grained
>>    control of bind(2) and connect(2) actions [1].
>> * System that does not need a network or that may want to disable network
>>    for security reasons (e.g. [2]) can achieve this by restricting the use
>>    of all possible protocols.
>>
>> [1] https://lore.kernel.org/all/ZJvy2SViorgc+cZI@google.com/
>> [2] https://cr.yp.to/unix/disablenetwork.html
>>
>> Implementation
>> ==============
>> This patchset adds control over the protocols used by implementing a
>> restriction of socket creation. This is possible thanks to the new type
>> of rule - `LANDLOCK_RULE_SOCKET`, that allows to restrict actions on
>> sockets, and a new access right - `LANDLOCK_ACCESS_SOCKET_CREATE`, that
>> corresponds to user space sockets creation. The key in this rule
>> corresponds to communication protocol signature from socket(2) syscall.
> 
> FYI, I sent a new patch series that adds a handled_perm field to
> rulesets:
> https://lore.kernel.org/all/20260312100444.2609563-6-mic@digikod.net/
> See also the rationale:
> https://lore.kernel.org/all/20260312100444.2609563-12-mic@digikod.net/
> 
> I think that would work well with the socket creation permission.  WDYT?

Agreed. AFAICS restrictions of protocols used for communication (eg.TCP)
will complement restriction of network namespace which sandboxed process
is pinned by LANDLOCK_PERM_NAMESPACE_ENTER permission.

> 
> Do you think you'll be able to continue this work or would you like me
> or Günther to complete the remaining last bits (while of course keeping
> you as the main author)?

Sorry for the delay. I will finish and send patch series ASAP.

> 
> 
>>
>> The right to create a socket is checked in the LSM hook which is called
>> in the __sock_create method. The following user space operations are
>> subject to this check: socket(2), socketpair(2), io_uring(7).
>>
>> `LANDLOCK_ACCESS_SOCKET_CREATE` does not restrict socket creation
>> performed by accept(2), because created socket is used for messaging
>> between already existing endpoints.
>>
>> Design discussion
>> ===================
>> 1. Should `SCTP_SOCKOPT_PEELOFF` and socketpair(2) be restricted?
>>
>> SCTP socket can be connected to a multiple endpoints (one-to-many
>> relation). Calling setsockopt(2) on such socket with option
>> `SCTP_SOCKOPT_PEELOFF` detaches one of existing connections to a separate
>> UDP socket. This detach is currently restrictable.
>>
>> Same applies for the socketpair(2) syscall. It was noted that denying
>> usage of socketpair(2) in sandboxed environment may be not meaninful [1].
>>
>> Currently both operations use general socket interface to create sockets.
>> Therefore it's not possible to distinguish between socket(2) and those
>> operations inside security_socket_create LSM hook which is currently
>> used for protocols restriction. Providing such separation may require
>> changes in socket layer (eg. in __sock_create) interface which may not be
>> acceptable.
>>
>> [1] https://lore.kernel.org/all/ZurZ7nuRRl0Zf2iM@google.com/
>>
>> Code coverage
>> =============
>> Code coverage(gcov) report with the launch of all the landlock selftests:
>> * security/landlock:
>> lines......: 94.0% (1200 of 1276 lines)
>> functions..: 95.0% (134 of 141 functions)
>>
>> * security/landlock/socket.c:
>> lines......: 100.0% (56 of 56 lines)
>> functions..: 100.0% (5 of 5 functions)
>>
>> Currently landlock-test-tools fails on mini.kernel_socket test due to lack
>> of SMC protocol support.
>>
>> General changes v3->v4
>> ======================
>> * Implementation
>>    * Adds protocol field to landlock_socket_attr.
>>    * Adds protocol masks support via wildcards values in
>>      landlock_socket_attr.
>>    * Changes LSM hook used from socket_post_create to socket_create.
>>    * Changes protocol ranges acceptable by socket rules.
>>    * Adds audit support.
>>    * Changes ABI version to 8.
>> * Tests
>>    * Adds 5 new tests:
>>      * mini.rule_with_wildcard, protocol_wildcard.access,
>>        mini.ruleset_with_wildcards_overlap:
>>        verify rulesets containing rules with wildcard values.
>>      * tcp_protocol.alias_restriction: verify that Landlock doesn't
>>        perform protocol mappings.
>>      * audit.socket_create: tests audit denial logging.
>>    * Squashes tests corresponding to Landlock rule adding to a single commit.
>> * Documentation
>>    * Refactors Documentation/userspace-api/landlock.rst.
>> * Commits
>>    * Rebases on mic-next.
>>    * Refactors commits.
>>
>> Previous versions
>> =================
>> v3: https://lore.kernel.org/all/20240904104824.1844082-1-ivanov.mikhail1@huawei-partners.com/
>> v2: https://lore.kernel.org/all/20240524093015.2402952-1-ivanov.mikhail1@huawei-partners.com/
>> v1: https://lore.kernel.org/all/20240408093927.1759381-1-ivanov.mikhail1@huawei-partners.com/
>>
>> Mikhail Ivanov (19):
>>    landlock: Support socket access-control
>>    selftests/landlock: Test creating a ruleset with unknown access
>>    selftests/landlock: Test adding a socket rule
>>    selftests/landlock: Testing adding rule with wildcard value
>>    selftests/landlock: Test acceptable ranges of socket rule key
>>    landlock: Add hook on socket creation
>>    selftests/landlock: Test basic socket restriction
>>    selftests/landlock: Test network stack error code consistency
>>    selftests/landlock: Test overlapped rulesets with rules of protocol
>>      ranges
>>    selftests/landlock: Test that kernel space sockets are not restricted
>>    selftests/landlock: Test protocol mappings
>>    selftests/landlock: Test socketpair(2) restriction
>>    selftests/landlock: Test SCTP peeloff restriction
>>    selftests/landlock: Test that accept(2) is not restricted
>>    lsm: Support logging socket common data
>>    landlock: Log socket creation denials
>>    selftests/landlock: Test socket creation denial log for audit
>>    samples/landlock: Support socket protocol restrictions
>>    landlock: Document socket rule type support
>>
>>   Documentation/userspace-api/landlock.rst      |   48 +-
>>   include/linux/lsm_audit.h                     |    8 +
>>   include/uapi/linux/landlock.h                 |   60 +-
>>   samples/landlock/sandboxer.c                  |  118 +-
>>   security/landlock/Makefile                    |    2 +-
>>   security/landlock/access.h                    |    3 +
>>   security/landlock/audit.c                     |   12 +
>>   security/landlock/audit.h                     |    1 +
>>   security/landlock/limits.h                    |    4 +
>>   security/landlock/ruleset.c                   |   37 +-
>>   security/landlock/ruleset.h                   |   46 +-
>>   security/landlock/setup.c                     |    2 +
>>   security/landlock/socket.c                    |  198 +++
>>   security/landlock/socket.h                    |   20 +
>>   security/landlock/syscalls.c                  |   61 +-
>>   security/lsm_audit.c                          |    4 +
>>   tools/testing/selftests/landlock/base_test.c  |    2 +-
>>   tools/testing/selftests/landlock/common.h     |   14 +
>>   tools/testing/selftests/landlock/config       |   47 +
>>   tools/testing/selftests/landlock/net_test.c   |   11 -
>>   .../selftests/landlock/protocols_define.h     |  169 +++
>>   .../testing/selftests/landlock/socket_test.c  | 1169 +++++++++++++++++
>>   22 files changed, 1990 insertions(+), 46 deletions(-)
>>   create mode 100644 security/landlock/socket.c
>>   create mode 100644 security/landlock/socket.h
>>   create mode 100644 tools/testing/selftests/landlock/protocols_define.h
>>   create mode 100644 tools/testing/selftests/landlock/socket_test.c
>>
>>
>> base-commit: 6dde339a3df80a57ac3d780d8cfc14d9262e2acd
>> -- 
>> 2.34.1
>>
>>

^ permalink raw reply

* Re: [PATCH 1/7] lsm: Add granular mount hooks to replace security_sb_mount
From: Stephen Smalley @ 2026-04-13 17:14 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-security-module, linux-fsdevel, selinux, apparmor, paul,
	jmorris, serge, viro, brauner, jack, john.johansen, omosnace, mic,
	gnoack, takedakn, penguin-kernel, herton, kernel-team
In-Reply-To: <20260318184400.3502908-2-song@kernel.org>

On Wed, Mar 18, 2026 at 2:44 PM Song Liu <song@kernel.org> wrote:
>
> Add six new LSM hooks for mount operations:
>
> - mount_bind(from, to, recurse): bind mount with pre-resolved
>   struct path for source and destination.
> - mount_new(fc, mp, mnt_flags, flags, data): new mount, called after
>   mount options are parsed. The flags and data parameters carry the
>   original mount(2) flags and data for LSMs that need them (AppArmor,
>   Tomoyo).
> - mount_remount(fc, mp, mnt_flags, flags, data): filesystem remount,
>   called after mount options are parsed into the fs_context.
> - mount_reconfigure(mp, mnt_flags, flags): mount flag reconfiguration
>   (MS_REMOUNT|MS_BIND path).
> - mount_move(from, to): move mount with pre-resolved paths.
> - mount_change_type(mp, ms_flags): propagation type changes.
>
> These replace the monolithic security_sb_mount() which conflates
> multiple distinct operations into a single hook, and suffers from
> TOCTOU issues where LSMs re-resolve string-based dev_name via
> kern_path().
>
> The mount_move hook is added alongside the existing move_mount hook.
> During the transition, LSMs register for both hooks. The move_mount
> hook will be removed once all LSMs have been converted.
>
> Some LSMs, such as apparmor and tomoyo, audit the original input passed
> in the mount syscall. To keep the same behavior, argument data and flags
> are passed in do_* functions. These can be removed if these LSMs no
> longer need these information.
>
> All new hooks are registered as sleepable BPF LSM hooks.
>
> Code generated with the assistance of Claude, reviewed by human.
>
> Signed-off-by: Song Liu <song@kernel.org>

Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com # for selinux only

> ---
>  fs/namespace.c                |  35 ++++++++++--
>  include/linux/lsm_hook_defs.h |  12 ++++
>  include/linux/security.h      |  50 +++++++++++++++++
>  kernel/bpf/bpf_lsm.c          |   7 +++
>  security/security.c           | 101 ++++++++++++++++++++++++++++++++++
>  5 files changed, 199 insertions(+), 6 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 854f4fc66469..de33070e514a 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2875,6 +2875,10 @@ static int do_change_type(const struct path *path, int ms_flags)
>         if (!type)
>                 return -EINVAL;
>
> +       err = security_mount_change_type(path, ms_flags);
> +       if (err)
> +               return err;
> +
>         guard(namespace_excl)();
>
>         err = may_change_propagation(mnt);
> @@ -3007,6 +3011,10 @@ static int do_loopback(const struct path *path, const char *old_name,
>         if (err)
>                 return err;
>
> +       err = security_mount_bind(&old_path, path, recurse);
> +       if (err)
> +               return err;
> +
>         if (mnt_ns_loop(old_path.dentry))
>                 return -EINVAL;
>
> @@ -3319,7 +3327,8 @@ static void mnt_warn_timestamp_expiry(const struct path *mountpoint,
>   * superblock it refers to.  This is triggered by specifying MS_REMOUNT|MS_BIND
>   * to mount(2).
>   */
> -static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
> +static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags,
> +                             unsigned long flags)
>  {
>         struct super_block *sb = path->mnt->mnt_sb;
>         struct mount *mnt = real_mount(path->mnt);
> @@ -3334,6 +3343,10 @@ static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
>         if (!can_change_locked_flags(mnt, mnt_flags))
>                 return -EPERM;
>
> +       ret = security_mount_reconfigure(path, mnt_flags, flags);
> +       if (ret)
> +               return ret;
> +
>         /*
>          * We're only checking whether the superblock is read-only not
>          * changing it, so only take down_read(&sb->s_umount).
> @@ -3357,7 +3370,7 @@ static int do_reconfigure_mnt(const struct path *path, unsigned int mnt_flags)
>   * on it - tough luck.
>   */
>  static int do_remount(const struct path *path, int sb_flags,
> -                     int mnt_flags, void *data)
> +                     int mnt_flags, void *data, unsigned long flags)
>  {
>         int err;
>         struct super_block *sb = path->mnt->mnt_sb;
> @@ -3384,6 +3397,9 @@ static int do_remount(const struct path *path, int sb_flags,
>         fc->oldapi = true;
>
>         err = parse_monolithic_mount_data(fc, data);
> +       if (!err)
> +               err = security_mount_remount(fc, path, mnt_flags, flags,
> +                                           data);
>         if (!err) {
>                 down_write(&sb->s_umount);
>                 err = -EPERM;
> @@ -3713,6 +3729,10 @@ static int do_move_mount_old(const struct path *path, const char *old_name)
>         if (err)
>                 return err;
>
> +       err = security_mount_move(&old_path, path);
> +       if (err)
> +               return err;
> +
>         return do_move_mount(&old_path, path, 0);
>  }
>
> @@ -3791,7 +3811,7 @@ static int do_new_mount_fc(struct fs_context *fc, const struct path *mountpoint,
>   */
>  static int do_new_mount(const struct path *path, const char *fstype,
>                         int sb_flags, int mnt_flags,
> -                       const char *name, void *data)
> +                       const char *name, void *data, unsigned long flags)
>  {
>         struct file_system_type *type;
>         struct fs_context *fc;
> @@ -3835,6 +3855,9 @@ static int do_new_mount(const struct path *path, const char *fstype,
>                 err = parse_monolithic_mount_data(fc, data);
>         if (!err && !mount_capable(fc))
>                 err = -EPERM;
> +
> +       if (!err)
> +               err = security_mount_new(fc, path, mnt_flags, flags, data);
>         if (!err)
>                 err = do_new_mount_fc(fc, path, mnt_flags);
>
> @@ -4146,9 +4169,9 @@ int path_mount(const char *dev_name, const struct path *path,
>                             SB_I_VERSION);
>
>         if ((flags & (MS_REMOUNT | MS_BIND)) == (MS_REMOUNT | MS_BIND))
> -               return do_reconfigure_mnt(path, mnt_flags);
> +               return do_reconfigure_mnt(path, mnt_flags, flags);
>         if (flags & MS_REMOUNT)
> -               return do_remount(path, sb_flags, mnt_flags, data_page);
> +               return do_remount(path, sb_flags, mnt_flags, data_page, flags);
>         if (flags & MS_BIND)
>                 return do_loopback(path, dev_name, flags & MS_REC);
>         if (flags & (MS_SHARED | MS_PRIVATE | MS_SLAVE | MS_UNBINDABLE))
> @@ -4157,7 +4180,7 @@ int path_mount(const char *dev_name, const struct path *path,
>                 return do_move_mount_old(path, dev_name);
>
>         return do_new_mount(path, type_page, sb_flags, mnt_flags, dev_name,
> -                           data_page);
> +                           data_page, flags);
>  }
>
>  int do_mount(const char *dev_name, const char __user *dir_name,
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 8c42b4bde09c..6bb67059fb43 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -81,6 +81,18 @@ LSM_HOOK(int, 0, sb_clone_mnt_opts, const struct super_block *oldsb,
>          unsigned long *set_kern_flags)
>  LSM_HOOK(int, 0, move_mount, const struct path *from_path,
>          const struct path *to_path)
> +LSM_HOOK(int, 0, mount_bind, const struct path *from, const struct path *to,
> +        bool recurse)
> +LSM_HOOK(int, 0, mount_new, struct fs_context *fc, const struct path *mp,
> +        int mnt_flags, unsigned long flags, void *data)
> +LSM_HOOK(int, 0, mount_remount, struct fs_context *fc,
> +        const struct path *mp, int mnt_flags, unsigned long flags,
> +        void *data)
> +LSM_HOOK(int, 0, mount_reconfigure, const struct path *mp,
> +        unsigned int mnt_flags, unsigned long flags)
> +LSM_HOOK(int, 0, mount_move, const struct path *from_path,
> +        const struct path *to_path)
> +LSM_HOOK(int, 0, mount_change_type, const struct path *mp, int ms_flags)
>  LSM_HOOK(int, -EOPNOTSUPP, dentry_init_security, struct dentry *dentry,
>          int mode, const struct qstr *name, const char **xattr_name,
>          struct lsm_context *cp)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f..6e31de9b3d68 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -385,6 +385,17 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
>                                 unsigned long kern_flags,
>                                 unsigned long *set_kern_flags);
>  int security_move_mount(const struct path *from_path, const struct path *to_path);
> +int security_mount_bind(const struct path *from, const struct path *to,
> +                       bool recurse);
> +int security_mount_new(struct fs_context *fc, const struct path *mp,
> +                      int mnt_flags, unsigned long flags, void *data);
> +int security_mount_remount(struct fs_context *fc, const struct path *mp,
> +                          int mnt_flags, unsigned long flags, void *data);
> +int security_mount_reconfigure(const struct path *mp, unsigned int mnt_flags,
> +                              unsigned long flags);
> +int security_mount_move(const struct path *from_path,
> +                       const struct path *to_path);
> +int security_mount_change_type(const struct path *mp, int ms_flags);
>  int security_dentry_init_security(struct dentry *dentry, int mode,
>                                   const struct qstr *name,
>                                   const char **xattr_name,
> @@ -847,6 +858,45 @@ static inline int security_move_mount(const struct path *from_path,
>         return 0;
>  }
>
> +static inline int security_mount_bind(const struct path *from,
> +                                     const struct path *to, bool recurse)
> +{
> +       return 0;
> +}
> +
> +static inline int security_mount_new(struct fs_context *fc,
> +                                    const struct path *mp, int mnt_flags,
> +                                    unsigned long flags, void *data)
> +{
> +       return 0;
> +}
> +
> +static inline int security_mount_remount(struct fs_context *fc,
> +                                        const struct path *mp, int mnt_flags,
> +                                        unsigned long flags, void *data)
> +{
> +       return 0;
> +}
> +
> +static inline int security_mount_reconfigure(const struct path *mp,
> +                                            unsigned int mnt_flags,
> +                                            unsigned long flags)
> +{
> +       return 0;
> +}
> +
> +static inline int security_mount_move(const struct path *from_path,
> +                                     const struct path *to_path)
> +{
> +       return 0;
> +}
> +
> +static inline int security_mount_change_type(const struct path *mp,
> +                                            int ms_flags)
> +{
> +       return 0;
> +}
> +
>  static inline int security_path_notify(const struct path *path, u64 mask,
>                                 unsigned int obj_type)
>  {
> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index 0c4a0c8e6f70..65235d70ee23 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -383,6 +383,13 @@ BTF_ID(func, bpf_lsm_task_prctl)
>  BTF_ID(func, bpf_lsm_task_setscheduler)
>  BTF_ID(func, bpf_lsm_task_to_inode)
>  BTF_ID(func, bpf_lsm_userns_create)
> +BTF_ID(func, bpf_lsm_move_mount)
> +BTF_ID(func, bpf_lsm_mount_bind)
> +BTF_ID(func, bpf_lsm_mount_new)
> +BTF_ID(func, bpf_lsm_mount_remount)
> +BTF_ID(func, bpf_lsm_mount_reconfigure)
> +BTF_ID(func, bpf_lsm_mount_move)
> +BTF_ID(func, bpf_lsm_mount_change_type)
>  BTF_SET_END(sleepable_lsm_hooks)
>
>  BTF_SET_START(untrusted_lsm_hooks)
> diff --git a/security/security.c b/security/security.c
> index 67af9228c4e9..356ef228d5de 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1156,6 +1156,107 @@ int security_move_mount(const struct path *from_path,
>         return call_int_hook(move_mount, from_path, to_path);
>  }
>
> +/**
> + * security_mount_bind() - Check permissions for a bind mount
> + * @from: source path
> + * @to: destination mount point
> + * @recurse: whether this is a recursive bind mount
> + *
> + * Check permission before a bind mount is performed. Called with the
> + * source path already resolved, eliminating TOCTOU issues with
> + * string-based dev_name in security_sb_mount().
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_mount_bind(const struct path *from, const struct path *to,
> +                       bool recurse)
> +{
> +       return call_int_hook(mount_bind, from, to, recurse);
> +}
> +
> +/**
> + * security_mount_new() - Check permissions for a new mount
> + * @fc: filesystem context with parsed options
> + * @mp: mount point path
> + * @mnt_flags: mount flags (MNT_*)
> + * @flags: original mount flags (MS_*, used by AppArmor/Tomoyo)
> + * @data: filesystem specific data (used by AppArmor)
> + *
> + * Check permission before a new filesystem is mounted. Called after
> + * mount options are parsed, providing access to the fs_context.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_mount_new(struct fs_context *fc, const struct path *mp,
> +                      int mnt_flags, unsigned long flags, void *data)
> +{
> +       return call_int_hook(mount_new, fc, mp, mnt_flags, flags, data);
> +}
> +
> +/**
> + * security_mount_remount() - Check permissions for a remount
> + * @fc: filesystem context with parsed options
> + * @mp: mount point path
> + * @mnt_flags: mount flags (MNT_*)
> + * @flags: original mount flags (MS_*, used by AppArmor/Tomoyo)
> + * @data: filesystem specific data (used by AppArmor)
> + *
> + * Check permission before a filesystem is remounted. Called after
> + * mount options are parsed, providing access to the fs_context.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_mount_remount(struct fs_context *fc, const struct path *mp,
> +                          int mnt_flags, unsigned long flags, void *data)
> +{
> +       return call_int_hook(mount_remount, fc, mp, mnt_flags, flags, data);
> +}
> +
> +/**
> + * security_mount_reconfigure() - Check permissions for mount reconfiguration
> + * @mp: mount point path
> + * @mnt_flags: new mount flags (MNT_*)
> + * @flags: original mount flags (MS_*, used by AppArmor/Tomoyo)
> + *
> + * Check permission before mount flags are reconfigured (MS_REMOUNT|MS_BIND).
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_mount_reconfigure(const struct path *mp, unsigned int mnt_flags,
> +                              unsigned long flags)
> +{
> +       return call_int_hook(mount_reconfigure, mp, mnt_flags, flags);
> +}
> +
> +/**
> + * security_mount_move() - Check permissions for moving a mount
> + * @from_path: source mount path
> + * @to_path: destination mount point path
> + *
> + * Check permission before a mount is moved.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_mount_move(const struct path *from_path,
> +                       const struct path *to_path)
> +{
> +       return call_int_hook(mount_move, from_path, to_path);
> +}
> +
> +/**
> + * security_mount_change_type() - Check permissions for propagation changes
> + * @mp: mount point path
> + * @ms_flags: propagation flags (MS_SHARED, MS_PRIVATE, etc.)
> + *
> + * Check permission before mount propagation type is changed.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_mount_change_type(const struct path *mp, int ms_flags)
> +{
> +       return call_int_hook(mount_change_type, mp, ms_flags);
> +}
> +
>  /**
>   * security_path_notify() - Check if setting a watch is allowed
>   * @path: file path
> --
> 2.52.0
>

^ permalink raw reply

* Re: [PATCH 4/7] selinux: Convert from sb_mount to granular mount hooks
From: Stephen Smalley @ 2026-04-13 17:16 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-security-module, linux-fsdevel, selinux, apparmor, paul,
	jmorris, serge, viro, brauner, jack, john.johansen, omosnace, mic,
	gnoack, takedakn, penguin-kernel, herton, kernel-team
In-Reply-To: <20260318184400.3502908-5-song@kernel.org>

On Wed, Mar 18, 2026 at 2:44 PM Song Liu <song@kernel.org> wrote:
>
> Replace selinux_mount() with granular mount hooks, preserving the
> same permission checks:
>
> - mount_bind, mount_new, mount_change_type: FILE__MOUNTON
> - mount_remount, mount_reconfigure: FILESYSTEM__REMOUNT
> - mount_move: FILE__MOUNTON (reuses selinux_move_mount)
>
> The flags and data parameters are unused by SELinux.
>
> Code generated with the assistance of Claude, reviewed by human.
>
> Signed-off-by: Song Liu <song@kernel.org>

Not expecting you to do this, but after this lands, I think it would
make sense to revisit the SELinux checks and further specialize them
while providing backward compatibility.

Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com

> ---
>  security/selinux/hooks.c | 47 ++++++++++++++++++++++++++++++----------
>  1 file changed, 35 insertions(+), 12 deletions(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index d8224ea113d1..415b5541ab9e 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -2778,19 +2778,37 @@ static int selinux_sb_statfs(struct dentry *dentry)
>         return superblock_has_perm(cred, dentry->d_sb, FILESYSTEM__GETATTR, &ad);
>  }
>
> -static int selinux_mount(const char *dev_name,
> -                        const struct path *path,
> -                        const char *type,
> -                        unsigned long flags,
> -                        void *data)
> +static int selinux_mount_bind(const struct path *from, const struct path *to,
> +                             bool recurse)
>  {
> -       const struct cred *cred = current_cred();
> +       return path_has_perm(current_cred(), to, FILE__MOUNTON);
> +}
>
> -       if (flags & MS_REMOUNT)
> -               return superblock_has_perm(cred, path->dentry->d_sb,
> -                                          FILESYSTEM__REMOUNT, NULL);
> -       else
> -               return path_has_perm(cred, path, FILE__MOUNTON);
> +static int selinux_mount_new(struct fs_context *fc, const struct path *mp,
> +                            int mnt_flags, unsigned long flags, void *data)
> +{
> +       return path_has_perm(current_cred(), mp, FILE__MOUNTON);
> +}
> +
> +static int selinux_mount_remount(struct fs_context *fc, const struct path *mp,
> +                                int mnt_flags, unsigned long flags,
> +                                void *data)
> +{
> +       return superblock_has_perm(current_cred(), fc->root->d_sb,
> +                                  FILESYSTEM__REMOUNT, NULL);
> +}
> +
> +static int selinux_mount_reconfigure(const struct path *mp,
> +                                    unsigned int mnt_flags,
> +                                    unsigned long flags)
> +{
> +       return superblock_has_perm(current_cred(), mp->dentry->d_sb,
> +                                  FILESYSTEM__REMOUNT, NULL);
> +}
> +
> +static int selinux_mount_change_type(const struct path *mp, int ms_flags)
> +{
> +       return path_has_perm(current_cred(), mp, FILE__MOUNTON);
>  }
>
>  static int selinux_move_mount(const struct path *from_path,
> @@ -7449,7 +7467,12 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
>         LSM_HOOK_INIT(sb_kern_mount, selinux_sb_kern_mount),
>         LSM_HOOK_INIT(sb_show_options, selinux_sb_show_options),
>         LSM_HOOK_INIT(sb_statfs, selinux_sb_statfs),
> -       LSM_HOOK_INIT(sb_mount, selinux_mount),
> +       LSM_HOOK_INIT(mount_bind, selinux_mount_bind),
> +       LSM_HOOK_INIT(mount_new, selinux_mount_new),
> +       LSM_HOOK_INIT(mount_remount, selinux_mount_remount),
> +       LSM_HOOK_INIT(mount_reconfigure, selinux_mount_reconfigure),
> +       LSM_HOOK_INIT(mount_change_type, selinux_mount_change_type),
> +       LSM_HOOK_INIT(mount_move, selinux_move_mount),
>         LSM_HOOK_INIT(sb_umount, selinux_umount),
>         LSM_HOOK_INIT(sb_set_mnt_opts, selinux_set_mnt_opts),
>         LSM_HOOK_INIT(sb_clone_mnt_opts, selinux_sb_clone_mnt_opts),
> --
> 2.52.0
>

^ permalink raw reply

* Re: [PATCH 7/7] lsm: Remove security_sb_mount and security_move_mount
From: Stephen Smalley @ 2026-04-13 17:18 UTC (permalink / raw)
  To: Song Liu
  Cc: linux-security-module, linux-fsdevel, selinux, apparmor, paul,
	jmorris, serge, viro, brauner, jack, john.johansen, omosnace, mic,
	gnoack, takedakn, penguin-kernel, kernel-team
In-Reply-To: <20260318184400.3502908-8-song@kernel.org>

On Wed, Mar 18, 2026 at 2:44 PM Song Liu <song@kernel.org> wrote:
>
> Now that all LSMs have been converted to granular mount hooks,
> remove the old hooks:
>
> - security_sb_mount(): removed from lsm_hook_defs.h, security.h,
>   security.c, and its call in path_mount().
> - security_move_mount(): removed and replaced by security_mount_move()
>   in do_move_mount(). All LSMs now use mount_move exclusively.
>
> Code generated with the assistance of Claude, reviewed by human.
>
> Signed-off-by: Song Liu <song@kernel.org>

Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>
Tested-by: Stephen Smalley <stephen.smalley.work@gmail.com # for selinux only

> ---
>  fs/namespace.c                |  6 +-----
>  include/linux/lsm_hook_defs.h |  4 ----
>  include/linux/security.h      | 16 ---------------
>  kernel/bpf/bpf_lsm.c          |  2 --
>  security/apparmor/lsm.c       |  1 -
>  security/landlock/fs.c        |  1 -
>  security/security.c           | 38 -----------------------------------
>  security/selinux/hooks.c      |  2 --
>  8 files changed, 1 insertion(+), 69 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index de33070e514a..ba5baccdde67 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -4108,7 +4108,6 @@ int path_mount(const char *dev_name, const struct path *path,
>                 const char *type_page, unsigned long flags, void *data_page)
>  {
>         unsigned int mnt_flags = 0, sb_flags;
> -       int ret;
>
>         /* Discard magic */
>         if ((flags & MS_MGC_MSK) == MS_MGC_VAL)
> @@ -4121,9 +4120,6 @@ int path_mount(const char *dev_name, const struct path *path,
>         if (flags & MS_NOUSER)
>                 return -EINVAL;
>
> -       ret = security_sb_mount(dev_name, path, type_page, flags, data_page);
> -       if (ret)
> -               return ret;
>         if (!may_mount())
>                 return -EPERM;
>         if (flags & SB_MANDLOCK)
> @@ -4538,7 +4534,7 @@ static inline int vfs_move_mount(const struct path *from_path,
>  {
>         int ret;
>
> -       ret = security_move_mount(from_path, to_path);
> +       ret = security_mount_move(from_path, to_path);
>         if (ret)
>                 return ret;
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 6bb67059fb43..95537574c40b 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -69,8 +69,6 @@ LSM_HOOK(int, 0, sb_remount, struct super_block *sb, void *mnt_opts)
>  LSM_HOOK(int, 0, sb_kern_mount, const struct super_block *sb)
>  LSM_HOOK(int, 0, sb_show_options, struct seq_file *m, struct super_block *sb)
>  LSM_HOOK(int, 0, sb_statfs, struct dentry *dentry)
> -LSM_HOOK(int, 0, sb_mount, const char *dev_name, const struct path *path,
> -        const char *type, unsigned long flags, void *data)
>  LSM_HOOK(int, 0, sb_umount, struct vfsmount *mnt, int flags)
>  LSM_HOOK(int, 0, sb_pivotroot, const struct path *old_path,
>          const struct path *new_path)
> @@ -79,8 +77,6 @@ LSM_HOOK(int, 0, sb_set_mnt_opts, struct super_block *sb, void *mnt_opts,
>  LSM_HOOK(int, 0, sb_clone_mnt_opts, const struct super_block *oldsb,
>          struct super_block *newsb, unsigned long kern_flags,
>          unsigned long *set_kern_flags)
> -LSM_HOOK(int, 0, move_mount, const struct path *from_path,
> -        const struct path *to_path)
>  LSM_HOOK(int, 0, mount_bind, const struct path *from, const struct path *to,
>          bool recurse)
>  LSM_HOOK(int, 0, mount_new, struct fs_context *fc, const struct path *mp,
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 6e31de9b3d68..3610a49304c6 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -372,8 +372,6 @@ int security_sb_remount(struct super_block *sb, void *mnt_opts);
>  int security_sb_kern_mount(const struct super_block *sb);
>  int security_sb_show_options(struct seq_file *m, struct super_block *sb);
>  int security_sb_statfs(struct dentry *dentry);
> -int security_sb_mount(const char *dev_name, const struct path *path,
> -                     const char *type, unsigned long flags, void *data);
>  int security_sb_umount(struct vfsmount *mnt, int flags);
>  int security_sb_pivotroot(const struct path *old_path, const struct path *new_path);
>  int security_sb_set_mnt_opts(struct super_block *sb,
> @@ -384,7 +382,6 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
>                                 struct super_block *newsb,
>                                 unsigned long kern_flags,
>                                 unsigned long *set_kern_flags);
> -int security_move_mount(const struct path *from_path, const struct path *to_path);
>  int security_mount_bind(const struct path *from, const struct path *to,
>                         bool recurse);
>  int security_mount_new(struct fs_context *fc, const struct path *mp,
> @@ -818,13 +815,6 @@ static inline int security_sb_statfs(struct dentry *dentry)
>         return 0;
>  }
>
> -static inline int security_sb_mount(const char *dev_name, const struct path *path,
> -                                   const char *type, unsigned long flags,
> -                                   void *data)
> -{
> -       return 0;
> -}
> -
>  static inline int security_sb_umount(struct vfsmount *mnt, int flags)
>  {
>         return 0;
> @@ -852,12 +842,6 @@ static inline int security_sb_clone_mnt_opts(const struct super_block *oldsb,
>         return 0;
>  }
>
> -static inline int security_move_mount(const struct path *from_path,
> -                                     const struct path *to_path)
> -{
> -       return 0;
> -}
> -
>  static inline int security_mount_bind(const struct path *from,
>                                       const struct path *to, bool recurse)
>  {
> diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> index 65235d70ee23..3e61c54f9b48 100644
> --- a/kernel/bpf/bpf_lsm.c
> +++ b/kernel/bpf/bpf_lsm.c
> @@ -350,7 +350,6 @@ BTF_ID(func, bpf_lsm_release_secctx)
>  BTF_ID(func, bpf_lsm_sb_alloc_security)
>  BTF_ID(func, bpf_lsm_sb_eat_lsm_opts)
>  BTF_ID(func, bpf_lsm_sb_kern_mount)
> -BTF_ID(func, bpf_lsm_sb_mount)
>  BTF_ID(func, bpf_lsm_sb_remount)
>  BTF_ID(func, bpf_lsm_sb_set_mnt_opts)
>  BTF_ID(func, bpf_lsm_sb_show_options)
> @@ -383,7 +382,6 @@ BTF_ID(func, bpf_lsm_task_prctl)
>  BTF_ID(func, bpf_lsm_task_setscheduler)
>  BTF_ID(func, bpf_lsm_task_to_inode)
>  BTF_ID(func, bpf_lsm_userns_create)
> -BTF_ID(func, bpf_lsm_move_mount)
>  BTF_ID(func, bpf_lsm_mount_bind)
>  BTF_ID(func, bpf_lsm_mount_new)
>  BTF_ID(func, bpf_lsm_mount_remount)
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 7fe774535992..13a8049b1b59 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -1713,7 +1713,6 @@ static struct security_hook_list apparmor_hooks[] __ro_after_init = {
>         LSM_HOOK_INIT(capget, apparmor_capget),
>         LSM_HOOK_INIT(capable, apparmor_capable),
>
> -       LSM_HOOK_INIT(move_mount, apparmor_move_mount),
>         LSM_HOOK_INIT(mount_bind, apparmor_mount_bind),
>         LSM_HOOK_INIT(mount_new, apparmor_mount_new),
>         LSM_HOOK_INIT(mount_remount, apparmor_mount_remount),
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 6e810550efcb..5f723a70baa4 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1857,7 +1857,6 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
>         LSM_HOOK_INIT(mount_reconfigure, hook_mount_reconfigure),
>         LSM_HOOK_INIT(mount_change_type, hook_mount_change_type),
>         LSM_HOOK_INIT(mount_move, hook_move_mount),
> -       LSM_HOOK_INIT(move_mount, hook_move_mount),
>         LSM_HOOK_INIT(sb_umount, hook_sb_umount),
>         LSM_HOOK_INIT(sb_remount, hook_sb_remount),
>         LSM_HOOK_INIT(sb_pivotroot, hook_sb_pivotroot),
> diff --git a/security/security.c b/security/security.c
> index 356ef228d5de..af95868af34d 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1039,29 +1039,6 @@ int security_sb_statfs(struct dentry *dentry)
>         return call_int_hook(sb_statfs, dentry);
>  }
>
> -/**
> - * security_sb_mount() - Check permission for mounting a filesystem
> - * @dev_name: filesystem backing device
> - * @path: mount point
> - * @type: filesystem type
> - * @flags: mount flags
> - * @data: filesystem specific data
> - *
> - * Check permission before an object specified by @dev_name is mounted on the
> - * mount point named by @nd.  For an ordinary mount, @dev_name identifies a
> - * device if the file system type requires a device.  For a remount
> - * (@flags & MS_REMOUNT), @dev_name is irrelevant.  For a loopback/bind mount
> - * (@flags & MS_BIND), @dev_name identifies the        pathname of the object being
> - * mounted.
> - *
> - * Return: Returns 0 if permission is granted.
> - */
> -int security_sb_mount(const char *dev_name, const struct path *path,
> -                     const char *type, unsigned long flags, void *data)
> -{
> -       return call_int_hook(sb_mount, dev_name, path, type, flags, data);
> -}
> -
>  /**
>   * security_sb_umount() - Check permission for unmounting a filesystem
>   * @mnt: mounted filesystem
> @@ -1141,21 +1118,6 @@ int security_sb_clone_mnt_opts(const struct super_block *oldsb,
>  }
>  EXPORT_SYMBOL(security_sb_clone_mnt_opts);
>
> -/**
> - * security_move_mount() - Check permissions for moving a mount
> - * @from_path: source mount point
> - * @to_path: destination mount point
> - *
> - * Check permission before a mount is moved.
> - *
> - * Return: Returns 0 if permission is granted.
> - */
> -int security_move_mount(const struct path *from_path,
> -                       const struct path *to_path)
> -{
> -       return call_int_hook(move_mount, from_path, to_path);
> -}
> -
>  /**
>   * security_mount_bind() - Check permissions for a bind mount
>   * @from: source path
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 415b5541ab9e..446e9e242134 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -7477,8 +7477,6 @@ static struct security_hook_list selinux_hooks[] __ro_after_init = {
>         LSM_HOOK_INIT(sb_set_mnt_opts, selinux_set_mnt_opts),
>         LSM_HOOK_INIT(sb_clone_mnt_opts, selinux_sb_clone_mnt_opts),
>
> -       LSM_HOOK_INIT(move_mount, selinux_move_mount),
> -
>         LSM_HOOK_INIT(dentry_init_security, selinux_dentry_init_security),
>         LSM_HOOK_INIT(dentry_create_files_as, selinux_dentry_create_files_as),
>
> --
> 2.52.0
>

^ permalink raw reply

* Re: [PATCH] security: remove BUG_ON in security_skb_classify_flow
From: Stephen Smalley @ 2026-04-13 17:37 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: Jiayuan Chen, Stephen Smalley, linux-security-module, paul,
	jmorris, linux-kernel, Kaiyan Mei, Yinhao Hu, Dongliang Mu
In-Reply-To: <admI7uhx8OZ5NzFS@mail.hallyn.com>

On Fri, Apr 10, 2026 at 7:36 PM Serge E. Hallyn <serge@hallyn.com> wrote:
>
> On Fri, Apr 10, 2026 at 09:56:22AM +0800, Jiayuan Chen wrote:
> >
> > On 4/10/26 8:58 AM, Serge E. Hallyn wrote:
> > > On Wed, Apr 08, 2026 at 07:42:57PM +0800, Jiayuan Chen wrote:
> > > > A BPF program attached to the xfrm_decode_session hook can return a
> > > > non-zero value, which causes BUG_ON(rc) in security_skb_classify_flow()
> > > > to trigger a kernel panic.
> > > It would seem worth it to have pointed at the previous discussion at
> > >
> > > https://lore.kernel.org/all/CAEjxPJ5aA01in+Z1yLF1cwe-3uqL_E8SKGK4J294D5eRG5__5Q@mail.gmail.com/
> > >
> > > Based on that, I guess this is probably ok, but still,
> > >
> > > > Remove the BUG_ON and change the return type from void to int, so that
> > > > callers can optionally handle the error.
> > > but you don't have the existing callers handling the error.  It's
> > > conceivable they won't care, but it's also possible that they were
> > > counting on a BUG_ON in that case.
> > >
> > > What *should* callers (icmp_reply, etc) do if an error code is
> > > returned?  Should they ignore it?  In that case, would it be
> > > better to change security_skb_classify_flow() to return void?
> > >
> > Thanks for your pointer.
> >
> > So I think Feng's patch is sufficient and can by applied ?
>
> Well, selinux_xfrm_decode_session() calls selinux_xfrm_skb_sid_ingress()
> which *can* return -EINVAL.
>
> So I'd like to know, what is supposed to happen in that case?
>
> Stephen, do you know?  Is it safe for callers to ignore this?

I'm in favor of dropping the BUG_ON() from
security_skb_classify_flow() and just make it return void, ignoring
any non-zero return from xfrm_decode_session.
A slightly cleaner approach would be to introduce a separate LSM hook
for skb_classify_flow() that returns void rather than calling
xfrm_decode_session() from security_skb_classify_flow(); then any bug
handling can be done in the individual security module.

^ permalink raw reply

* Re: [PATCH] lsm: Fix the crash issue in xfrm_decode_session
From: Stephen Smalley @ 2026-04-13 17:39 UTC (permalink / raw)
  To: Feng Yang; +Cc: paul, jmorris, serge, linux-security-module, linux-kernel
In-Reply-To: <20260318061925.134954-1-yangfeng59949@163.com>

On Wed, Mar 18, 2026 at 2:20 AM Feng Yang <yangfeng59949@163.com> wrote:
>
> From: Feng Yang <yangfeng@kylinos.cn>
>
> After hooking the following BPF program:
> SEC("lsm/xfrm_decode_session")
> int BPF_PROG(lsm_hook_xfrm_decode_session, struct sk_buff *skb, u32 *secid, int ckall)
> {
>     return 1; // Any non-zero value
> }
> Subsequent packet transmission triggers will cause a kernel panic:
>
> [  112.838874] ------------[ cut here ]------------
> [  112.838895] kernel BUG at security/security.c:5282!
> [  112.838902] invalid opcode: 0000 [#1] PREEMPT SMP PTI
> [  112.838905] CPU: 5 PID: 4962 Comm: test Kdump: loaded Not tainted 6.19.0-rc5-gae23bc81ddf7 #2 PREEMPT(full)
> [  112.838907] Source Version: 55e2f799c748c8e195569363edbd1d6a4159675a
> [  112.838908] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS VirtualBox 12/01/2006
> [  112.838909] RIP: 0010:security_skb_classify_flow+0x3f/0x50
> [  112.838914] Code: 85 db 74 28 49 89 fc 48 8d 6e 14 eb 08 48 8b 1b 48 85 db 74 17 31 d2 48 8b 43 18 48 89 ee 4c 89 e7 e8 05 33 86 00 85 c0 74 e3 <0f> 0b 5b 5d 41 5c c3 cc cc cc cc 66 0f 1f 44 00 00 90 90 90 90 90
> [  112.838915] RSP: 0018:ffffc28400200b10 EFLAGS: 00010202
> [  112.838918] RAX: 0000000000000001 RBX: ffffffff91d346d8 RCX: 0000000000000000
> [  112.838919] RDX: ffffa0890f5eaf80 RSI: 0000000000000001 RDI: ffffa0890f5eaf80
> [  112.838920] RBP: ffffc28400200d04 R08: 00000000000000c7 R09: 0000000000000002
> [  112.838922] R10: 0000000000000000 R11: 000000000000000f R12: ffffa089086dedc0
> [  112.838923] R13: ffffc28400200cf0 R14: ffffa08901ab2000 R15: 0000000000000000
> [  112.838926] FS:  00007fb087dd2680(0000) GS:ffffa0891ba80000(0000) knlGS:0000000000000000
> [  112.838927] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  112.838929] CR2: 00007fb087d1b940 CR3: 0000000107520006 CR4: 00000000000706e0
> [  112.838930] Call Trace:
> [  112.838931]  <IRQ>
> [  112.838933]  icmp_route_lookup.constprop.0+0xd7/0x460
> [  112.838941]  ? switch_hrtimer_base+0x135/0x180
> [  112.838944]  ? update_sg_lb_stats+0x9c/0x440
> [  112.838949]  __icmp_send+0x3d3/0x740
> [  112.838952]  ? __udp4_lib_rcv+0x427/0x6f0
> [  112.838955]  __udp4_lib_rcv+0x427/0x6f0
> [  112.838957]  ip_protocol_deliver_rcu+0xb7/0x170
> [  112.838960]  ip_local_deliver_finish+0x76/0xa0
> [  112.838961]  __netif_receive_skb_one_core+0x89/0xa0
> [  112.838967]  process_backlog+0x95/0x140
> [  112.838969]  __napi_poll+0x2b/0x1c0
> [  112.838971]  net_rx_action+0x2aa/0x3a0
> [  112.838972]  ? swake_up_one+0x41/0x70
> [  112.838974]  ? kvm_sched_clock_read+0x11/0x20
> [  112.838977]  handle_softirqs+0xe3/0x2e0
> [  112.838980]  do_softirq+0x43/0x60
> [  112.838982]  </IRQ>
> [  112.838982]  <TASK>
> [  112.838983]  __local_bh_enable_ip+0x68/0x70
> [  112.838985]  __dev_queue_xmit+0x1c4/0x820
> [  112.838987]  ? nf_hook_slow+0x45/0xd0
> [  112.838989]  ip_finish_output2+0x1da/0x4a0
> [  112.838992]  ip_send_skb+0x86/0x90
> [  112.838994]  udp_send_skb+0x15e/0x380
> [  112.838996]  udp_sendmsg+0xb9a/0xf80
> [  112.838998]  ? __pfx_ip_generic_getfrag+0x10/0x10
> [  112.839003]  ? __sys_sendto+0x1e4/0x210
> [  112.839005]  __sys_sendto+0x1e4/0x210
> [  112.839007]  ? __handle_mm_fault+0x2fc/0x6c0
> [  112.839013]  __x64_sys_sendto+0x24/0x30
> [  112.839014]  do_syscall_64+0x5f/0x270
> [  112.839017]  entry_SYSCALL_64_after_hwframe+0x76/0xe0
> [  112.839020] RIP: 0033:0x7fb087cfdb17
> [  112.839021] Code: 0c 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b8 0f 1f 00 f3 0f 1e fa 80 3d 55 c8 0c 00 00 41 89 ca 74 10 b8 2c 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 71 c3 55 48 83 ec 30 44 89 4c 24 2c 4c 89 44
> [  112.839023] RSP: 002b:00007ffea64704e8 EFLAGS: 00000202 ORIG_RAX: 000000000000002c
> [  112.839025] RAX: ffffffffffffffda RBX: 00007ffea6470638 RCX: 00007fb087cfdb17
> [  112.839026] RDX: 0000000000000008 RSI: 00007ffea64704f8 RDI: 0000000000000003
> [  112.839027] RBP: 00007ffea6470520 R08: 00007ffea6470500 R09: 0000000000000010
> [  112.839029] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
> [  112.839030] R13: 00007ffea6470648 R14: 0000000000403df0 R15: 00007fb087e15000
> [  112.839032]  </TASK>
>
> This BUG_ON was first mentioned in [1], but I could not find any explanatory record of why this check is needed.
>
> [1] https://lore.kernel.org/all/Pine.LNX.4.64.0607122149070.573@d.namei/
>
> In the existing LSM_HOOK_INIT(xfrm_decode_session, selinux_xfrm_decode_session),
> when the `ckall` parameter of the `selinux_xfrm_decode_session` function is 0,
> it can only return 0 and will not trigger BUG_ON.
> Therefore, remove the BUG_ON check to fix this issue.
>
> Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> Closes: https://lore.kernel.org/all/4c4d04ba.6c12b.19c039b69e6.Coremail.kaiyanm@hust.edu.cn/
> Signed-off-by: Feng Yang <yangfeng@kylinos.cn>

Reviewed-by: Stephen Smalley <stephen.smalley.work@gmail.com>

With the proviso that we likely ought to follow up with a clean-up
that introduces a separate skb_classify_flow LSM hook that returns
void so we don't awkwardly ignore errors below and defer handling to
the individual security module.

> ---
>  security/security.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index 67af9228c4e9..198f650070da 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4991,10 +4991,7 @@ int security_xfrm_decode_session(struct sk_buff *skb, u32 *secid)
>
>  void security_skb_classify_flow(struct sk_buff *skb, struct flowi_common *flic)
>  {
> -       int rc = call_int_hook(xfrm_decode_session, skb, &flic->flowic_secid,
> -                              0);
> -
> -       BUG_ON(rc);
> +       call_int_hook(xfrm_decode_session, skb, &flic->flowic_secid, 0);
>  }
>  EXPORT_SYMBOL(security_skb_classify_flow);
>  #endif /* CONFIG_SECURITY_NETWORK_XFRM */
> --
> 2.43.0
>
>

^ permalink raw reply

* Re: [PATCH v2 0/4] Firmware LSM hook
From: Casey Schaufler @ 2026-04-13 17:36 UTC (permalink / raw)
  To: Jason Gunthorpe, Paul Moore
  Cc: Leon Romanovsky, 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, Casey Schaufler
In-Reply-To: <20260413164220.GP3694781@ziepe.ca>

On 4/13/2026 9:42 AM, Jason Gunthorpe wrote:
> On Sun, Apr 12, 2026 at 09:38:35PM -0400, Paul Moore wrote:
>>> We are not limited to LSM solution, the goal is to intercept commands
>>> which are submitted to the FW and "security" bucket sounded right to us.
>> Yes, it does sound "security relevant", but without a well defined
>> interface/format it is going to be difficult to write a generic LSM to
>> have any level of granularity beyond a basic "load firmware"
>> permission.
> I think to step back a bit, what this is trying to achieve is very
> similar to the iptables fwmark/secmark scheme.
>
> secmark allows the user to specify programmable rules via iptables
> which results in each packet being tagged with a SELinux context and
> then the userspace policy can consume that and make security decision
> based on that.

If you want to pursue something like this DO NOT USE A u32 TO REPRESENT
THE SECURITY CONTEXT! Use a struct lsm_context pointer. The limitations
imposed by a "secid" don't show up in SELinux, which introduced them, but
they sure do in Smack, and they really gum up the works for general LSM
stacking.


^ permalink raw reply

* Re: [PATCH v2 0/4] Firmware LSM hook
From: Casey Schaufler @ 2026-04-13 19:09 UTC (permalink / raw)
  To: Jason Gunthorpe, Paul Moore
  Cc: Leon Romanovsky, 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, Casey Schaufler
In-Reply-To: <bd7d139f-b5d6-42e1-be2c-4f71feed63cf@schaufler-ca.com>

On 4/13/2026 10:36 AM, Casey Schaufler wrote:
> On 4/13/2026 9:42 AM, Jason Gunthorpe wrote:
>> On Sun, Apr 12, 2026 at 09:38:35PM -0400, Paul Moore wrote:
>>>> We are not limited to LSM solution, the goal is to intercept commands
>>>> which are submitted to the FW and "security" bucket sounded right to us.
>>> Yes, it does sound "security relevant", but without a well defined
>>> interface/format it is going to be difficult to write a generic LSM to
>>> have any level of granularity beyond a basic "load firmware"
>>> permission.
>> I think to step back a bit, what this is trying to achieve is very
>> similar to the iptables fwmark/secmark scheme.
>>
>> secmark allows the user to specify programmable rules via iptables
>> which results in each packet being tagged with a SELinux context and
>> then the userspace policy can consume that and make security decision
>> based on that.
> If you want to pursue something like this DO NOT USE A u32 TO REPRESENT
> THE SECURITY CONTEXT! Use a struct lsm_context pointer. The limitations
> imposed by a "secid" don't show up in SELinux, which introduced them, but
> they sure do in Smack, and they really gum up the works for general LSM
> stacking.


Whoops. I meant a struct lsm_prop pointer. It must be Monday morning.


^ permalink raw reply

* Re: landlock: Add support for chmod and chown system calls families
From: Jeffrey Bencteux @ 2026-04-13 19:51 UTC (permalink / raw)
  To: Günther Noack
  Cc: mic, paul, jmorris, serge, linux-security-module, xiujianfeng
In-Reply-To: <adzjW7t7kJjne6Ze@google.com>

Hi Günther,

On Mon, Apr 13, 2026 at 02:36:43PM +0200, Günther Noack wrote:
> Before you start your investigation completely from scratch,
> did you see the prior work on this topic?
> 
> * https://github.com/landlock-lsm/linux/issues/11
> * https://lore.kernel.org/all/20220822114701.26975-1-xiujianfeng@huawei.com/

I missed it, thanks for pointing it out.

> That specific patchset was unfortunately abandoned at the time, but I
> suspect that some of the discussion still applies for your patchset as
> well?

Indeed, my feeling it that Xiu's patchset is more elaborate than mine.

> In my understanding, it was in the end blocked on a LSM hook change.
> (If this is needed, a common approach for doing that hook change is to
> add it to the same patch series as one of the earliest commits.)

To my understanding, it is too. The implementation of
LANDLOCK_ACCESS_FS_(READ|WRITE)_METADATA are tied to several LSM hooks
changes (currently working with dentry/inode and not struct path as
arguments as discussed here:
https://lore.kernel.org/all/df99abcc-e7ec-ad34-27fa-25abee28a300@digikod.net


^ permalink raw reply

* Re: [PATCH v2 0/4] Firmware LSM hook
From: Paul Moore @ 2026-04-13 22:36 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, 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: <20260413164220.GP3694781@ziepe.ca>

On Mon, Apr 13, 2026 at 12:42 PM Jason Gunthorpe <jgg@ziepe.ca> wrote:
> On Sun, Apr 12, 2026 at 09:38:35PM -0400, Paul Moore wrote:
> > > We are not limited to LSM solution, the goal is to intercept commands
> > > which are submitted to the FW and "security" bucket sounded right to us.
> >
> > Yes, it does sound "security relevant", but without a well defined
> > interface/format it is going to be difficult to write a generic LSM to
> > have any level of granularity beyond a basic "load firmware"
> > permission.
>
> I think to step back a bit, what this is trying to achieve is very
> similar to the iptables fwmark/secmark scheme.

Points for thinking outside the box a bit, but from what I've seen
thus far, it differs from secmark in a few important areas.  The
secmark concept relies on the admin to configure the network stack to
apply secmark labels to network traffic as it flows through the
system, the LSM then applies security policy to these packets based on
their label.  The firmware LSM hooks, at least as I currently
understand them, rely on the LSM hook callback to parse the firmware
op/request and apply a security policy to the request.

We've already talked about the first issue, parsing the request, and
my suggestion was to make the LSM hook call from within the firmware
(the firmware must have some way to call into the kernel/driver code,
no?) so that only the firmware would need to parse the request.  If we
wanted to adopt a secmark-esque approach, one could develop a second
parsing mechanism that would be responsible for assigning a LSM label
to the request, and then pass the firmware request to the LSM, but I
do worry a bit about the added complexity associated with keeping the
parser sync'd with the driver/fw.

However, even if we solve the parsing problem, I worry we have
another, closely related issue, of having to categorize all of the
past, present, and future firmware requests into a set of LSM specific
actions.  The past and present requests are just a matter of code,
that isn't too worrying, but what do we do about unknown future
requests?  Beyond simply encoding the request into a operation
token/enum/int, what additional information beyond the action type
would a LSM need to know to apply a security policy?  Would it be
reasonable to blindly allow or reject unknown requests?  If so, what
would break?

> ... Once classified we want this to work with more than SELinux
> only, we have a particular interest in the eBPF LSM.

One of the design requirements for the LSM framework is that it
provides an abstraction layer between the kernel and the underlying
security mechanisms implemented by the various LSMs.  Some operations
will always fall outside the scope of individual LSMs due to their
nature, but as a general rule we try to ensure that LSM hooks are
useful across multiple LSMs.

> Following the fwmark example, if there was some programmable in-kernel
> function to convert the cmd into a SELinux label would we be able to
> enable SELinux following the SECMARK design?

As Casey already mentioned, any sort of classifier would need to be
able to support multiple LSMs.  The only example that comes to mind at
the moment is the NetLabel mechanism which translates between
on-the-wire CIPSO and CALIPSO labels and multiple LSMs (Smack and
SELinux currently).

> Would there be an objection if that in-kernel function was using a
> system-wide eBPF uploaded with some fwctl uAPI?

We'd obviously need to see patches, but there is precedent in
separating labeling from enforcement.  We've discussed SecMark and
NetLabel in the networking space, but technically, the VFS/filesystem
xattr implementations could also be considered as a labeling mechanism
outside of the LSM.

> Finally, would there be an objection to enabling the same function in
> eBPF by feeding it the entire command and have it classify and make a
> security decision in a single eBPF program?

Keeping in mind that from an LSM perspective we need to support
multiple implementations, both in terms of language mechanics (eBPF,
Rust, C) and security philosophies (Smack, SELinux, AppArmor, etc.),
so it would be very unlikely that we would want a specific shortcut or
mechanism that would only work for one language or philosophy.

> Is there some other way to enable eBPF?

If one develops a workable LSM hook then I see no reason why one
couldn't write a BPF LSM to use that hook; that's what we do today.

> I see eBPF doesn't interwork with SECMARK today so there isn't a ready example?

I'm not aware of anyone ever doing to work to try/enable secmark with
BPF, but I see no reason why someone couldn't work on that.  Just make
sure to take into account Casey's comments about multiple LSM support;
any new LSM interfaces will need to support multiple simultaneous LSMs
(the original secmark work predated that).

However, it seems like direct reuse of secmark isn't what is needed,
or even wanted, you were just using that as an example of separating
labeling from enforcement, yes?

-- 
paul-moore.com

^ permalink raw reply

* Re: [GIT PULL] Landlock update for v7.1-rc1
From: pr-tracker-bot @ 2026-04-13 22:47 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Linus Torvalds, Mickaël Salaün, Georgia Garcia,
	Günther Noack, Günther Noack, Jann Horn, Justin Suess,
	Paul Moore, Sebastian Andrzej Siewior, linux-kernel,
	linux-security-module
In-Reply-To: <20260409173124.2478023-1-mic@digikod.net>

The pull request you sent on Thu,  9 Apr 2026 19:31:24 +0200:

> https://git.kernel.org/pub/scm/linux/kernel/git/mic/linux.git tags/landlock-7.1-rc1

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/b8f82cb0d84d00c04cdbdce42f67df71b8507e8b

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

^ permalink raw reply

* Re: [GIT PULL] lsm/lsm-pr-20260410
From: pr-tracker-bot @ 2026-04-13 22:47 UTC (permalink / raw)
  To: Paul Moore; +Cc: Linus Torvalds, linux-security-module, linux-kernel
In-Reply-To: <9a2a59bd8d9548fb5dab128f4859fa3d@paul-moore.com>

The pull request you sent on Fri, 10 Apr 2026 19:26:05 -0400:

> https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git tags/lsm-pr-20260410

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/3ba310f2a3ca70f0497aab5c2e8aa85a12e19406

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox