* Re: [PATCH v5 10/11] lsm: consolidate all of the LSM framework initcalls
From: Lorenzo Stoakes @ 2026-01-29 17:09 UTC (permalink / raw)
To: Vlastimil Babka
Cc: Paul Moore, linux-security-module, linux-integrity, selinux,
john.johansen, zohar, roberto.sassu, wufan, mic, gnoack, kees,
mortonm, casey, penguin-kernel, nicolas.bouchinet, xiujianfeng,
linux-mm, David Hildenbrand, Liam R. Howlett, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, linux-kernel
In-Reply-To: <74286aca-a565-489f-ad2c-886c650ea2bc@suse.cz>
On Thu, Jan 29, 2026 at 06:02:00PM +0100, Vlastimil Babka wrote:
> Agreed, the mmap_min_addr should stay visible and applied unconditionally.
> AFAICS the only relation to SECURITY/LSM is whether CONFIG_LSM_MMAP_MIN_ADDR
> is used as an additional lower limit to both CONFIG_DEFAULT_MMAP_MIN_ADDR
> and the sysctl-written value?
Thanks, yeah we should probably actually move the non-LSM-relevant stuff
out to mm to be honest.
But that's future work, for an -rc8 hotfix we need to make the init of this
particular module not dependent on normal LSM initialisation, as horrid as
that is...
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v5 10/11] lsm: consolidate all of the LSM framework initcalls
From: Vlastimil Babka @ 2026-01-29 17:02 UTC (permalink / raw)
To: Lorenzo Stoakes, Paul Moore
Cc: linux-security-module, linux-integrity, selinux, john.johansen,
zohar, roberto.sassu, wufan, mic, gnoack, kees, mortonm, casey,
penguin-kernel, nicolas.bouchinet, xiujianfeng, linux-mm,
David Hildenbrand, Liam R. Howlett, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, linux-kernel
In-Reply-To: <14638978-b133-457a-ae9c-31ba54e3964c@lucifer.local>
On 1/29/26 17:48, Lorenzo Stoakes wrote:
> On Thu, Jan 29, 2026 at 04:31:16PM +0000, Lorenzo Stoakes wrote:
>
> Sorry to clarify here I meant to say - if I set CONFIG_SECURITY but _not_
> CONFIG_SECURITY_SELINUX the tunable does in fact still appear (and afaict
> still work...)
>
> So LSM_MMAP_MIN_ADDR is really weird to require SECURITY_SELINUX, perhaps a
> historic artifact where we wanted a different default or something like
> this?
>
> I know that we use that in preference to CONFIG_DEFAULT_MMAP_MIN_ADDR if
> specified.
>
> The description really probably needs updating.
>
> The key config here we should be looking at is DEFAULT_MMAP_MIN_ADDR which
> emphatically does _not_ require CONFIG_SECURITY and also in its description
> explicitly mentions the tunable:
>
> This value can be changed after boot using the
> /proc/sys/vm/mmap_min_addr tunable.
>
> The mmap_min_addr global value exposed in min_addr.c is referenced in
> several places in mm and other parts of the kernel - fs/exec.c,
> fs/userlandfd.c, kernel/sys.c, mm/mmap.c, mm/vma.c.
>
> So this now silently going to zero everywhere and ignoring
> CONFIG_DEFAULT_MMAP_MIN_ADDR is surely a userspace-breaking regression and
> needs fixing in rc8?
>
> Which means that... people can now mmap() at NULL everywhere despite setting
> CONFIG_DEFAULT_MMAP_MIN_ADDR > 0? :)
>
> That seems like a _really bad idea_ (TM).
>
> So this is emphatically not a report of a trivial self test break, but
> rather of something more serious AFAICT.
>
> So yeah I think this has to be reverted/fixed.
Agreed, the mmap_min_addr should stay visible and applied unconditionally.
AFAICS the only relation to SECURITY/LSM is whether CONFIG_LSM_MMAP_MIN_ADDR
is used as an additional lower limit to both CONFIG_DEFAULT_MMAP_MIN_ADDR
and the sysctl-written value?
^ permalink raw reply
* Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure
From: Mickaël Salaün @ 2026-01-29 16:54 UTC (permalink / raw)
To: Günther Noack
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Randy Dunlap
In-Reply-To: <20260129.691d9b85a887@gnoack.org>
On Thu, Jan 29, 2026 at 08:56:37AM +0100, Günther Noack wrote:
> On Wed, Jan 28, 2026 at 10:34:02PM +0100, Mickaël Salaün wrote:
> > On Sun, Jan 25, 2026 at 08:58:53PM +0100, Günther Noack wrote:
> > > Tradeoffs:
> > >
> > > This change improves performance, at a slight size increase to the
> > > layer masks data structure.
> > >
> > > At the moment, for the filesystem access rights, the data structure
> > > has the same size as before, but once we introduce the 17th filesystem
> > > access right, it will double in size (from 32 to 64 bytes), as
> >
> > ...for all access rights (e.g. even if there is no new network one)
>
> Added.
>
> > > --- a/security/landlock/audit.c
> > > +++ b/security/landlock/audit.c
> > > @@ -180,38 +180,21 @@ static void test_get_hierarchy(struct kunit *const test)
> > >
> > > #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> > >
> > > +/* get_denied_layer - get the youngest layer that denied the access_request */
> >
> > /* Get the youngest layer that denied the access_request. */
>
> OK, done. I also changed to non-docstring style for the
> access_mask_subset() helper.
>
> >
> > > static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> > > access_mask_t *const access_request,
> > > - const layer_mask_t (*const layer_masks)[],
> > > - const size_t layer_masks_size)
> > > + const struct layer_access_masks *masks)
> > > {
> > > - const unsigned long access_req = *access_request;
> > > - unsigned long access_bit;
> > > - access_mask_t missing = 0;
> > > - long youngest_layer = -1;
> > > -
> > > - for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> > > - const layer_mask_t mask = (*layer_masks)[access_bit];
> > > - long layer;
> > > -
> > > - if (!mask)
> > > - continue;
> > > -
> > > - /* __fls(1) == 0 */
> > > - layer = __fls(mask);
> > > - if (layer > youngest_layer) {
> > > - youngest_layer = layer;
> > > - missing = BIT(access_bit);
> > > - } else if (layer == youngest_layer) {
> > > - missing |= BIT(access_bit);
> > > + for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
> >
> > size_t i
>
> This is one of the two places where this didn't work.
>
> The loop goes from top to bottom here, and the "i >= 0" check would
> always be true for a size_t.
>
> If there is a more idiomatic way to write that loop, I can switch to
> it, but would otherwise lean towards keeping it as it is?
Indeed. We can use ssize_t as in get_hierarchy().
>
>
> > > + if (masks->access[i] & *access_request) {
> > > + *access_request &= masks->access[i];
> > > + return i;
> > > }
> > > }
> > >
> > > - for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
> > > - const layer_mask_t mask = (*layer_masks)[access_bit];
> > > + for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
> >
> > size_t i
>
> Ditto, the loop goes from top to bottom here.
>
>
> > > + const access_mask_t denied = masks->access[i] & optional_access;
> > > + const unsigned long newly_denied = denied & ~all_denied;
> > >
>
>
> > > -static bool
> > > -scope_to_request(const access_mask_t access_request,
> > > - layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> > > +static bool scope_to_request(const access_mask_t access_request,
> > > + struct layer_access_masks *masks)
> > > {
> > > - const unsigned long access_req = access_request;
> > > - unsigned long access_bit;
> > > + bool saw_unfulfilled_access = false;
> > >
> > > - if (WARN_ON_ONCE(!layer_masks))
> > > + if (WARN_ON_ONCE(!masks))
> > > return true;
> > >
> > > - for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
> > > - (*layer_masks)[access_bit] = 0;
> > > -
> > > - return is_layer_masks_allowed(layer_masks);
> > > + for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
> > > + masks->access[i] &= access_request;
> > > + if (masks->access[i])
> >
> > {
> >
> > > + saw_unfulfilled_access = true;
> >
> > break;
> > }
>
> Two lines above, this loop mutates masks->access[...]:
>
> masks->access[i] &= access_request
>
> If we break the loop early, we would not actually scope it down to the
> request entirely? Is this safe?
You're right, don't add this break. BTW, would a test catch it?
>
> > > + }
> > > + return !saw_unfulfilled_access;
> > > }
>
> –Günther
>
^ permalink raw reply
* Re: [PATCH v5 10/11] lsm: consolidate all of the LSM framework initcalls
From: Lorenzo Stoakes @ 2026-01-29 16:48 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module, linux-integrity, selinux, john.johansen,
zohar, roberto.sassu, wufan, mic, gnoack, kees, mortonm, casey,
penguin-kernel, nicolas.bouchinet, xiujianfeng, linux-mm,
David Hildenbrand, Vlastimil Babka, Liam R. Howlett,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, linux-kernel
In-Reply-To: <0146e385-935b-4f66-9e6d-51bb47ae4bdc@lucifer.local>
On Thu, Jan 29, 2026 at 04:31:16PM +0000, Lorenzo Stoakes wrote:
> +cc linux-mm, maintainers/reviewers of mm/Kconfig
>
> On Fri, Oct 17, 2025 at 04:48:24PM -0400, Paul Moore wrote:
> > The LSM framework itself registers a small number of initcalls, this
> > patch converts these initcalls into the new initcall mechanism.
> >
> > Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> > Reviewed-by: John Johansen <john.johhansen@canonical.com>
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
>
> Hi,
>
> This commit message doesn't mention at all that you've removed
> /proc/sys/vm/mmap_min_addr altogether if CONFIG_SECURITY is not set.
>
> Did you intend this change? If you did you should probably mention that
> you're doing this :)
>
> I mean it's a bit late now as this is upstream (but not _too_ late as we
> have rc8 ;), but this has broken something for me locally (mremap mm
> selftest) and I bisected to this commit.
>
> Note that CONFIG_SECURITY states:
>
> This allows you to choose different security modules to be
> configured into your kernel.
>
> If this option is not selected, the default Linux security
> model will be used.
>
> So is the 'default' Linux security model not to provide this tunable at
> all?
>
> Though I see LSM_MMAP_MIN_ADDR depends on SECURITY && SECURITY_SELINUX, the
> Makefile in security/ has:
>
> obj-$(CONFIG_MMU) += min_addr.o
>
> Which suggests that min_addr depends on MMU only, and not on
> LSM_MMAP_MIN_ADDR at all...
>
> And I don't have CONFIG_SECURITY_SELINUX set yet have
> /proc/sys/vm/mmap_min_addr?
Sorry to clarify here I meant to say - if I set CONFIG_SECURITY but _not_
CONFIG_SECURITY_SELINUX the tunable does in fact still appear (and afaict
still work...)
So LSM_MMAP_MIN_ADDR is really weird to require SECURITY_SELINUX, perhaps a
historic artifact where we wanted a different default or something like
this?
I know that we use that in preference to CONFIG_DEFAULT_MMAP_MIN_ADDR if
specified.
The description really probably needs updating.
The key config here we should be looking at is DEFAULT_MMAP_MIN_ADDR which
emphatically does _not_ require CONFIG_SECURITY and also in its description
explicitly mentions the tunable:
This value can be changed after boot using the
/proc/sys/vm/mmap_min_addr tunable.
The mmap_min_addr global value exposed in min_addr.c is referenced in
several places in mm and other parts of the kernel - fs/exec.c,
fs/userlandfd.c, kernel/sys.c, mm/mmap.c, mm/vma.c.
So this now silently going to zero everywhere and ignoring
CONFIG_DEFAULT_MMAP_MIN_ADDR is surely a userspace-breaking regression and
needs fixing in rc8?
Which means that... people can now mmap() at NULL everywhere despite setting
CONFIG_DEFAULT_MMAP_MIN_ADDR > 0? :)
That seems like a _really bad idea_ (TM).
So this is emphatically not a report of a trivial self test break, but
rather of something more serious AFAICT.
So yeah I think this has to be reverted/fixed.
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH v5 10/11] lsm: consolidate all of the LSM framework initcalls
From: Lorenzo Stoakes @ 2026-01-29 16:31 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module, linux-integrity, selinux, john.johansen,
zohar, roberto.sassu, wufan, mic, gnoack, kees, mortonm, casey,
penguin-kernel, nicolas.bouchinet, xiujianfeng, linux-mm,
David Hildenbrand, Vlastimil Babka, Liam R. Howlett,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, linux-kernel
In-Reply-To: <20251017204815.505363-21-paul@paul-moore.com>
+cc linux-mm, maintainers/reviewers of mm/Kconfig
On Fri, Oct 17, 2025 at 04:48:24PM -0400, Paul Moore wrote:
> The LSM framework itself registers a small number of initcalls, this
> patch converts these initcalls into the new initcall mechanism.
>
> Reviewed-by: Casey Schaufler <casey@schaufler-ca.com>
> Reviewed-by: John Johansen <john.johhansen@canonical.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
Hi,
This commit message doesn't mention at all that you've removed
/proc/sys/vm/mmap_min_addr altogether if CONFIG_SECURITY is not set.
Did you intend this change? If you did you should probably mention that
you're doing this :)
I mean it's a bit late now as this is upstream (but not _too_ late as we
have rc8 ;), but this has broken something for me locally (mremap mm
selftest) and I bisected to this commit.
Note that CONFIG_SECURITY states:
This allows you to choose different security modules to be
configured into your kernel.
If this option is not selected, the default Linux security
model will be used.
So is the 'default' Linux security model not to provide this tunable at
all?
Though I see LSM_MMAP_MIN_ADDR depends on SECURITY && SECURITY_SELINUX, the
Makefile in security/ has:
obj-$(CONFIG_MMU) += min_addr.o
Which suggests that min_addr depends on MMU only, and not on
LSM_MMAP_MIN_ADDR at all...
And I don't have CONFIG_SECURITY_SELINUX set yet have
/proc/sys/vm/mmap_min_addr?
So yeah, this is all very very confusing.
So I think maybe we need a revert/hotfix here if this was unintended?
I think we might be breaking userspace here... For one the mremap mm
selftest breaks immediately :)
Note that prior to this change the default of 64k seems to be set which
seems to contradict the docs in Documentation/admin-guide/sysctl/vm.rst:
By default this value is set to 0 and no protections will be enforced by
the security module. Setting this value to something like 64k will allow
the vast majority of applications to work correctly and provide defense in
depth against future potential kernel bugs.
Also to add to the fun, we have CONFIG_DEFAULT_MMAP_MIN_ADDR as defined in
mm/Kconfig:
config DEFAULT_MMAP_MIN_ADDR
int "Low address space to protect from user allocation"
depends on MMU
default 4096
Which is _only_ referenced in security/min_addr.c which of course, is now
not being used at all.
So we have a config option that people _think_ they are setting to
something to enforce a minimum address but are in fact not if
!CONFIG_SECURITY?
Thanks, Lorenzo
> ---
> security/inode.c | 3 +--
> security/lsm.h | 20 ++++++++++++++++++++
> security/lsm_init.c | 14 ++++++++++++--
> security/min_addr.c | 5 +++--
> 4 files changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/security/inode.c b/security/inode.c
> index 6620c3e42af2..ab8d6a2acadb 100644
> --- a/security/inode.c
> +++ b/security/inode.c
> @@ -368,7 +368,7 @@ static const struct file_operations lsm_ops = {
> };
> #endif
>
> -static int __init securityfs_init(void)
> +int __init securityfs_init(void)
> {
> int retval;
>
> @@ -387,4 +387,3 @@ static int __init securityfs_init(void)
> #endif
> return 0;
> }
> -core_initcall(securityfs_init);
> diff --git a/security/lsm.h b/security/lsm.h
> index 8dc267977ae0..81aadbc61685 100644
> --- a/security/lsm.h
> +++ b/security/lsm.h
> @@ -35,4 +35,24 @@ extern struct kmem_cache *lsm_inode_cache;
> int lsm_cred_alloc(struct cred *cred, gfp_t gfp);
> int lsm_task_alloc(struct task_struct *task);
>
> +/* LSM framework initializers */
> +
> +#ifdef CONFIG_MMU
> +int min_addr_init(void);
> +#else
> +static inline int min_addr_init(void)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_MMU */
> +
> +#ifdef CONFIG_SECURITYFS
> +int securityfs_init(void);
> +#else
> +static inline int securityfs_init(void)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_SECURITYFS */
> +
> #endif /* _LSM_H_ */
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index aacdac406ba5..0f668bca98f9 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -488,7 +488,12 @@ int __init security_init(void)
> */
> static int __init security_initcall_pure(void)
> {
> - return lsm_initcall(pure);
> + int rc_adr, rc_lsm;
> +
> + rc_adr = min_addr_init();
> + rc_lsm = lsm_initcall(pure);
> +
> + return (rc_adr ? rc_adr : rc_lsm);
> }
> pure_initcall(security_initcall_pure);
>
> @@ -506,7 +511,12 @@ early_initcall(security_initcall_early);
> */
> static int __init security_initcall_core(void)
> {
> - return lsm_initcall(core);
> + int rc_sfs, rc_lsm;
> +
> + rc_sfs = securityfs_init();
> + rc_lsm = lsm_initcall(core);
> +
> + return (rc_sfs ? rc_sfs : rc_lsm);
> }
> core_initcall(security_initcall_core);
>
> diff --git a/security/min_addr.c b/security/min_addr.c
> index c55bb84b8632..0fde5ec9abc8 100644
> --- a/security/min_addr.c
> +++ b/security/min_addr.c
> @@ -5,6 +5,8 @@
> #include <linux/sysctl.h>
> #include <linux/minmax.h>
>
> +#include "lsm.h"
> +
> /* amount of vm to protect from userspace access by both DAC and the LSM*/
> unsigned long mmap_min_addr;
> /* amount of vm to protect from userspace using CAP_SYS_RAWIO (DAC) */
> @@ -52,11 +54,10 @@ static const struct ctl_table min_addr_sysctl_table[] = {
> },
> };
>
> -static int __init init_mmap_min_addr(void)
> +int __init min_addr_init(void)
> {
> register_sysctl_init("vm", min_addr_sysctl_table);
> update_mmap_min_addr();
>
> return 0;
> }
> -pure_initcall(init_mmap_min_addr);
> --
> 2.51.1.dirty
>
>
^ permalink raw reply
* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Roberto Sassu @ 2026-01-29 16:18 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-integrity
Cc: Eric Biggers, James Bottomley, Mimi Zohar, David Howells,
Paul Moore, James Morris, Serge E. Hallyn, open list:KEYS-TRUSTED,
open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <20260125192526.782202-2-jarkko@kernel.org>
On Sun, 2026-01-25 at 21:25 +0200, Jarkko Sakkinen wrote:
> 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> use should be pooled rather than directly used. This both reduces
> latency and improves its predictability.
>
> 2. Linux is better off overall if every subsystem uses the same source for
> generating the random numbers required.
>
> Thus, unset '.get_random', which causes fallback to kernel_get_random().
>
> One might argue that TPM RNG should be used for the generated trusted keys,
> so that they have matching entropy with the TPM internally generated
> objects.
>
> This argument does have some weight into it but as far cryptography goes,
> FIPS certification sets the exact bar, not which exact FIPS certified RNG
> will be used. Thus, the rational choice is obviously to pick the lowest
> latency path, which is kernel RNG.
>
> Finally, there is an actual defence in depth benefit when using kernel RNG
> as it helps to mitigate TPM firmware bugs concerning RNG implementation,
> given the obfuscation by the other entropy sources.
>
> Reviewed-by: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v7:
> - A new patch. Simplifies follow up patches.
> ---
> security/keys/trusted-keys/trusted_tpm1.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> index 636acb66a4f6..7ce7e31bcdfb 100644
> --- a/security/keys/trusted-keys/trusted_tpm1.c
> +++ b/security/keys/trusted-keys/trusted_tpm1.c
> @@ -6,6 +6,16 @@
> * See Documentation/security/keys/trusted-encrypted.rst
> */
>
> +/**
> + * DOC: Random Number Generation
> + *
> + * tpm_get_random() was previously used here as the RNG in order to have equal
> + * entropy with the objects fully inside the TPM. However, as far as goes,
> + * kernel RNG is equally fine, as long as long as it is FIPS certified. Also,
> + * using kernel RNG has the benefit of mitigating bugs in the TPM firmware
> + * associated with the RNG.
> + */
If we switch to the kernel RNG that is better, and the TPM one is
flawed, I guess we are going to have big problems anyway, since the TPM
random number generator is used by the TPM itself internally.
I think it makes sense to leave as it is.
Thanks
Roberto
> +
> #include <crypto/hash_info.h>
> #include <crypto/sha1.h>
> #include <crypto/utils.h>
> @@ -936,11 +946,6 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> return ret;
> }
>
> -static int trusted_tpm_get_random(unsigned char *key, size_t key_len)
> -{
> - return tpm_get_random(chip, key, key_len);
> -}
> -
> static int __init init_digests(void)
> {
> int i;
> @@ -992,6 +997,5 @@ struct trusted_key_ops trusted_key_tpm_ops = {
> .init = trusted_tpm_init,
> .seal = trusted_tpm_seal,
> .unseal = trusted_tpm_unseal,
> - .get_random = trusted_tpm_get_random,
> .exit = trusted_tpm_exit,
> };
^ permalink raw reply
* Re: [PATCH v4 13/17] module: Report signature type to users
From: Petr Pavlu @ 2026-01-29 14:44 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Sami Tolvanen,
Daniel Gomez, Paul Moore, James Morris, Serge E. Hallyn,
Jonathan Corbet, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Naveen N Rao, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Nicolas Schier, Daniel Gomez,
Aaron Tomlin, Christophe Leroy (CS GROUP), Nicolas Schier,
Nicolas Bouchinet, Xiu Jianfeng, Fabian Grünbichler,
Arnout Engelen, Mattia Rizzolo, kpcyrd, Christian Heusel,
Câju Mihai-Drosi, Sebastian Andrzej Siewior, linux-kbuild,
linux-kernel, linux-arch, linux-modules, linux-security-module,
linux-doc, linuxppc-dev, linux-integrity
In-Reply-To: <20260113-module-hashes-v4-13-0b932db9b56b@weissschuh.net>
On 1/13/26 1:28 PM, Thomas Weißschuh wrote:
> The upcoming CONFIG_MODULE_HASHES will introduce a signature type.
> This needs to be handled by callers differently than PKCS7 signatures.
>
> Report the signature type to the caller and let them verify it.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> [...]
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index d65bc300a78c..2a28a0ece809 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3348,19 +3348,24 @@ static int module_integrity_check(struct load_info *info, int flags)
> {
> bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
> MODULE_INIT_IGNORE_VERMAGIC);
> + enum pkey_id_type sig_type;
> size_t sig_len;
> const u8 *sig;
> int err = 0;
>
> if (IS_ENABLED(CONFIG_MODULE_SIG_POLICY)) {
> err = mod_split_sig(info->hdr, &info->len, mangled_module,
> - &sig_len, &sig, "module");
> + &sig_type, &sig_len, &sig, "module");
> if (err)
> return err;
> }
>
> - if (IS_ENABLED(CONFIG_MODULE_SIG))
> + if (IS_ENABLED(CONFIG_MODULE_SIG) && sig_type == PKEY_ID_PKCS7) {
> err = module_sig_check(info, sig, sig_len);
> + } else {
> + pr_err("module: not signed with expected PKCS#7 message\n");
> + err = -ENOPKG;
> + }
The new else branch means that if the user chooses not to configure any
module integrity policy, they will no longer be able to load any
modules. I think this entire if-else part should be moved under the
IS_ENABLED(CONFIG_MODULE_SIG_POLICY) block above, as I'm mentioning on
patch #12.
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v4 12/17] module: Move signature splitting up
From: Petr Pavlu @ 2026-01-29 14:41 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Sami Tolvanen,
Daniel Gomez, Paul Moore, James Morris, Serge E. Hallyn,
Jonathan Corbet, Madhavan Srinivasan, Michael Ellerman,
Nicholas Piggin, Naveen N Rao, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Nicolas Schier, Daniel Gomez,
Aaron Tomlin, Christophe Leroy (CS GROUP), Nicolas Schier,
Nicolas Bouchinet, Xiu Jianfeng, Fabian Grünbichler,
Arnout Engelen, Mattia Rizzolo, kpcyrd, Christian Heusel,
Câju Mihai-Drosi, Sebastian Andrzej Siewior, linux-kbuild,
linux-kernel, linux-arch, linux-modules, linux-security-module,
linux-doc, linuxppc-dev, linux-integrity
In-Reply-To: <20260113-module-hashes-v4-12-0b932db9b56b@weissschuh.net>
On 1/13/26 1:28 PM, Thomas Weißschuh wrote:
> The signature splitting will also be used by CONFIG_MODULE_HASHES.
>
> Move it up the callchain, so the result can be reused.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> [...]
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index c09b25c0166a..d65bc300a78c 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3346,10 +3346,21 @@ static int early_mod_check(struct load_info *info, int flags)
>
> static int module_integrity_check(struct load_info *info, int flags)
> {
> + bool mangled_module = flags & (MODULE_INIT_IGNORE_MODVERSIONS |
> + MODULE_INIT_IGNORE_VERMAGIC);
> + size_t sig_len;
> + const u8 *sig;
> int err = 0;
>
> + if (IS_ENABLED(CONFIG_MODULE_SIG_POLICY)) {
> + err = mod_split_sig(info->hdr, &info->len, mangled_module,
> + &sig_len, &sig, "module");
> + if (err)
> + return err;
> + }
> +
> if (IS_ENABLED(CONFIG_MODULE_SIG))
> - err = module_sig_check(info, flags);
> + err = module_sig_check(info, sig, sig_len);
>
> if (err)
> return err;
I suggest moving the IS_ENABLED(CONFIG_MODULE_SIG) block under the
new IS_ENABLED(CONFIG_MODULE_SIG_POLICY) section. I realize that
CONFIG_MODULE_SIG implies CONFIG_MODULE_SIG_POLICY, but I believe this
change makes it more apparent that this it the case. Otherwise, one
might for example wonder if sig_len in the module_sig_check() call can
be undefined.
if (IS_ENABLED(CONFIG_MODULE_SIG_POLICY)) {
err = mod_split_sig(info->hdr, &info->len, mangled_module,
&sig_len, &sig, "module");
if (err)
return err;
if (IS_ENABLED(CONFIG_MODULE_SIG))
err = module_sig_check(info, sig, sig_len);
}
--
Thanks,
Petr
^ permalink raw reply
* Re: [PATCH v5 0/6] Extend "trusted" keys to support a new trust source named the PowerVM Key Wrapping Module (PKWM)
From: Nayna Jain @ 2026-01-29 10:15 UTC (permalink / raw)
To: Srish Srinivasan, linux-integrity, keyrings, linuxppc-dev
Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
zohar, rnsastry, linux-kernel, linux-security-module
In-Reply-To: <20260127145228.48320-1-ssrish@linux.ibm.com>
On 1/27/26 9:52 AM, Srish Srinivasan wrote:
> Power11 has introduced a feature called the PowerVM Key Wrapping Module
> (PKWM), where PowerVM in combination with Power LPAR Platform KeyStore
> (PLPKS) [1] supports a new feature called "Key Wrapping" [2] to protect
> user secrets by wrapping them using a hypervisor generated wrapping key.
> This wrapping key is an AES-GCM-256 symmetric key that is stored as an
> object in the PLPKS. It has policy based protections that prevents it from
> being read out or exposed to the user. This wrapping key can then be used
> by the OS to wrap or unwrap secrets via hypervisor calls.
>
> This patchset intends to add the PKWM, which is a combination of IBM
> PowerVM and PLPKS, as a new trust source for trusted keys. The wrapping key
> does not exist by default and its generation is requested by the kernel at
> the time of PKWM initialization. This key is then persisted by the PKWM and
> is used for wrapping any kernel provided key, and is never exposed to the
> user. The kernel is aware of only the label to this wrapping key.
>
> Along with the PKWM implementation, this patchset includes two preparatory
> patches: one fixing the kernel-doc inconsistencies in the PLPKS code and
> another reorganizing PLPKS config variables in the sysfs.
Tested the entire patch series. Seems to work as expected.
Tested-by: Nayna Jain <nayna@linux.ibm.com>
Thanks & Regards,
- Nayna
^ permalink raw reply
* Re: [RFC][PATCH v2] ima: Add support for staging measurements for deletion and trimming
From: Roberto Sassu @ 2026-01-29 8:20 UTC (permalink / raw)
To: steven chen, corbet, zohar, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, nramas, Roberto Sassu
In-Reply-To: <6b36d6b3-34e8-4bdc-bd68-d71ddf44eba8@linux.microsoft.com>
On 1/28/2026 10:30 PM, steven chen wrote:
> On 12/12/2025 9:19 AM, Roberto Sassu wrote:
>> From: Roberto Sassu <roberto.sassu@huawei.com>
>>
>> Introduce the ability of staging the entire (or a portion of the) IMA
>> measurement list for deletion. Staging means moving the current
>> content of
>> the measurement list to a separate location, and allowing users to
>> read and
>> delete it. This causes the measurement list to be atomically truncated
>> before new measurements can be added. Staging can be done only once at a
>> time. In the event of kexec(), staging is reverted and staged entries
>> will
>> be carried over to the new kernel.
>>
>> User space is responsible to concatenate the staged IMA measurements list
>> portions following the temporal order in which the operations were done,
>> together with the current measurement list. Then, it can send the
>> collected
>> data to the remote verifiers.
>>
>> Also introduce the ability of trimming N measurements entries from the
>> IMA
>> measurements list, provided that user space has already read them.
>> Trimming
>> combines staging and deletion in one operation.
>>
>> The benefit of these solutions is the ability to free precious kernel
>> memory, in exchange of delegating user space to reconstruct the full
>> measurement list from the chunks. No trust needs to be given to user
>> space,
>> since the integrity of the measurement list is protected by the TPM.
>>
>> By default, staging/trimming the measurements list does not alter the
>> hash
>> table. When staging/trimming are done, IMA is still able to detect
>> collisions on the staged and later deleted measurement entries, by
>> keeping
>> the entry digests (only template data are freed).
>>
>> However, since during the measurements list serialization only the SHA1
>> digest is passed, and since there are no template data to recalculate the
>> other digests from, the hash table is currently not populated with
>> digests
>> from staged/deleted entries after kexec().
>>
>> Introduce the new kernel option ima_flush_htable to decide whether or not
>> the digests of staged measurement entries are flushed from the hash
>> table.
>>
>> Then, introduce ascii_runtime_measurements_staged_<algo> and
>> binary_runtime_measurement_staged_<algo> interfaces to stage/trim/delete
>> the measurements. Use 'echo A > <IMA interface>' and
>> 'echo D > <IMA interface>' to respectively stage and delete the entire
>> measurements list. Use 'echo N > <IMA interface>', with N between 1 and
>> LONG_MAX, to stage the selected portion of the measurements list, and
>> 'echo -N > <IMA interface>' to trim N measurements entries.
>>
>> The ima_measure_users counter (protected by the ima_measure_lock
>> mutex) has
>> been introduced to protect access to the measurements list and the staged
>> part. The open method of all the measurement interfaces has been extended
>> to allow only one writer at a time or, in alternative, multiple readers.
>> The write permission is used to stage/trim/delete the measurements, the
>> read permission to read them. Write requires also the CAP_SYS_ADMIN
>> capability.
>>
>> Finally, introduce and maintain dedicate counters for the number of
>> measurement entries and binary size, for the current measurements list
>> (BINARY_SIZE), for the current measurements list plus staged entries
>> (BINARY_SIZE_STAGED) useful for kexec() segment allocation, and for the
>> entire measurement list without staging/trimming (BINARY_SIZE_FULL)
>> useful
>> for the kexec-related critical data records.
> Is the following possible race condition for staged list:
>
> Agent A: create staged list Staged list A1
> new measurement added Measurement list M1
> Two lists in kernel: A1 and M1
>
> Agent B: read staged list (A1) to do verification
> new measurement added Measurement list M2
> Two lists in kernel: A1 and M2
>
> Agent A: verified and remove staged list (A1)
> new measurement added Measurement list M3
> One list in kernel: M3
>
> Agent C: create staged list Staged list C1
> new measurement added Measurement list M4
> Two lists in kernel: C1 and M4
>
> Agent B: remove staged list (?), C1 removed ---this will cause problem
> new measurement added Measurement list M5
> One list in kernel: M5
>
> Agent C: try to remove staged list(?)
If you remember the patch, we added a read-write protection to the
measurements interfaces. As long as you keep the interface open for
write no one else can make change on the staging. Sure, you can drop the
write, and reopen for read, but then you should expect someone else to
operate on the interface.
If you want to be sure no one else changes the staged measurements, just
keep the interface open for write, read the staged measurements and
delete them.
Roberto
> Possible solution?
> Save the total number trimmed T or tag
>
> Trim request sync this parameter to trim the staged list
>
> Regards,
>
> Steven
>
>> Note: This code derives from the Alt-IMA Huawei project, and is being
>> released under the dual license model (GPL-2.0 OR MIT).
>>
>> Link: https://github.com/linux-integrity/linux/issues/1
>> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
>> ---
>> .../admin-guide/kernel-parameters.txt | 4 +
>> security/integrity/ima/ima.h | 18 +-
>> security/integrity/ima/ima_fs.c | 240 +++++++++++++++++-
>> security/integrity/ima/ima_kexec.c | 42 ++-
>> security/integrity/ima/ima_queue.c | 169 +++++++++++-
>> 5 files changed, 439 insertions(+), 34 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/
>> Documentation/admin-guide/kernel-parameters.txt
>> index 6c42061ca20e..e5f1e11bd0a2 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -2215,6 +2215,10 @@
>> Use the canonical format for the binary runtime
>> measurements, instead of host native format.
>> + ima_flush_htable [IMA]
>> + Flush the IMA hash table when staging for deletion or
>> + trimming measurement entries.
>> +
>> ima_hash= [IMA]
>> Format: { md5 | sha1 | rmd160 | sha256 | sha384
>> | sha512 | ... }
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index e3d71d8d56e3..8a6be4284210 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -28,6 +28,15 @@ enum ima_show_type { IMA_SHOW_BINARY,
>> IMA_SHOW_BINARY_NO_FIELD_LEN,
>> IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
>> enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>> +/*
>> + * BINARY_SIZE: size of the current measurements list
>> + * BINARY_SIZE_STAGED: size of current measurements list + staged
>> entries
>> + * BINARY_SIZE_FULL: size of measurements list since IMA initialization
>> + */
>> +enum binary_size_types {
>> + BINARY_SIZE, BINARY_SIZE_STAGED, BINARY_SIZE_FULL, BINARY__LAST
>> +};
>> +
>> /* digest size for IMA, fits SHA1 or MD5 */
>> #define IMA_DIGEST_SIZE SHA1_DIGEST_SIZE
>> #define IMA_EVENT_NAME_LEN_MAX 255
>> @@ -117,6 +126,8 @@ struct ima_queue_entry {
>> struct ima_template_entry *entry;
>> };
>> extern struct list_head ima_measurements; /* list of all
>> measurements */
>> +extern struct list_head ima_measurements_staged; /* list of staged
>> meas. */
>> +extern bool ima_measurements_staged_exist; /* If there are staged
>> meas. */
>> /* Some details preceding the binary serialized measurement list */
>> struct ima_kexec_hdr {
>> @@ -281,10 +292,12 @@ struct ima_template_desc
>> *ima_template_desc_current(void);
>> struct ima_template_desc *ima_template_desc_buf(void);
>> struct ima_template_desc *lookup_template_desc(const char *name);
>> bool ima_template_has_modsig(const struct ima_template_desc
>> *ima_template);
>> +int ima_queue_stage_trim(unsigned long req_value, bool trim);
>> +int ima_queue_delete_staged_trimmed(bool staged_moved);
>> int ima_restore_measurement_entry(struct ima_template_entry *entry);
>> int ima_restore_measurement_list(loff_t bufsize, void *buf);
>> int ima_measurements_show(struct seq_file *m, void *v);
>> -unsigned long ima_get_binary_runtime_size(void);
>> +unsigned long ima_get_binary_runtime_size(enum binary_size_types type);
>> int ima_init_template(void);
>> void ima_init_template_list(void);
>> int __init ima_init_digests(void);
>> @@ -298,11 +311,12 @@ int ima_lsm_policy_change(struct notifier_block
>> *nb, unsigned long event,
>> extern spinlock_t ima_queue_lock;
>> struct ima_h_table {
>> - atomic_long_t len; /* number of stored measurements in the
>> list */
>> + atomic_long_t len[BINARY__LAST]; /* num of stored meas. in the
>> list */
>> atomic_long_t violations;
>> struct hlist_head queue[IMA_MEASURE_HTABLE_SIZE];
>> };
>> extern struct ima_h_table ima_htable;
>> +extern struct mutex ima_extend_list_mutex;
>> static inline unsigned int ima_hash_key(u8 *digest)
>> {
>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/
>> ima_fs.c
>> index 87045b09f120..a96f7c36b34a 100644
>> --- a/security/integrity/ima/ima_fs.c
>> +++ b/security/integrity/ima/ima_fs.c
>> @@ -24,7 +24,18 @@
>> #include "ima.h"
>> +/*
>> + * Requests:
>> + * 'A\n': stage the entire measurements list
>> + * '[1, LONG_MAX]\n' stage N measurements entries
>> + * '-[1, LONG_MAX]\n' trim N measurements entries
>> + * 'D\n': delete staged measurements
>> + */
>> +#define STAGED_REQ_LENGTH 21
>> +
>> static DEFINE_MUTEX(ima_write_mutex);
>> +static DEFINE_MUTEX(ima_measure_lock);
>> +static long ima_measure_users;
>> bool ima_canonical_fmt;
>> static int __init default_canonical_fmt_setup(char *str)
>> @@ -64,7 +75,8 @@ static ssize_t ima_show_measurements_count(struct
>> file *filp,
>> char __user *buf,
>> size_t count, loff_t *ppos)
>> {
>> - return ima_show_htable_value(buf, count, ppos, &ima_htable.len);
>> + return ima_show_htable_value(buf, count, ppos,
>> + &ima_htable.len[BINARY_SIZE]);
>> }
>> @@ -74,14 +86,15 @@ static const struct file_operations
>> ima_measurements_count_ops = {
>> };
>> /* returns pointer to hlist_node */
>> -static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
>> +static void *_ima_measurements_start(struct seq_file *m, loff_t *pos,
>> + struct list_head *head)
>> {
>> loff_t l = *pos;
>> struct ima_queue_entry *qe;
>> /* we need a lock since pos could point beyond last element */
>> rcu_read_lock();
>> - list_for_each_entry_rcu(qe, &ima_measurements, later) {
>> + list_for_each_entry_rcu(qe, head, later) {
>> if (!l--) {
>> rcu_read_unlock();
>> return qe;
>> @@ -91,7 +104,18 @@ static void *ima_measurements_start(struct
>> seq_file *m, loff_t *pos)
>> return NULL;
>> }
>> -static void *ima_measurements_next(struct seq_file *m, void *v,
>> loff_t *pos)
>> +static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
>> +{
>> + return _ima_measurements_start(m, pos, &ima_measurements);
>> +}
>> +
>> +static void *ima_measurements_staged_start(struct seq_file *m, loff_t
>> *pos)
>> +{
>> + return _ima_measurements_start(m, pos, &ima_measurements_staged);
>> +}
>> +
>> +static void *_ima_measurements_next(struct seq_file *m, void *v,
>> loff_t *pos,
>> + struct list_head *head)
>> {
>> struct ima_queue_entry *qe = v;
>> @@ -103,7 +127,18 @@ static void *ima_measurements_next(struct
>> seq_file *m, void *v, loff_t *pos)
>> rcu_read_unlock();
>> (*pos)++;
>> - return (&qe->later == &ima_measurements) ? NULL : qe;
>> + return (&qe->later == head) ? NULL : qe;
>> +}
>> +
>> +static void *ima_measurements_next(struct seq_file *m, void *v,
>> loff_t *pos)
>> +{
>> + return _ima_measurements_next(m, v, pos, &ima_measurements);
>> +}
>> +
>> +static void *ima_measurements_staged_next(struct seq_file *m, void *v,
>> + loff_t *pos)
>> +{
>> + return _ima_measurements_next(m, v, pos, &ima_measurements_staged);
>> }
>> static void ima_measurements_stop(struct seq_file *m, void *v)
>> @@ -202,16 +237,147 @@ static const struct seq_operations
>> ima_measurments_seqops = {
>> .show = ima_measurements_show
>> };
>> +static int _ima_measurements_open(struct inode *inode, struct file
>> *file,
>> + const struct seq_operations *seq_ops)
>> +{
>> + bool write = !!(file->f_mode & FMODE_WRITE);
>> + int ret;
>> +
>> + if (write && !capable(CAP_SYS_ADMIN))
>> + return -EPERM;
>> +
>> + mutex_lock(&ima_measure_lock);
>> + if ((write && ima_measure_users != 0) ||
>> + (!write && ima_measure_users < 0)) {
>> + mutex_unlock(&ima_measure_lock);
>> + return -EBUSY;
>> + }
>> +
>> + ret = seq_open(file, seq_ops);
>> + if (ret < 0) {
>> + mutex_unlock(&ima_measure_lock);
>> + return ret;
>> + }
>> +
>> + if (write)
>> + ima_measure_users--;
>> + else
>> + ima_measure_users++;
>> +
>> + mutex_unlock(&ima_measure_lock);
>> + return ret;
>> +}
>> +
>> static int ima_measurements_open(struct inode *inode, struct file
>> *file)
>> {
>> - return seq_open(file, &ima_measurments_seqops);
>> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
>> +}
>> +
>> +static int ima_measurements_release(struct inode *inode, struct file
>> *file)
>> +{
>> + bool write = !!(file->f_mode & FMODE_WRITE);
>> + int ret;
>> +
>> + mutex_lock(&ima_measure_lock);
>> + ret = seq_release(inode, file);
>> + if (!ret) {
>> + if (write)
>> + ima_measure_users++;
>> + else
>> + ima_measure_users--;
>> + }
>> +
>> + mutex_unlock(&ima_measure_lock);
>> + return ret;
>> }
>> static const struct file_operations ima_measurements_ops = {
>> .open = ima_measurements_open,
>> .read = seq_read,
>> .llseek = seq_lseek,
>> - .release = seq_release,
>> + .release = ima_measurements_release,
>> +};
>> +
>> +static const struct seq_operations ima_measurments_staged_seqops = {
>> + .start = ima_measurements_staged_start,
>> + .next = ima_measurements_staged_next,
>> + .stop = ima_measurements_stop,
>> + .show = ima_measurements_show
>> +};
>> +
>> +static int ima_measurements_staged_open(struct inode *inode, struct
>> file *file)
>> +{
>> + return _ima_measurements_open(inode, file,
>> + &ima_measurments_staged_seqops);
>> +}
>> +
>> +static ssize_t ima_measurements_staged_read(struct file *file, char
>> __user *buf,
>> + size_t size, loff_t *ppos)
>> +{
>> + if (!ima_measurements_staged_exist)
>> + return -ENOENT;
>> +
>> + return seq_read(file, buf, size, ppos);
>> +}
>> +
>> +static ssize_t ima_measurements_staged_write(struct file *file,
>> + const char __user *buf,
>> + size_t datalen, loff_t *ppos)
>> +{
>> + char req[STAGED_REQ_LENGTH], *req_ptr = req;
>> + unsigned long req_value;
>> + bool trim = false;
>> + int ret;
>> +
>> + if (*ppos > 0 || datalen < 2 || datalen > STAGED_REQ_LENGTH)
>> + return -EINVAL;
>> +
>> + if (copy_from_user(req, buf, datalen) != 0)
>> + return -EFAULT;
>> +
>> + if (req[datalen - 1] != '\n')
>> + return -EINVAL;
>> +
>> + req[datalen - 1] = '\0';
>> + req_ptr = req;
>> +
>> + switch (req[0]) {
>> + case 'A':
>> + if (datalen != 2 || req[1] != '\0')
>> + return -EINVAL;
>> +
>> + ret = ima_queue_stage_trim(LONG_MAX, false);
>> + break;
>> + case 'D':
>> + if (datalen != 2 || req[1] != '\0')
>> + return -EINVAL;
>> +
>> + ret = ima_queue_delete_staged_trimmed(false);
>> + break;
>> + case '-':
>> + trim = true;
>> + req_ptr++;
>> + fallthrough;
>> + default:
>> + ret = kstrtoul(req_ptr, 0, &req_value);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = ima_queue_stage_trim(req_value, trim);
>> + }
>> +
>> + if (ret < 0)
>> + return ret;
>> +
>> + return datalen;
>> +}
>> +
>> +static const struct file_operations ima_measurements_staged_ops = {
>> + .open = ima_measurements_staged_open,
>> + .read = ima_measurements_staged_read,
>> + .write = ima_measurements_staged_write,
>> + .llseek = seq_lseek,
>> + .release = ima_measurements_release,
>> };
>> void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
>> @@ -279,14 +445,37 @@ static const struct seq_operations
>> ima_ascii_measurements_seqops = {
>> static int ima_ascii_measurements_open(struct inode *inode, struct
>> file *file)
>> {
>> - return seq_open(file, &ima_ascii_measurements_seqops);
>> + return _ima_measurements_open(inode, file,
>> + &ima_ascii_measurements_seqops);
>> }
>> static const struct file_operations ima_ascii_measurements_ops = {
>> .open = ima_ascii_measurements_open,
>> .read = seq_read,
>> .llseek = seq_lseek,
>> - .release = seq_release,
>> + .release = ima_measurements_release,
>> +};
>> +
>> +static const struct seq_operations
>> ima_ascii_measurements_staged_seqops = {
>> + .start = ima_measurements_staged_start,
>> + .next = ima_measurements_staged_next,
>> + .stop = ima_measurements_stop,
>> + .show = ima_ascii_measurements_show
>> +};
>> +
>> +static int ima_ascii_measurements_staged_open(struct inode *inode,
>> + struct file *file)
>> +{
>> + return _ima_measurements_open(inode, file,
>> + &ima_ascii_measurements_staged_seqops);
>> +}
>> +
>> +static const struct file_operations ima_ascii_measurements_staged_ops
>> = {
>> + .open = ima_ascii_measurements_staged_open,
>> + .read = ima_measurements_staged_read,
>> + .write = ima_measurements_staged_write,
>> + .llseek = seq_lseek,
>> + .release = ima_measurements_release,
>> };
>> static ssize_t ima_read_policy(char *path)
>> @@ -419,6 +608,25 @@ static int __init
>> create_securityfs_measurement_lists(void)
>> &ima_measurements_ops);
>> if (IS_ERR(dentry))
>> return PTR_ERR(dentry);
>> +
>> + sprintf(file_name, "ascii_runtime_measurements_staged_%s",
>> + hash_algo_name[algo]);
>> + dentry = securityfs_create_file(file_name,
>> + S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
>> + ima_dir, (void *)(uintptr_t)i,
>> + &ima_ascii_measurements_staged_ops);
>> + if (IS_ERR(dentry))
>> + return PTR_ERR(dentry);
>> +
>> + sprintf(file_name, "binary_runtime_measurements_staged_%s",
>> + hash_algo_name[algo]);
>> + dentry = securityfs_create_file(file_name,
>> + S_IRUSR | S_IRGRP |
>> + S_IWUSR | S_IWGRP,
>> + ima_dir, (void *)(uintptr_t)i,
>> + &ima_measurements_staged_ops);
>> + if (IS_ERR(dentry))
>> + return PTR_ERR(dentry);
>> }
>> return 0;
>> @@ -528,6 +736,20 @@ int __init ima_fs_init(void)
>> goto out;
>> }
>> + dentry =
>> securityfs_create_symlink("binary_runtime_measurements_staged",
>> + ima_dir, "binary_runtime_measurements_staged_sha1", NULL);
>> + if (IS_ERR(dentry)) {
>> + ret = PTR_ERR(dentry);
>> + goto out;
>> + }
>> +
>> + dentry =
>> securityfs_create_symlink("ascii_runtime_measurements_staged",
>> + ima_dir, "ascii_runtime_measurements_staged_sha1", NULL);
>> + if (IS_ERR(dentry)) {
>> + ret = PTR_ERR(dentry);
>> + goto out;
>> + }
>> +
>> dentry = securityfs_create_file("runtime_measurements_count",
>> S_IRUSR | S_IRGRP, ima_dir, NULL,
>> &ima_measurements_count_ops);
>> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/
>> ima/ima_kexec.c
>> index 7362f68f2d8b..13c7e78aeefd 100644
>> --- a/security/integrity/ima/ima_kexec.c
>> +++ b/security/integrity/ima/ima_kexec.c
>> @@ -40,8 +40,8 @@ void ima_measure_kexec_event(const char *event_name)
>> long len;
>> int n;
>> - buf_size = ima_get_binary_runtime_size();
>> - len = atomic_long_read(&ima_htable.len);
>> + buf_size = ima_get_binary_runtime_size(BINARY_SIZE_FULL);
>> + len = atomic_long_read(&ima_htable.len[BINARY_SIZE_FULL]);
>> n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
>> "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
>> @@ -78,6 +78,17 @@ static int ima_alloc_kexec_file_buf(size_t
>> segment_size)
>> return 0;
>> }
>> +static int ima_dump_measurement(struct ima_kexec_hdr *khdr,
>> + struct ima_queue_entry *qe)
>> +{
>> + if (ima_kexec_file.count >= ima_kexec_file.size)
>> + return -EINVAL;
>> +
>> + khdr->count++;
>> + ima_measurements_show(&ima_kexec_file, qe);
>> + return 0;
>> +}
>> +
>> static int ima_dump_measurement_list(unsigned long *buffer_size,
>> void **buffer,
>> unsigned long segment_size)
>> {
>> @@ -93,17 +104,25 @@ static int ima_dump_measurement_list(unsigned
>> long *buffer_size, void **buffer,
>> memset(&khdr, 0, sizeof(khdr));
>> khdr.version = 1;
>> - /* This is an append-only list, no need to hold the RCU read lock */
>> - list_for_each_entry_rcu(qe, &ima_measurements, later, true) {
>> - if (ima_kexec_file.count < ima_kexec_file.size) {
>> - khdr.count++;
>> - ima_measurements_show(&ima_kexec_file, qe);
>> - } else {
>> - ret = -EINVAL;
>> +
>> + /* It can race with ima_queue_stage_trim(). */
>> + mutex_lock(&ima_extend_list_mutex);
>> +
>> + list_for_each_entry(qe, &ima_measurements_staged, later) {
>> + ret = ima_dump_measurement(&khdr, qe);
>> + if (ret < 0)
>> + break;
>> + }
>> +
>> + list_for_each_entry(qe, &ima_measurements, later) {
>> + if (!ret)
>> + ret = ima_dump_measurement(&khdr, qe);
>> + if (ret < 0)
>> break;
>> - }
>> }
>> + mutex_unlock(&ima_extend_list_mutex);
>> +
>> /*
>> * fill in reserved space with some buffer details
>> * (eg. version, buffer size, number of measurements)
>> @@ -157,7 +176,8 @@ void ima_add_kexec_buffer(struct kimage *image)
>> else
>> extra_memory = CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB * 1024;
>> - binary_runtime_size = ima_get_binary_runtime_size() + extra_memory;
>> + binary_runtime_size =
>> ima_get_binary_runtime_size(BINARY_SIZE_STAGED) +
>> + extra_memory;
>> if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
>> kexec_segment_size = ULONG_MAX;
>> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/
>> ima/ima_queue.c
>> index 590637e81ad1..7dfa24b8ae31 100644
>> --- a/security/integrity/ima/ima_queue.c
>> +++ b/security/integrity/ima/ima_queue.c
>> @@ -22,19 +22,32 @@
>> #define AUDIT_CAUSE_LEN_MAX 32
>> +bool ima_flush_htable;
>> +static int __init ima_flush_htable_setup(char *str)
>> +{
>> + ima_flush_htable = true;
>> + return 1;
>> +}
>> +__setup("ima_flush_htable", ima_flush_htable_setup);
>> +
>> /* pre-allocated array of tpm_digest structures to extend a PCR */
>> static struct tpm_digest *digests;
>> LIST_HEAD(ima_measurements); /* list of all measurements */
>> +LIST_HEAD(ima_measurements_staged); /* list of staged measurements */
>> +static LIST_HEAD(ima_measurements_trim); /* list of measurements to
>> trim */
>> +bool ima_measurements_staged_exist; /* If there are staged
>> measurements */
>> #ifdef CONFIG_IMA_KEXEC
>> -static unsigned long binary_runtime_size;
>> +static unsigned long binary_runtime_size[BINARY__LAST];
>> #else
>> -static unsigned long binary_runtime_size = ULONG_MAX;
>> +static unsigned long binary_runtime_size[BINARY_SIZE] = ULONG_MAX;
>> +static unsigned long binary_runtime_size[BINARY_SIZE_FULL] = ULONG_MAX;
>> +static unsigned long binary_runtime_size[BINARY_SIZE_STAGED] =
>> ULONG_MAX;
>> #endif
>> /* key: inode (before secure-hashing a file) */
>> struct ima_h_table ima_htable = {
>> - .len = ATOMIC_LONG_INIT(0),
>> + .len = { ATOMIC_LONG_INIT(0) },
>> .violations = ATOMIC_LONG_INIT(0),
>> .queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
>> };
>> @@ -43,7 +56,7 @@ struct ima_h_table ima_htable = {
>> * and extending the TPM PCR aggregate. Since tpm_extend can take
>> * long (and the tpm driver uses a mutex), we can't use the spinlock.
>> */
>> -static DEFINE_MUTEX(ima_extend_list_mutex);
>> +DEFINE_MUTEX(ima_extend_list_mutex);
>> /*
>> * Used internally by the kernel to suspend measurements.
>> @@ -101,7 +114,7 @@ static int ima_add_digest_entry(struct
>> ima_template_entry *entry,
>> bool update_htable)
>> {
>> struct ima_queue_entry *qe;
>> - unsigned int key;
>> + unsigned int i, key;
>> qe = kmalloc(sizeof(*qe), GFP_KERNEL);
>> if (qe == NULL) {
>> @@ -113,18 +126,23 @@ static int ima_add_digest_entry(struct
>> ima_template_entry *entry,
>> INIT_LIST_HEAD(&qe->later);
>> list_add_tail_rcu(&qe->later, &ima_measurements);
>> - atomic_long_inc(&ima_htable.len);
>> + for (i = 0; i < BINARY__LAST; i++)
>> + atomic_long_inc(&ima_htable.len[i]);
>> +
>> if (update_htable) {
>> key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
>> hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
>> }
>> - if (binary_runtime_size != ULONG_MAX) {
>> + if (binary_runtime_size[BINARY_SIZE_FULL] != ULONG_MAX) {
>> int size;
>> size = get_binary_runtime_size(entry);
>> - binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
>> - binary_runtime_size + size : ULONG_MAX;
>> +
>> + for (i = 0; i < BINARY__LAST; i++)
>> + binary_runtime_size[i] =
>> + (binary_runtime_size[i] < ULONG_MAX - size) ?
>> + binary_runtime_size[i] + size : ULONG_MAX;
>> }
>> return 0;
>> }
>> @@ -134,12 +152,18 @@ static int ima_add_digest_entry(struct
>> ima_template_entry *entry,
>> * entire binary_runtime_measurement list, including the ima_kexec_hdr
>> * structure.
>> */
>> -unsigned long ima_get_binary_runtime_size(void)
>> +unsigned long ima_get_binary_runtime_size(enum binary_size_types type)
>> {
>> - if (binary_runtime_size >= (ULONG_MAX - sizeof(struct
>> ima_kexec_hdr)))
>> + unsigned long val;
>> +
>> + mutex_lock(&ima_extend_list_mutex);
>> + val = binary_runtime_size[type];
>> + mutex_unlock(&ima_extend_list_mutex);
>> +
>> + if (val >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
>> return ULONG_MAX;
>> else
>> - return binary_runtime_size + sizeof(struct ima_kexec_hdr);
>> + return val + sizeof(struct ima_kexec_hdr);
>> }
>> static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
>> @@ -220,6 +244,127 @@ int ima_add_template_entry(struct
>> ima_template_entry *entry, int violation,
>> return result;
>> }
>> +int ima_queue_stage_trim(unsigned long req_value, bool trim)
>> +{
>> + unsigned long req_value_copy = req_value, to_remove = 0;
>> + struct list_head *moved = &ima_measurements_staged;
>> + struct ima_queue_entry *qe;
>> +
>> + if (req_value == 0 || req_value > LONG_MAX)
>> + return -EINVAL;
>> +
>> + if (ima_measurements_staged_exist)
>> + return -EEXIST;
>> +
>> + if (trim)
>> + moved = &ima_measurements_trim;
>> +
>> + mutex_lock(&ima_extend_list_mutex);
>> + if (list_empty(&ima_measurements)) {
>> + mutex_unlock(&ima_extend_list_mutex);
>> + return -ENOENT;
>> + }
>> +
>> + if (req_value == LONG_MAX) {
>> + list_replace(&ima_measurements, moved);
>> + INIT_LIST_HEAD(&ima_measurements);
>> + atomic_long_set(&ima_htable.len[BINARY_SIZE], 0);
>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>> + binary_runtime_size[BINARY_SIZE] = 0;
>> +
>> + if (trim) {
>> + atomic_long_set(&ima_htable.len[BINARY_SIZE_STAGED], 0);
>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>> + binary_runtime_size[BINARY_SIZE_STAGED] = 0;
>> + }
>> + } else {
>> + list_for_each_entry(qe, &ima_measurements, later) {
>> + to_remove += get_binary_runtime_size(qe->entry);
>> + if (--req_value_copy == 0)
>> + break;
>> + }
>> +
>> + if (req_value_copy > 0) {
>> + mutex_unlock(&ima_extend_list_mutex);
>> + return -ENOENT;
>> + }
>> +
>> + __list_cut_position(moved, &ima_measurements, &qe->later);
>> + atomic_long_sub(req_value, &ima_htable.len[BINARY_SIZE]);
>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>> + binary_runtime_size[BINARY_SIZE] -= to_remove;
>> +
>> + if (trim) {
>> + atomic_long_sub(req_value,
>> + &ima_htable.len[BINARY_SIZE_STAGED]);
>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>> + binary_runtime_size[BINARY_SIZE_STAGED] -=
>> + to_remove;
>> + }
>> + }
>> +
>> + if (ima_flush_htable)
>> + /* Either staged/trimmed entries are removed from hash table. */
>> + list_for_each_entry(qe, moved, later)
>> + /* It can race with ima_lookup_digest_entry(). */
>> + hlist_del_rcu(&qe->hnext);
>> +
>> + mutex_unlock(&ima_extend_list_mutex);
>> + ima_measurements_staged_exist = true;
>> +
>> + if (ima_flush_htable)
>> + synchronize_rcu();
>> +
>> + if (trim)
>> + return ima_queue_delete_staged_trimmed(true);
>> +
>> + return 0;
>> +}
>> +
>> +int ima_queue_delete_staged_trimmed(bool staged_moved)
>> +{
>> + struct ima_queue_entry *qe, *qe_tmp;
>> + unsigned int i;
>> +
>> + if (!ima_measurements_staged_exist)
>> + return -ENOENT;
>> +
>> + if (!staged_moved) {
>> + mutex_lock(&ima_extend_list_mutex);
>> + list_replace(&ima_measurements_staged, &ima_measurements_trim);
>> + INIT_LIST_HEAD(&ima_measurements_staged);
>> + atomic_long_set(&ima_htable.len[BINARY_SIZE_STAGED], 0);
>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>> + binary_runtime_size[BINARY_SIZE_STAGED] = 0;
>> +
>> + mutex_unlock(&ima_extend_list_mutex);
>> + }
>> +
>> + list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_trim,
>> later) {
>> + /*
>> + * Ok because after list delete qe is only accessed by
>> + * ima_lookup_digest_entry().
>> + */
>> + for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
>> + kfree(qe->entry->template_data[i].data);
>> + qe->entry->template_data[i].data = NULL;
>> + qe->entry->template_data[i].len = 0;
>> + }
>> +
>> + list_del(&qe->later);
>> +
>> + /* No leak if !ima_flush_htable, referenced by ima_htable. */
>> + if (ima_flush_htable) {
>> + kfree(qe->entry->digests);
>> + kfree(qe->entry);
>> + kfree(qe);
>> + }
>> + }
>> +
>> + ima_measurements_staged_exist = false;
>> + return 0;
>> +}
>> +
>> int ima_restore_measurement_entry(struct ima_template_entry *entry)
>> {
>> int result = 0;
>
>
^ permalink raw reply
* Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure
From: Günther Noack @ 2026-01-29 7:56 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Randy Dunlap
In-Reply-To: <20260128.quaido7ia0Xu@digikod.net>
On Wed, Jan 28, 2026 at 10:34:02PM +0100, Mickaël Salaün wrote:
> On Sun, Jan 25, 2026 at 08:58:53PM +0100, Günther Noack wrote:
> > Tradeoffs:
> >
> > This change improves performance, at a slight size increase to the
> > layer masks data structure.
> >
> > At the moment, for the filesystem access rights, the data structure
> > has the same size as before, but once we introduce the 17th filesystem
> > access right, it will double in size (from 32 to 64 bytes), as
>
> ...for all access rights (e.g. even if there is no new network one)
Added.
> > --- a/security/landlock/audit.c
> > +++ b/security/landlock/audit.c
> > @@ -180,38 +180,21 @@ static void test_get_hierarchy(struct kunit *const test)
> >
> > #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> >
> > +/* get_denied_layer - get the youngest layer that denied the access_request */
>
> /* Get the youngest layer that denied the access_request. */
OK, done. I also changed to non-docstring style for the
access_mask_subset() helper.
>
> > static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> > access_mask_t *const access_request,
> > - const layer_mask_t (*const layer_masks)[],
> > - const size_t layer_masks_size)
> > + const struct layer_access_masks *masks)
> > {
> > - const unsigned long access_req = *access_request;
> > - unsigned long access_bit;
> > - access_mask_t missing = 0;
> > - long youngest_layer = -1;
> > -
> > - for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> > - const layer_mask_t mask = (*layer_masks)[access_bit];
> > - long layer;
> > -
> > - if (!mask)
> > - continue;
> > -
> > - /* __fls(1) == 0 */
> > - layer = __fls(mask);
> > - if (layer > youngest_layer) {
> > - youngest_layer = layer;
> > - missing = BIT(access_bit);
> > - } else if (layer == youngest_layer) {
> > - missing |= BIT(access_bit);
> > + for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
>
> size_t i
This is one of the two places where this didn't work.
The loop goes from top to bottom here, and the "i >= 0" check would
always be true for a size_t.
If there is a more idiomatic way to write that loop, I can switch to
it, but would otherwise lean towards keeping it as it is?
> > + if (masks->access[i] & *access_request) {
> > + *access_request &= masks->access[i];
> > + return i;
> > }
> > }
> >
> > - for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
> > - const layer_mask_t mask = (*layer_masks)[access_bit];
> > + for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
>
> size_t i
Ditto, the loop goes from top to bottom here.
> > + const access_mask_t denied = masks->access[i] & optional_access;
> > + const unsigned long newly_denied = denied & ~all_denied;
> >
> > -static bool
> > -scope_to_request(const access_mask_t access_request,
> > - layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> > +static bool scope_to_request(const access_mask_t access_request,
> > + struct layer_access_masks *masks)
> > {
> > - const unsigned long access_req = access_request;
> > - unsigned long access_bit;
> > + bool saw_unfulfilled_access = false;
> >
> > - if (WARN_ON_ONCE(!layer_masks))
> > + if (WARN_ON_ONCE(!masks))
> > return true;
> >
> > - for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
> > - (*layer_masks)[access_bit] = 0;
> > -
> > - return is_layer_masks_allowed(layer_masks);
> > + for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
> > + masks->access[i] &= access_request;
> > + if (masks->access[i])
>
> {
>
> > + saw_unfulfilled_access = true;
>
> break;
> }
Two lines above, this loop mutates masks->access[...]:
masks->access[i] &= access_request
If we break the loop early, we would not actually scope it down to the
request entirely? Is this safe?
> > + }
> > + return !saw_unfulfilled_access;
> > }
–Günther
^ permalink raw reply
* Re: [PATCH v2 1/3] selftests/landlock: Add filesystem access benchmark
From: Mickaël Salaün @ 2026-01-28 21:31 UTC (permalink / raw)
To: Günther Noack
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Randy Dunlap
In-Reply-To: <20260125195853.109967-2-gnoack3000@gmail.com>
On Sun, Jan 25, 2026 at 08:58:51PM +0100, Günther Noack wrote:
> fs_bench benchmarks the performance of Landlock's path walk
> by exercising it in a scenario that amplifies Landlock's overhead:
>
> * Create a large number of nested directories
> * Enforce a Landlock policy in which a rule is associated with each of
> these subdirectories
> * Benchmark openat() applied to the deepest directory,
> forcing Landlock to walk the entire path.
>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> tools/testing/selftests/landlock/.gitignore | 1 +
> tools/testing/selftests/landlock/Makefile | 1 +
> tools/testing/selftests/landlock/fs_bench.c | 161 ++++++++++++++++++++
> 3 files changed, 163 insertions(+)
> create mode 100644 tools/testing/selftests/landlock/fs_bench.c
>
> diff --git a/tools/testing/selftests/landlock/.gitignore b/tools/testing/selftests/landlock/.gitignore
> index a820329cae0d..1974e17a2611 100644
> --- a/tools/testing/selftests/landlock/.gitignore
> +++ b/tools/testing/selftests/landlock/.gitignore
> @@ -1,4 +1,5 @@
> /*_test
> +/fs_bench
> /sandbox-and-launch
> /true
> /wait-pipe
> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
> index 044b83bde16e..fc43225d319a 100644
> --- a/tools/testing/selftests/landlock/Makefile
> +++ b/tools/testing/selftests/landlock/Makefile
> @@ -9,6 +9,7 @@ LOCAL_HDRS += $(wildcard *.h)
> src_test := $(wildcard *_test.c)
>
> TEST_GEN_PROGS := $(src_test:.c=)
> +TEST_GEN_PROGS += fs_bench
>
> TEST_GEN_PROGS_EXTENDED := \
> true \
> diff --git a/tools/testing/selftests/landlock/fs_bench.c b/tools/testing/selftests/landlock/fs_bench.c
> new file mode 100644
> index 000000000000..a3b686418bc5
> --- /dev/null
> +++ b/tools/testing/selftests/landlock/fs_bench.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Landlock filesystem benchmark
You might want to add some copyright.
> + */
> +
> +#define _GNU_SOURCE
> +#include <err.h>
> +#include <fcntl.h>
> +#include <linux/landlock.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <sys/prctl.h>
> +#include <sys/stat.h>
> +#include <sys/syscall.h>
> +#include <sys/times.h>
> +#include <time.h>
> +#include <unistd.h>
> +
> +void usage(const char *argv0)
const
> +{
> + printf("Usage:\n");
> + printf(" %s [OPTIONS]\n", argv0);
> + printf("\n");
> + printf(" Benchmark expensive Landlock checks for D nested dirs\n");
> + printf("\n");
> + printf("Options:\n");
> + printf(" -h help\n");
> + printf(" -L disable Landlock (as a baseline)\n");
> + printf(" -d D set directory depth to D\n");
> + printf(" -n N set number of benchmark iterations to N\n");
> +}
> +
> +/*
> + * Build a deep directory, enforce Landlock and return the FD to the
> + * deepest dir. On any failure, exit the process with an error.
> + */
> +int build_directory(size_t depth, bool use_landlock)
const
> +{
> + const char *path = "d"; /* directory name */
> + int abi, ruleset_fd, current, previous;
> +
> + if (use_landlock) {
> + abi = syscall(SYS_landlock_create_ruleset, NULL, 0,
> + LANDLOCK_CREATE_RULESET_VERSION);
Please include wrappers.h and use the related syscall helpers. One of
the benefit is to use __NR_* constants defined by the installed kernel
headers.
> + if (abi < 7)
> + err(1, "Landlock ABI too low: got %d, wanted 7+", abi);
> + }
> +
> + ruleset_fd = -1;
> + if (use_landlock) {
> + struct landlock_ruleset_attr attr = {
> + .handled_access_fs =
> + 0xffff, /* All FS access rights as of 2026-01 */
> + };
> + ruleset_fd = syscall(SYS_landlock_create_ruleset, &attr,
> + sizeof(attr), 0U);
> + if (ruleset_fd < 0)
> + err(1, "landlock_create_ruleset");
> + }
> +
> + current = open(".", O_PATH);
> + if (current < 0)
> + err(1, "open(.)");
> +
> + while (depth--) {
> + if (use_landlock) {
> + struct landlock_path_beneath_attr attr = {
> + .allowed_access = LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + .parent_fd = current,
> + };
> + if (syscall(SYS_landlock_add_rule, ruleset_fd,
> + LANDLOCK_RULE_PATH_BENEATH, &attr, 0) < 0)
> + err(1, "landlock_add_rule");
> + }
> +
> + if (mkdirat(current, path, 0700) < 0)
> + err(1, "mkdirat(%s)", path);
We should have a loop to build the directories, then start the timer and
have another loop to add Landlock rules.
> +
> + previous = current;
> + current = openat(current, path, O_PATH);
> + if (current < 0)
> + err(1, "open(%s)", path);
> +
> + close(previous);
> + }
> +
> + if (use_landlock) {
> + if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) < 0)
> + err(1, "prctl");
> +
> + if (syscall(SYS_landlock_restrict_self, ruleset_fd, 0) < 0)
> + err(1, "landlock_restrict_self");
> + }
> +
> + close(ruleset_fd);
> + return current;
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + bool use_landlock = true;
> + size_t num_iterations = 100000;
> + size_t num_subdirs = 10000;
> + int c, current, fd;
> + struct tms start_time, end_time;
> +
> + setbuf(stdout, NULL);
> + while ((c = getopt(argc, argv, "hLd:n:")) != -1) {
> + switch (c) {
> + case 'h':
> + usage(argv[0]);
> + return EXIT_SUCCESS;
> + case 'L':
> + use_landlock = false;
> + break;
> + case 'd':
> + num_subdirs = atoi(optarg);
> + break;
> + case 'n':
> + num_iterations = atoi(optarg);
> + break;
> + default:
> + usage(argv[0]);
> + return EXIT_FAILURE;
> + }
> + }
> +
> + printf("*** Benchmark ***\n");
We should probably use ksft_*() helpers in main (see
seccomp_benchmark.c).
> + printf("%zu dirs, %zu iterations, %s landlock\n", num_subdirs,
> + num_iterations, use_landlock ? "with" : "without");
> +
> + if (times(&start_time) == -1)
> + err(1, "times");
> +
> + current = build_directory(num_subdirs, use_landlock);
> +
> + for (int i = 0; i < num_iterations; i++) {
> + fd = openat(current, ".", O_DIRECTORY);
We can use AT_EMPTY_PATH (with an empty path) instead of "."
I guess the benchmark should not change, but better to check again.
> + if (fd != -1) {
> + if (use_landlock)
> + errx(1, "openat succeeded, expected error");
> +
> + close(fd);
> + }
> + }
> +
> + if (times(&end_time) == -1)
> + err(1, "times");
The created directories should be removed here (setup and teardown).
> +
> + printf("*** Benchmark concluded ***\n");
> + printf("System: %ld clocks\n",
> + end_time.tms_stime - start_time.tms_stime);
> + printf("User : %ld clocks\n",
> + end_time.tms_utime - start_time.tms_utime);
> + printf("Clocks per second: %ld\n", CLOCKS_PER_SEC);
> +
> + close(current);
> +}
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH v2 0/3] landlock: Refactor layer masks
From: Mickaël Salaün @ 2026-01-28 21:31 UTC (permalink / raw)
To: Günther Noack
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Randy Dunlap
In-Reply-To: <20260125195853.109967-1-gnoack3000@gmail.com>
On Sun, Jan 25, 2026 at 08:58:50PM +0100, Günther Noack wrote:
> Hello!
>
> This patch set "transposes" the layer masks matrix, which was
> previously modeled as a access-max-sized array of layer masks, and
> changes it to be a layer-max-sized array of access masks instead.
> (It is a pure refactoring, there are no user-visible changes.)
>
> This unlocks a few code simplifications and in multiple places it
> removes the need for loops and branches that deal with individual
> bits. Instead, the changed data structure now lends itself for more
> bitwise operations. The underlying hypothesis for me was that by
> using more bitwise operations and fewer branches, we would get an
> overall speedup even when the data structure size increases slightly
> in some cases.
>
> Tentative results with and without this patch set show that the
> hypothesis likely holds true. The benchmark I used exercises a "worst
> case" scenario that attempts to be bottlenecked on the affected code:
> constructs a large number of nested directories, with one "path
> beneath" rule each and then tries to open the innermost directory many
> times. The benchmark is intentionally unrealistic to amplify the
> amount of time used for the path walk logic and forces Landlock to
> walk the full path (eventually failing the open syscall). (I'll send
> the benchmark program in a reply to this mail for full transparency.)
>
> Measured with the benchmark program, the patch set results in a
> speedup of about -10%. The benchmark results are only tentative and
> have been produced in Qemu:
>
> With the patch, the benchmark runs in 6046 clocks (measured with
> times(3)):
>
> *** Benchmark ***
> 10000 dirs, 100000 iterations, with landlock
> *** Benchmark concluded ***
> System: 6046 clocks
> User : 1 clocks
> Clocks per second: 1000000
>
> Without the patch, we get 6713 clocks, which is 11% more
>
> *** Benchmark ***
> 10000 dirs, 100000 iterations, with landlock
> *** Benchmark concluded ***
> System: 6713 clocks
> User : 0 clocks
> Clocks per second: 1000000
>
> The base revision used for benchmarking was commit 7a51784da76d
> ("tools/sched_ext: update scx_show_state.py for scx_aborting change")
>
> In real-life scenarios, the speed improvement from this patch set will
> be less pronounced than in the artificial benchmark, as people do not
> usually stack directories that deeply and attach so many rules to
> them, and the EACCES error should also be the exception rather than
> the norm.
>
> I am looking forward to your feedback.
>
> P.S.: I am open to suggestions on what the "layer masks" variables
> should be called, because the name "layer masks" might be less
> appropriate after this change. I have not fixed up the name
> everywhere because fixing up the code took priority for now.
Could you please clarify your thoughts and explain why this name might
not be appropriate anymore? Any list of name proposals?
If we rename the variables, this should be done in the same refactoring
patch.
>
> ---
> Changes since previous versions:
>
> V2: (This patch set)
>
> * Remove the refactoring around the deny_mask_t type,
> it is better to send that as a separate patch (mic review)
Feel free to include the new dedicated patch in this series.
> * Added the benchmark program to the selftests
> * Fix unused variable report for "access_dom":
> https://lore.kernel.org/all/202601200900.wonk9M0m-lkp@intel.com/
> * Use size_t and ARRAY_SIZE to loop over the layers (mic review)
> * Documentation
> * Fixing up and adding back documentaiton (mic review)
> * Documented landlock_unmask_layers()
> * Fixed up kernel docs in a place where it was improperly updated
> (Spotted by Randy Dunlap
> https://lore.kernel.org/all/20260123025121.3713403-1-rdunlap@infradead.org/)
> * Minor
> * Const, some newlines (mic review)
>
> V1: (Initial version)
>
> https://lore.kernel.org/all/20251230103917.10549-3-gnoack3000@gmail.com/
>
> Günther Noack (3):
> selftests/landlock: Add filesystem access benchmark
> landlock: access_mask_subset() helper
> landlock: transpose the layer masks data structure
>
> security/landlock/access.h | 16 +-
> security/landlock/audit.c | 84 ++---
> security/landlock/audit.h | 3 +-
> security/landlock/domain.c | 45 +--
> security/landlock/domain.h | 4 +-
> security/landlock/fs.c | 354 +++++++++-----------
> security/landlock/net.c | 11 +-
> security/landlock/ruleset.c | 88 ++---
> security/landlock/ruleset.h | 21 +-
> tools/testing/selftests/landlock/.gitignore | 1 +
> tools/testing/selftests/landlock/Makefile | 1 +
> tools/testing/selftests/landlock/fs_bench.c | 161 +++++++++
> 12 files changed, 444 insertions(+), 345 deletions(-)
> create mode 100644 tools/testing/selftests/landlock/fs_bench.c
>
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [PATCH v2 3/3] landlock: transpose the layer masks data structure
From: Mickaël Salaün @ 2026-01-28 21:34 UTC (permalink / raw)
To: Günther Noack
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Randy Dunlap
In-Reply-To: <20260125195853.109967-4-gnoack3000@gmail.com>
On Sun, Jan 25, 2026 at 08:58:53PM +0100, Günther Noack wrote:
> The layer masks data structure tracks the requested but unfulfilled
> access rights during an operation's security check. It stores one bit
> for each combination of access right and layer index. If the bit is
> set, that access right is not granted (yet) in the given layer and we
> have to traverse the path further upwards to grant it.
>
> Previously, the layer masks were stored as arrays mapping from access
> right indices to layer_mask_t. The layer_mask_t value then indicates
> all layers in which the given access right is still (tentatively)
> denied.
>
> This patch introduces struct layer_access_masks instead: This struct
> contains an array with the access_mask_t of each (tentatively) denied
> access right in that layer.
>
> The hypothesis of this patch is that this simplifies the code enough
> so that the resulting code will run faster:
>
> * We can use bitwise operations in multiple places where we previously
> looped over bits individually with macros. (Should require less
> branch speculation and lends itself to better loop unrolling.)
>
> * Code is ~75 lines smaller.
>
> Other noteworthy changes:
>
> * In no_more_access(), call a new helper function may_refer(), which
> only solves the asymmetric case. Previously, the code interleaved
> the checks for the two symmetric cases in RENAME_EXCHANGE. It feels
> that the code is clearer when renames without RENAME_EXCHANGE are
> more obviously the normal case.
>
> Tradeoffs:
>
> This change improves performance, at a slight size increase to the
> layer masks data structure.
>
> At the moment, for the filesystem access rights, the data structure
> has the same size as before, but once we introduce the 17th filesystem
> access right, it will double in size (from 32 to 64 bytes), as
...for all access rights (e.g. even if there is no new network one)
> access_mask_t grows from 16 to 32 bit. See the link below for
> measurements.
>
> Link: https://lore.kernel.org/all/20260120.haeCh4li9Vae@digikod.net/
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> security/landlock/access.h | 10 +-
> security/landlock/audit.c | 84 +++------
> security/landlock/audit.h | 3 +-
> security/landlock/domain.c | 45 +++--
> security/landlock/domain.h | 4 +-
> security/landlock/fs.c | 352 ++++++++++++++++--------------------
> security/landlock/net.c | 11 +-
> security/landlock/ruleset.c | 88 ++++-----
> security/landlock/ruleset.h | 21 ++-
> 9 files changed, 274 insertions(+), 344 deletions(-)
>
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 5c0caef9eaf6..1c911fa3555d 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -61,14 +61,14 @@ union access_masks_all {
> static_assert(sizeof(typeof_member(union access_masks_all, masks)) ==
> sizeof(typeof_member(union access_masks_all, all)));
>
> -typedef u16 layer_mask_t;
> -
> -/* Makes sure all layers can be checked. */
> -static_assert(BITS_PER_TYPE(layer_mask_t) >= LANDLOCK_MAX_NUM_LAYERS);
> -
> /*
> * Tracks domains responsible of a denied access. This is required to avoid
> * storing in each object the full layer_masks[] required by update_request().
> + *
> + * Each nibble represents the layer index of the newest layer which denied a
> + * certain access right. For file system access rights, the upper four bits are
> + * the index of the layer which denies LANDLOCK_ACCESS_FS_IOCTL_DEV and the
> + * lower nibble represents LANDLOCK_ACCESS_FS_TRUNCATE.
> */
> typedef u8 deny_masks_t;
>
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index e899995f1fd5..979a33f480aa 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -180,38 +180,21 @@ static void test_get_hierarchy(struct kunit *const test)
>
> #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
>
> +/* get_denied_layer - get the youngest layer that denied the access_request */
/* Get the youngest layer that denied the access_request. */
> static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> access_mask_t *const access_request,
> - const layer_mask_t (*const layer_masks)[],
> - const size_t layer_masks_size)
> + const struct layer_access_masks *masks)
> {
> - const unsigned long access_req = *access_request;
> - unsigned long access_bit;
> - access_mask_t missing = 0;
> - long youngest_layer = -1;
> -
> - for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> - const layer_mask_t mask = (*layer_masks)[access_bit];
> - long layer;
> -
> - if (!mask)
> - continue;
> -
> - /* __fls(1) == 0 */
> - layer = __fls(mask);
> - if (layer > youngest_layer) {
> - youngest_layer = layer;
> - missing = BIT(access_bit);
> - } else if (layer == youngest_layer) {
> - missing |= BIT(access_bit);
> + for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
size_t i
> + if (masks->access[i] & *access_request) {
> + *access_request &= masks->access[i];
> + return i;
> }
> }
>
> - *access_request = missing;
> - if (youngest_layer == -1)
> - return domain->num_layers - 1;
> -
> - return youngest_layer;
> + /* Not found - fall back to default values */
> + *access_request = 0;
> + return domain->num_layers - 1;
> }
>
> #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
> @@ -221,50 +204,39 @@ static void test_get_denied_layer(struct kunit *const test)
> const struct landlock_ruleset dom = {
> .num_layers = 5,
> };
> - const layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT(1),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_READ_DIR)] = BIT(1) | BIT(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_REMOVE_DIR)] = BIT(2),
> + const struct layer_access_masks masks = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_READ_DIR,
> + .access[1] = LANDLOCK_ACCESS_FS_READ_FILE |
> + LANDLOCK_ACCESS_FS_READ_DIR,
> + .access[2] = LANDLOCK_ACCESS_FS_REMOVE_DIR,
> };
> access_mask_t access;
>
> access = LANDLOCK_ACCESS_FS_EXECUTE;
> - KUNIT_EXPECT_EQ(test, 0,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 0, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_EXECUTE);
>
> access = LANDLOCK_ACCESS_FS_READ_FILE;
> - KUNIT_EXPECT_EQ(test, 1,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_FILE);
>
> access = LANDLOCK_ACCESS_FS_READ_DIR;
> - KUNIT_EXPECT_EQ(test, 1,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
>
> access = LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR;
> - KUNIT_EXPECT_EQ(test, 1,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access,
> LANDLOCK_ACCESS_FS_READ_FILE |
> LANDLOCK_ACCESS_FS_READ_DIR);
>
> access = LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_READ_DIR;
> - KUNIT_EXPECT_EQ(test, 1,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 1, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, LANDLOCK_ACCESS_FS_READ_DIR);
>
> access = LANDLOCK_ACCESS_FS_WRITE_FILE;
> - KUNIT_EXPECT_EQ(test, 4,
> - get_denied_layer(&dom, &access, &layer_masks,
> - sizeof(layer_masks)));
> + KUNIT_EXPECT_EQ(test, 4, get_denied_layer(&dom, &access, &masks));
> KUNIT_EXPECT_EQ(test, access, 0);
> }
>
> @@ -361,18 +333,15 @@ static bool is_valid_request(const struct landlock_request *const request)
> return false;
>
> if (request->access) {
> - if (WARN_ON_ONCE(!(!!request->layer_masks ^
> + if (WARN_ON_ONCE(!(!!request->masks ^
> !!request->all_existing_optional_access)))
> return false;
> } else {
> - if (WARN_ON_ONCE(request->layer_masks ||
> + if (WARN_ON_ONCE(request->masks ||
> request->all_existing_optional_access))
> return false;
> }
>
> - if (WARN_ON_ONCE(!!request->layer_masks ^ !!request->layer_masks_size))
> - return false;
> -
> if (request->deny_masks) {
> if (WARN_ON_ONCE(!request->all_existing_optional_access))
> return false;
> @@ -405,13 +374,12 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
> missing = request->access;
> if (missing) {
> /* Gets the nearest domain that denies the request. */
> - if (request->layer_masks) {
> + if (request->masks) {
> youngest_layer = get_denied_layer(
> - subject->domain, &missing, request->layer_masks,
> - request->layer_masks_size);
> + subject->domain, &missing, request->masks);
> } else {
> youngest_layer = get_layer_from_deny_masks(
> - &missing, request->all_existing_optional_access,
> + &missing, _LANDLOCK_ACCESS_FS_OPTIONAL,
> request->deny_masks);
> }
> youngest_denied =
> diff --git a/security/landlock/audit.h b/security/landlock/audit.h
> index 92428b7fc4d8..104472060ef5 100644
> --- a/security/landlock/audit.h
> +++ b/security/landlock/audit.h
> @@ -43,8 +43,7 @@ struct landlock_request {
> access_mask_t access;
>
> /* Required fields for requests with layer masks. */
> - const layer_mask_t (*layer_masks)[];
> - size_t layer_masks_size;
> + const struct layer_access_masks *masks;
>
> /* Required fields for requests with deny masks. */
> const access_mask_t all_existing_optional_access;
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index a647b68e8d06..5b11ddb22d3a 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -7,6 +7,7 @@
> * Copyright © 2024-2025 Microsoft Corporation
> */
>
> +#include "ruleset.h"
> #include <kunit/test.h>
> #include <linux/bitops.h>
> #include <linux/bits.h>
> @@ -182,32 +183,36 @@ static void test_get_layer_deny_mask(struct kunit *const test)
> deny_masks_t
> landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
> const access_mask_t optional_access,
> - const layer_mask_t (*const layer_masks)[],
> - const size_t layer_masks_size)
> + const struct layer_access_masks *const masks)
> {
> const unsigned long access_opt = optional_access;
> unsigned long access_bit;
> + access_mask_t all_denied = 0;
> deny_masks_t deny_masks = 0;
>
> /* This may require change with new object types. */
> - WARN_ON_ONCE(access_opt !=
> - (optional_access & all_existing_optional_access));
> + WARN_ON_ONCE(!access_mask_subset(optional_access,
> + all_existing_optional_access));
>
> - if (WARN_ON_ONCE(!layer_masks))
> + if (WARN_ON_ONCE(!masks))
> return 0;
>
> if (WARN_ON_ONCE(!access_opt))
> return 0;
>
> - for_each_set_bit(access_bit, &access_opt, layer_masks_size) {
> - const layer_mask_t mask = (*layer_masks)[access_bit];
> + for (int i = ARRAY_SIZE(masks->access) - 1; i >= 0; i--) {
size_t i
> + const access_mask_t denied = masks->access[i] & optional_access;
> + const unsigned long newly_denied = denied & ~all_denied;
>
> - if (!mask)
> + if (!newly_denied)
> continue;
>
> - /* __fls(1) == 0 */
> - deny_masks |= get_layer_deny_mask(all_existing_optional_access,
> - access_bit, __fls(mask));
> + for_each_set_bit(access_bit, &newly_denied,
> + 8 * sizeof(access_mask_t)) {
> + deny_masks |= get_layer_deny_mask(
> + all_existing_optional_access, access_bit, i);
> + }
> + all_denied |= denied;
> }
> return deny_masks;
> }
> @@ -216,28 +221,28 @@ landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
>
> static void test_landlock_get_deny_masks(struct kunit *const test)
> {
> - const layer_mask_t layers1[BITS_PER_TYPE(access_mask_t)] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
> - BIT_ULL(9),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = BIT_ULL(1),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = BIT_ULL(2) |
> - BIT_ULL(0),
> + const struct layer_access_masks layers1 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + .access[1] = LANDLOCK_ACCESS_FS_TRUNCATE,
> + .access[2] = LANDLOCK_ACCESS_FS_IOCTL_DEV,
> + .access[9] = LANDLOCK_ACCESS_FS_EXECUTE,
> };
>
> KUNIT_EXPECT_EQ(test, 0x1,
> landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
> LANDLOCK_ACCESS_FS_TRUNCATE,
> - &layers1, ARRAY_SIZE(layers1)));
> + &layers1));
> KUNIT_EXPECT_EQ(test, 0x20,
> landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
> LANDLOCK_ACCESS_FS_IOCTL_DEV,
> - &layers1, ARRAY_SIZE(layers1)));
> + &layers1));
> KUNIT_EXPECT_EQ(
> test, 0x21,
> landlock_get_deny_masks(_LANDLOCK_ACCESS_FS_OPTIONAL,
> LANDLOCK_ACCESS_FS_TRUNCATE |
> LANDLOCK_ACCESS_FS_IOCTL_DEV,
> - &layers1, ARRAY_SIZE(layers1)));
> + &layers1));
> }
>
> #endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
> diff --git a/security/landlock/domain.h b/security/landlock/domain.h
> index 621f054c9a2b..227066d667f7 100644
> --- a/security/landlock/domain.h
> +++ b/security/landlock/domain.h
> @@ -10,6 +10,7 @@
> #ifndef _SECURITY_LANDLOCK_DOMAIN_H
> #define _SECURITY_LANDLOCK_DOMAIN_H
>
> +#include "ruleset.h"
> #include <linux/limits.h>
> #include <linux/mm.h>
> #include <linux/path.h>
> @@ -122,8 +123,7 @@ struct landlock_hierarchy {
> deny_masks_t
> landlock_get_deny_masks(const access_mask_t all_existing_optional_access,
> const access_mask_t optional_access,
> - const layer_mask_t (*const layer_masks)[],
> - size_t layer_masks_size);
> + const struct layer_access_masks *const masks);
>
> int landlock_init_hierarchy_log(struct landlock_hierarchy *const hierarchy);
>
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index bf8e37fcc7c0..cef0013c2cf6 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -398,57 +398,55 @@ static const struct access_masks any_fs = {
> .fs = ~0,
> };
>
> +/*
> + * Returns true iff the child file with the given src_child access rights under
> + * src_parent would result in having the same or fewer access rights if it were
> + * moved under new_parent.
> + */
> +static bool may_refer(const struct layer_access_masks *const src_parent,
> + const struct layer_access_masks *const src_child,
> + const struct layer_access_masks *const new_parent,
> + const bool child_is_dir)
> +{
> + for (size_t i = 0; i < ARRAY_SIZE(new_parent->access); i++) {
> + access_mask_t child_access = src_parent->access[i] &
> + src_child->access[i];
> + access_mask_t parent_access = new_parent->access[i];
> +
> + if (!child_is_dir) {
> + child_access &= ACCESS_FILE;
> + parent_access &= ACCESS_FILE;
> + }
> +
> + if (!access_mask_subset(child_access, parent_access))
> + return false;
> + }
> + return true;
> +}
> +
> /*
> * Check that a destination file hierarchy has more restrictions than a source
> * file hierarchy. This is only used for link and rename actions.
> *
> - * @layer_masks_child2: Optional child masks.
> + * Returns: true if child1 may be moved from parent1 to parent2 without
> + * increasing its access rights. If child2 is set, an additional condition is
> + * that child2 may be used from parent2 to parent1 without increasing its access
> + * rights.
> */
> -static bool no_more_access(
> - const layer_mask_t (*const layer_masks_parent1)[LANDLOCK_NUM_ACCESS_FS],
> - const layer_mask_t (*const layer_masks_child1)[LANDLOCK_NUM_ACCESS_FS],
> - const bool child1_is_directory,
> - const layer_mask_t (*const layer_masks_parent2)[LANDLOCK_NUM_ACCESS_FS],
> - const layer_mask_t (*const layer_masks_child2)[LANDLOCK_NUM_ACCESS_FS],
> - const bool child2_is_directory)
> +static bool no_more_access(const struct layer_access_masks *const parent1,
> + const struct layer_access_masks *const child1,
> + const bool child1_is_dir,
> + const struct layer_access_masks *const parent2,
> + const struct layer_access_masks *const child2,
> + const bool child2_is_dir)
> {
> - unsigned long access_bit;
> + if (!may_refer(parent1, child1, parent2, child1_is_dir))
> + return false;
>
> - for (access_bit = 0; access_bit < ARRAY_SIZE(*layer_masks_parent2);
> - access_bit++) {
> - /* Ignores accesses that only make sense for directories. */
> - const bool is_file_access =
> - !!(BIT_ULL(access_bit) & ACCESS_FILE);
> + if (!child2)
> + return true;
>
> - if (child1_is_directory || is_file_access) {
> - /*
> - * Checks if the destination restrictions are a
> - * superset of the source ones (i.e. inherited access
> - * rights without child exceptions):
> - * restrictions(parent2) >= restrictions(child1)
> - */
> - if ((((*layer_masks_parent1)[access_bit] &
> - (*layer_masks_child1)[access_bit]) |
> - (*layer_masks_parent2)[access_bit]) !=
> - (*layer_masks_parent2)[access_bit])
> - return false;
> - }
> -
> - if (!layer_masks_child2)
> - continue;
> - if (child2_is_directory || is_file_access) {
> - /*
> - * Checks inverted restrictions for RENAME_EXCHANGE:
> - * restrictions(parent1) >= restrictions(child2)
> - */
> - if ((((*layer_masks_parent2)[access_bit] &
> - (*layer_masks_child2)[access_bit]) |
> - (*layer_masks_parent1)[access_bit]) !=
> - (*layer_masks_parent1)[access_bit])
> - return false;
> - }
> - }
> - return true;
> + return may_refer(parent2, child2, parent1, child2_is_dir);
> }
>
> #define NMA_TRUE(...) KUNIT_EXPECT_TRUE(test, no_more_access(__VA_ARGS__))
> @@ -458,25 +456,25 @@ static bool no_more_access(
>
> static void test_no_more_access(struct kunit *const test)
> {
> - const layer_mask_t rx0[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_READ_FILE)] = BIT_ULL(0),
> + const struct layer_access_masks rx0 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_READ_FILE,
> };
> - const layer_mask_t mx0[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_REG)] = BIT_ULL(0),
> + const struct layer_access_masks mx0 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_MAKE_REG,
> };
> - const layer_mask_t x0[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0),
> + const struct layer_access_masks x0 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
> };
> - const layer_mask_t x1[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(1),
> + const struct layer_access_masks x1 = {
> + .access[1] = LANDLOCK_ACCESS_FS_EXECUTE,
> };
> - const layer_mask_t x01[LANDLOCK_NUM_ACCESS_FS] = {
> - [BIT_INDEX(LANDLOCK_ACCESS_FS_EXECUTE)] = BIT_ULL(0) |
> - BIT_ULL(1),
> + const struct layer_access_masks x01 = {
> + .access[0] = LANDLOCK_ACCESS_FS_EXECUTE,
> + .access[1] = LANDLOCK_ACCESS_FS_EXECUTE,
> };
> - const layer_mask_t allows_all[LANDLOCK_NUM_ACCESS_FS] = {};
> + const struct layer_access_masks allows_all = {};
>
> /* Checks without restriction. */
> NMA_TRUE(&x0, &allows_all, false, &allows_all, NULL, false);
> @@ -564,31 +562,30 @@ static void test_no_more_access(struct kunit *const test)
> #undef NMA_TRUE
> #undef NMA_FALSE
>
> -static bool is_layer_masks_allowed(
> - layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> +static bool is_layer_masks_allowed(const struct layer_access_masks *masks)
> {
> - return !memchr_inv(layer_masks, 0, sizeof(*layer_masks));
> + return !memchr_inv(&masks->access, 0, sizeof(masks->access));
> }
>
> /*
> - * Removes @layer_masks accesses that are not requested.
> + * Removes @masks accesses that are not requested.
> *
> * Returns true if the request is allowed, false otherwise.
> */
> -static bool
> -scope_to_request(const access_mask_t access_request,
> - layer_mask_t (*const layer_masks)[LANDLOCK_NUM_ACCESS_FS])
> +static bool scope_to_request(const access_mask_t access_request,
> + struct layer_access_masks *masks)
> {
> - const unsigned long access_req = access_request;
> - unsigned long access_bit;
> + bool saw_unfulfilled_access = false;
>
> - if (WARN_ON_ONCE(!layer_masks))
> + if (WARN_ON_ONCE(!masks))
> return true;
>
> - for_each_clear_bit(access_bit, &access_req, ARRAY_SIZE(*layer_masks))
> - (*layer_masks)[access_bit] = 0;
> -
> - return is_layer_masks_allowed(layer_masks);
> + for (size_t i = 0; i < ARRAY_SIZE(masks->access); i++) {
> + masks->access[i] &= access_request;
> + if (masks->access[i])
{
> + saw_unfulfilled_access = true;
break;
}
> + }
> + return !saw_unfulfilled_access;
> }
>
> #ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
^ permalink raw reply
* Re: [RFC PATCH 2/2] landlock: transpose the layer masks data structure
From: Mickaël Salaün @ 2026-01-28 21:49 UTC (permalink / raw)
To: Günther Noack, Tingmao Wang
Cc: linux-security-module, Justin Suess, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze
In-Reply-To: <20260123.13e99fee0197@gnoack.org>
Re-CCing everyone since I guess they were accidentally removed.
On Fri, Jan 23, 2026 at 10:33:42PM +0100, Günther Noack wrote:
> Hello!
>
> On Wed, Jan 21, 2026 at 11:22:28PM +0100, Mickaël Salaün wrote:
> > The goal of the initial design was to minimize the amount of memory wrt
> > the number of different access rights because the maximum number of
> > layers is 16 whereas access rights could grow up to 64.
> >
> > Transposing the matrix increases the memory footprint in theory but
> > because we still need the struct layer_access_masks matrix, it should
> > actually be better. See stack usage delta with audit (generated with
> > check-linux.sh):
>
> Thanks for the review and for doing these measurements!
>
> > landlock_unmask_layers 208 80 -128
> > landlock_init_layer_masks 192 96 -96
> > landlock_log_denial 176 80 -96
> > current_check_access_path 336 304 -32
> > current_check_refer_path 592 560 -32
> > hook_file_open 352 320 -32
> > hook_file_send_sigiotask 176 160 -16
> > hook_file_truncate 112 96 -16
> > hook_move_mount 128 112 -16
> > hook_ptrace_access_check 192 176 -16
> > hook_ptrace_traceme 160 144 -16
> > hook_sb_mount 128 112 -16
> > hook_sb_pivotroot 128 112 -16
> > hook_sb_remount 128 112 -16
> > hook_sb_umount 128 112 -16
> > hook_task_kill 176 160 -16
> > current_check_access_socket 336 352 +16
> > is_access_to_paths_allowed 384 400 +16
> >
> > ...and stack usage delta without audit:
> >
> > landlock_unmask_layers 208 80 -128
> > landlock_init_layer_masks 192 96 -96
> > hook_file_open 208 192 -16
> > current_check_access_socket 176 208 +32
>
> These stack usage measurement look as expected. With the current set
> of 16 FS access rights, the matrix for the FS case is 16x16 bits (32
> bytes), both in the old code and after the refactoring.
>
> The differences we see above are therefore not savings in the data
> structure itself, but due to the code simplifications in surrounding
> functions where we now need fewer function parameters and local
> variables.
>
> > However, when we'll add the next access right, access_mask_t will be u32
> > instead of u16, and stack usage delta will increase:
> >
> > current_check_access_socket 352 384 +32
> > hook_file_open 320 352 +32
> > current_check_access_path 304 352 +48
> > current_check_refer_path 560 608 +48
> > is_access_to_paths_allowed 400 464 +64
>
> The data structure grows by 32 bytes, and access_mask_t grows by 2
> bytes (but is as local variable probably aligned to 64 bit boundary).
> These functions all have the layer masks as local variables, so it is
> expected that they grow.
Yes, but we should keep in mind that this grows for all type of access
rights: even if there is no new network access right, the cost will now
be the same for FS and network access checks, which was not the case
before (on purpose).
>
> >
> > Even if the cumulative stack usage delta seems reasonable, the commit
> > message should talk about these drawbacks.
> >
> > I think the improved compiled code, and the overall simplification are
> > worth it.
>
> You are right, the 17th access right will change it somewhat and I
> should add something to the commit message to explain my reasoning why
> this is OK.
>
> Abstractly speaking, we have a matrix with 1 bit per layer and access
> right. I illustrate below how this looks, it feels a bit simpler than
> explaining it in words. (I am marking the access right / layer
> combinations with an "x" which are both an actual FS access right and
> which are in the first layer - assuming that this is our most common
> case.)
>
> In the old way of representing that matrix, we are using a fixed 16
> bits to represent the set of layers where an access right is still
> needed (wasting 15/16 bits in the common case where only one layer is
> active), but we store the right number of access rights. Because this
> representation was not using access_mask_t, the functions using it
> required more loops and conditionals.
>
> layers
> fedcba98 76543210
> access0 ________ _______x
> access1 ________ _______x
> access2 ________ _______x
> access3 ________ _______x
> access4 ________ _______x
> access5 ________ _______x
> access6 ________ _______x
> access7 ________ _______x
> access8 ________ _______x
> access9 ________ _______x
> accessa ________ _______x
> accessb ________ _______x
> accessc ________ _______x
> accessd ________ _______x
> accesse ________ _______x
> accessf ________ _______x
>
> In the new way of representing that matrix, we are using a fixed 16
> (soon 32) bits for the access rights (wasting 0 (soon 15) bits for FS
> access rights). We also use a fixed number of layers (but this now
> becomes tractable to change). We get simplified code and improved
> performance.
>
> access rights
> fedcba98 76543210
> layer0 xxxxxxxx xxxxxxxx
> layer1 ________ ________
> layer2 ________ ________
> layer3 ________ ________
> layer4 ________ ________
> layer5 ________ ________
> layer6 ________ ________
> layer7 ________ ________
> layer8 ________ ________
> layer9 ________ ________
> layera ________ ________
> layerb ________ ________
> layerc ________ ________
> layerd ________ ________
> layere ________ ________
> layerf ________ ________
>
> Once we introduce the 17th FS access right, the matrix will use a
> total of 64 instead of 32 bytes and look like this:
>
> access rights
> 11111111 11111111 00000000 00000000
> fedcba98 76543210 fedcba98 76543210
> layer0 ________ _______x xxxxxxxx xxxxxxxx
> layer1 ________ ________ ________ ________
> layer2 ________ ________ ________ ________
> layer3 ________ ________ ________ ________
> layer4 ________ ________ ________ ________
> layer5 ________ ________ ________ ________
> layer6 ________ ________ ________ ________
> layer7 ________ ________ ________ ________
> layer8 ________ ________ ________ ________
> layer9 ________ ________ ________ ________
> layera ________ ________ ________ ________
> layerb ________ ________ ________ ________
> layerc ________ ________ ________ ________
> layerd ________ ________ ________ ________
> layere ________ ________ ________ ________
> layerf ________ ________ ________ ________
>
>
> In my mind, it is a tradeoff where we get slightly better performance
> and simpler code at the cost of a slightly increased stack space. But
> in my understanding, as long as we stay below the stack size limit,
> this should be acceptable. (Also, this is only 64 bytes extra, the
> risk of exeeding stack space because of it seems low.)
>
>
> (P.S. A thing that we can try as a follow-up is:
>
> Now that this is an array indexed by layer, it becomes easier to only
> look at the layers that are actually active. In the common case,
> that means that we would be able to skip looking at these layers,
> and we might be able to save a few memory accesses.
>
> Although, I am not sure it will improve performance - we should
> better measure the performance impact carefully; the matrix still fits
> in two cache lines. It might not make a big difference how many of
> the adjacent layers we access, and it could be offset by having to
> pass the number of layers around and by loop unrolling tricks that the
> compiler can't use any more.)
>
>
> > On Tue, Dec 30, 2025 at 11:39:21AM +0100, Günther Noack wrote:
> > > The layer masks data structure tracks the requested but unfulfilled
> > > access rights during an operations security check. It stores one bit
> >
> > operation?
>
> I missed the apostrophe: "operation's"
>
>
> > > for each combination of access right and layer index. If the bit is
> > > set, that access right is not granted (yet) in the given layer and we
> > > have to traverse the path further upwards to grant it.
>
>
> > > static size_t get_denied_layer(const struct landlock_ruleset *const domain,
> > > access_mask_t *const access_request,
> > > - const layer_mask_t (*const layer_masks)[],
> > > - const size_t layer_masks_size)
> > > + const struct layer_access_masks *masks)
> > > {
> > > - const unsigned long access_req = *access_request;
> > > - unsigned long access_bit;
> > > - access_mask_t missing = 0;
> > > - long youngest_layer = -1;
> > > -
> > > - for_each_set_bit(access_bit, &access_req, layer_masks_size) {
> > > - const access_mask_t mask = (*layer_masks)[access_bit];
> > > - long layer;
> > > -
> > > - if (!mask)
> > > - continue;
> > > -
> > > - /* __fls(1) == 0 */
> > > - layer = __fls(mask);
> > > - if (layer > youngest_layer) {
> > > - youngest_layer = layer;
> > > - missing = BIT(access_bit);
> > > - } else if (layer == youngest_layer) {
> > > - missing |= BIT(access_bit);
> > > + for (int i = LANDLOCK_MAX_NUM_LAYERS - 1; i >= 0; i--) {
> >
> > All the loop indexes should be size_t (same as before).
> >
> > Instead of LANDLOCK_MAX_NUM_LAYERS, ARRAY_SIZE(masks->access) would be
> > better.
>
> Done, will be fixed in next patch set version.
>
> (I have also fixed all the other places where I could have used size_t
> and ARRAY_SIZE(). Omitting them from this email for brevity, but I
> looked at them all.)
>
> (Remark on the side, I like being able to define the loop variable in
> the for loop, but I notice that we have not done that much so far. I
> assume this is OK?)
I think it is (was?) not in the guideline, but as long as checkpatch.pl
doesn't complain, it looks good to me. I guess it's officially OK since
C11.
>
> > > + if (masks->access[i] & *access_request) {
> > > + *access_request &= masks->access[i];
> > > + return i;
>
>
> > > -static size_t
> > > -get_layer_from_deny_masks(access_mask_t *const access_request,
> > > - const access_mask_t all_existing_optional_access,
> > > - const deny_masks_t deny_masks)
> > > +/*
> > > + * get_layer_from_fs_deny_masks - get the layer which denied the access request
> > > + *
> > > + * As a side effect, stores the denied access rights from that layer(!) in
> > > + * *access_request.
> > > + */
> > > +static size_t get_layer_from_fs_deny_masks(access_mask_t *const access_request,
> > > + const deny_masks_t deny_masks)
> >
> > I'm not a fan of this change. We come from a generic approach to a
> > specific and hardcoded one. This is simpler *for now*, but could we get
> > a better implementation?
> >
> > Anyway, please create at least a dedicated patch for the
> > non-transposition changes.
>
> OK, I'll have a look into decoupling these aspects of the change.
> (Not coded yet, but it's doable and would be cleaner for the patch
> organization.)
>
>
> > > {
> > > - const unsigned long access_opt = all_existing_optional_access;
> > > - const unsigned long access_req = *access_request;
> > > - access_mask_t missing = 0;
> > > + const access_mask_t access_req = *access_request;
> > > size_t youngest_layer = 0;
> > > - size_t access_index = 0;
> > > - unsigned long access_bit;
> > > + access_mask_t missing = 0;
> > >
> > > - /* This will require change with new object types. */
> > > - WARN_ON_ONCE(access_opt != _LANDLOCK_ACCESS_FS_OPTIONAL);
> > > + WARN_ON_ONCE((access_req | _LANDLOCK_ACCESS_FS_OPTIONAL) !=
> > > + _LANDLOCK_ACCESS_FS_OPTIONAL);
> > >
> > > - for_each_set_bit(access_bit, &access_opt,
^ permalink raw reply
* Re: [PATCH v2 2/3] landlock: access_mask_subset() helper
From: Günther Noack @ 2026-01-28 21:38 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Randy Dunlap
In-Reply-To: <20260128.raiD8oseH2ee@digikod.net>
On Wed, Jan 28, 2026 at 10:31:52PM +0100, Mickaël Salaün wrote:
> On Sun, Jan 25, 2026 at 08:58:52PM +0100, Günther Noack wrote:
> > --- a/security/landlock/access.h
> > +++ b/security/landlock/access.h
> > @@ -97,4 +97,10 @@ landlock_upgrade_handled_access_masks(struct access_masks access_masks)
> > return access_masks;
> > }
> >
> > +/** access_mask_subset - true iff a has a subset of the bits of b. */
> > +static inline bool access_mask_subset(access_mask_t a, access_mask_t b)
>
> What about renaming "a" to "subset" and "b" to "superset"?
Sure, sounds reasonable. Will be done in V3.
> > +{
> > + return (a | b) == b;
> > +}
–Günther
^ permalink raw reply
* Re: [PATCH v2 2/3] landlock: access_mask_subset() helper
From: Mickaël Salaün @ 2026-01-28 21:31 UTC (permalink / raw)
To: Günther Noack
Cc: linux-security-module, Tingmao Wang, Justin Suess,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Randy Dunlap
In-Reply-To: <20260125195853.109967-3-gnoack3000@gmail.com>
On Sun, Jan 25, 2026 at 08:58:52PM +0100, Günther Noack wrote:
> This helper function checks whether an access_mask_t has a subset of the
> bits enabled than another one. This expresses the intent a bit smoother
> in the code and does not cost us anything when it gets inlined.
>
> Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> ---
> security/landlock/access.h | 6 ++++++
> security/landlock/fs.c | 2 +-
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/security/landlock/access.h b/security/landlock/access.h
> index 7961c6630a2d..5c0caef9eaf6 100644
> --- a/security/landlock/access.h
> +++ b/security/landlock/access.h
> @@ -97,4 +97,10 @@ landlock_upgrade_handled_access_masks(struct access_masks access_masks)
> return access_masks;
> }
>
> +/** access_mask_subset - true iff a has a subset of the bits of b. */
> +static inline bool access_mask_subset(access_mask_t a, access_mask_t b)
What about renaming "a" to "subset" and "b" to "superset"?
> +{
> + return (a | b) == b;
> +}
> +
> #endif /* _SECURITY_LANDLOCK_ACCESS_H */
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 8205673c8b1c..bf8e37fcc7c0 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1704,7 +1704,7 @@ static int hook_file_open(struct file *const file)
> ARRAY_SIZE(layer_masks));
> #endif /* CONFIG_AUDIT */
>
> - if ((open_access_request & allowed_access) == open_access_request)
> + if (access_mask_subset(open_access_request, allowed_access))
> return 0;
>
> /* Sets access to reflect the actual request. */
> --
> 2.52.0
>
>
^ permalink raw reply
* Re: [RFC][PATCH v2] ima: Add support for staging measurements for deletion and trimming
From: steven chen @ 2026-01-28 21:30 UTC (permalink / raw)
To: Roberto Sassu, corbet, zohar, dmitry.kasatkin, eric.snowberg,
paul, jmorris, serge
Cc: linux-doc, linux-kernel, linux-integrity, linux-security-module,
gregorylumen, nramas, Roberto Sassu, steven chen
In-Reply-To: <20251212171932.316676-1-roberto.sassu@huaweicloud.com>
On 12/12/2025 9:19 AM, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduce the ability of staging the entire (or a portion of the) IMA
> measurement list for deletion. Staging means moving the current content of
> the measurement list to a separate location, and allowing users to read and
> delete it. This causes the measurement list to be atomically truncated
> before new measurements can be added. Staging can be done only once at a
> time. In the event of kexec(), staging is reverted and staged entries will
> be carried over to the new kernel.
>
> User space is responsible to concatenate the staged IMA measurements list
> portions following the temporal order in which the operations were done,
> together with the current measurement list. Then, it can send the collected
> data to the remote verifiers.
>
> Also introduce the ability of trimming N measurements entries from the IMA
> measurements list, provided that user space has already read them. Trimming
> combines staging and deletion in one operation.
>
> The benefit of these solutions is the ability to free precious kernel
> memory, in exchange of delegating user space to reconstruct the full
> measurement list from the chunks. No trust needs to be given to user space,
> since the integrity of the measurement list is protected by the TPM.
>
> By default, staging/trimming the measurements list does not alter the hash
> table. When staging/trimming are done, IMA is still able to detect
> collisions on the staged and later deleted measurement entries, by keeping
> the entry digests (only template data are freed).
>
> However, since during the measurements list serialization only the SHA1
> digest is passed, and since there are no template data to recalculate the
> other digests from, the hash table is currently not populated with digests
> from staged/deleted entries after kexec().
>
> Introduce the new kernel option ima_flush_htable to decide whether or not
> the digests of staged measurement entries are flushed from the hash table.
>
> Then, introduce ascii_runtime_measurements_staged_<algo> and
> binary_runtime_measurement_staged_<algo> interfaces to stage/trim/delete
> the measurements. Use 'echo A > <IMA interface>' and
> 'echo D > <IMA interface>' to respectively stage and delete the entire
> measurements list. Use 'echo N > <IMA interface>', with N between 1 and
> LONG_MAX, to stage the selected portion of the measurements list, and
> 'echo -N > <IMA interface>' to trim N measurements entries.
>
> The ima_measure_users counter (protected by the ima_measure_lock mutex) has
> been introduced to protect access to the measurements list and the staged
> part. The open method of all the measurement interfaces has been extended
> to allow only one writer at a time or, in alternative, multiple readers.
> The write permission is used to stage/trim/delete the measurements, the
> read permission to read them. Write requires also the CAP_SYS_ADMIN
> capability.
>
> Finally, introduce and maintain dedicate counters for the number of
> measurement entries and binary size, for the current measurements list
> (BINARY_SIZE), for the current measurements list plus staged entries
> (BINARY_SIZE_STAGED) useful for kexec() segment allocation, and for the
> entire measurement list without staging/trimming (BINARY_SIZE_FULL) useful
> for the kexec-related critical data records.
Is the following possible race condition for staged list:
Agent A: create staged list Staged list A1
new measurement added Measurement list M1
Two lists in kernel: A1 and M1
Agent B: read staged list (A1) to do verification
new measurement added Measurement list M2
Two lists in kernel: A1 and M2
Agent A: verified and remove staged list (A1)
new measurement added Measurement list M3
One list in kernel: M3
Agent C: create staged list Staged list C1
new measurement added Measurement list M4
Two lists in kernel: C1 and M4
Agent B: remove staged list (?), C1 removed ---this will cause problem
new measurement added Measurement list M5
One list in kernel: M5
Agent C: try to remove staged list(?)
Possible solution?
Save the total number trimmed T or tag
Trim request sync this parameter to trim the staged list
Regards,
Steven
> Note: This code derives from the Alt-IMA Huawei project, and is being
> released under the dual license model (GPL-2.0 OR MIT).
>
> Link: https://github.com/linux-integrity/linux/issues/1
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
> .../admin-guide/kernel-parameters.txt | 4 +
> security/integrity/ima/ima.h | 18 +-
> security/integrity/ima/ima_fs.c | 240 +++++++++++++++++-
> security/integrity/ima/ima_kexec.c | 42 ++-
> security/integrity/ima/ima_queue.c | 169 +++++++++++-
> 5 files changed, 439 insertions(+), 34 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 6c42061ca20e..e5f1e11bd0a2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2215,6 +2215,10 @@
> Use the canonical format for the binary runtime
> measurements, instead of host native format.
>
> + ima_flush_htable [IMA]
> + Flush the IMA hash table when staging for deletion or
> + trimming measurement entries.
> +
> ima_hash= [IMA]
> Format: { md5 | sha1 | rmd160 | sha256 | sha384
> | sha512 | ... }
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index e3d71d8d56e3..8a6be4284210 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -28,6 +28,15 @@ enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN,
> IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
> enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
>
> +/*
> + * BINARY_SIZE: size of the current measurements list
> + * BINARY_SIZE_STAGED: size of current measurements list + staged entries
> + * BINARY_SIZE_FULL: size of measurements list since IMA initialization
> + */
> +enum binary_size_types {
> + BINARY_SIZE, BINARY_SIZE_STAGED, BINARY_SIZE_FULL, BINARY__LAST
> +};
> +
> /* digest size for IMA, fits SHA1 or MD5 */
> #define IMA_DIGEST_SIZE SHA1_DIGEST_SIZE
> #define IMA_EVENT_NAME_LEN_MAX 255
> @@ -117,6 +126,8 @@ struct ima_queue_entry {
> struct ima_template_entry *entry;
> };
> extern struct list_head ima_measurements; /* list of all measurements */
> +extern struct list_head ima_measurements_staged; /* list of staged meas. */
> +extern bool ima_measurements_staged_exist; /* If there are staged meas. */
>
> /* Some details preceding the binary serialized measurement list */
> struct ima_kexec_hdr {
> @@ -281,10 +292,12 @@ struct ima_template_desc *ima_template_desc_current(void);
> struct ima_template_desc *ima_template_desc_buf(void);
> struct ima_template_desc *lookup_template_desc(const char *name);
> bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
> +int ima_queue_stage_trim(unsigned long req_value, bool trim);
> +int ima_queue_delete_staged_trimmed(bool staged_moved);
> int ima_restore_measurement_entry(struct ima_template_entry *entry);
> int ima_restore_measurement_list(loff_t bufsize, void *buf);
> int ima_measurements_show(struct seq_file *m, void *v);
> -unsigned long ima_get_binary_runtime_size(void);
> +unsigned long ima_get_binary_runtime_size(enum binary_size_types type);
> int ima_init_template(void);
> void ima_init_template_list(void);
> int __init ima_init_digests(void);
> @@ -298,11 +311,12 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
> extern spinlock_t ima_queue_lock;
>
> struct ima_h_table {
> - atomic_long_t len; /* number of stored measurements in the list */
> + atomic_long_t len[BINARY__LAST]; /* num of stored meas. in the list */
> atomic_long_t violations;
> struct hlist_head queue[IMA_MEASURE_HTABLE_SIZE];
> };
> extern struct ima_h_table ima_htable;
> +extern struct mutex ima_extend_list_mutex;
>
> static inline unsigned int ima_hash_key(u8 *digest)
> {
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 87045b09f120..a96f7c36b34a 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -24,7 +24,18 @@
>
> #include "ima.h"
>
> +/*
> + * Requests:
> + * 'A\n': stage the entire measurements list
> + * '[1, LONG_MAX]\n' stage N measurements entries
> + * '-[1, LONG_MAX]\n' trim N measurements entries
> + * 'D\n': delete staged measurements
> + */
> +#define STAGED_REQ_LENGTH 21
> +
> static DEFINE_MUTEX(ima_write_mutex);
> +static DEFINE_MUTEX(ima_measure_lock);
> +static long ima_measure_users;
>
> bool ima_canonical_fmt;
> static int __init default_canonical_fmt_setup(char *str)
> @@ -64,7 +75,8 @@ static ssize_t ima_show_measurements_count(struct file *filp,
> char __user *buf,
> size_t count, loff_t *ppos)
> {
> - return ima_show_htable_value(buf, count, ppos, &ima_htable.len);
> + return ima_show_htable_value(buf, count, ppos,
> + &ima_htable.len[BINARY_SIZE]);
>
> }
>
> @@ -74,14 +86,15 @@ static const struct file_operations ima_measurements_count_ops = {
> };
>
> /* returns pointer to hlist_node */
> -static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
> +static void *_ima_measurements_start(struct seq_file *m, loff_t *pos,
> + struct list_head *head)
> {
> loff_t l = *pos;
> struct ima_queue_entry *qe;
>
> /* we need a lock since pos could point beyond last element */
> rcu_read_lock();
> - list_for_each_entry_rcu(qe, &ima_measurements, later) {
> + list_for_each_entry_rcu(qe, head, later) {
> if (!l--) {
> rcu_read_unlock();
> return qe;
> @@ -91,7 +104,18 @@ static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
> return NULL;
> }
>
> -static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
> +static void *ima_measurements_start(struct seq_file *m, loff_t *pos)
> +{
> + return _ima_measurements_start(m, pos, &ima_measurements);
> +}
> +
> +static void *ima_measurements_staged_start(struct seq_file *m, loff_t *pos)
> +{
> + return _ima_measurements_start(m, pos, &ima_measurements_staged);
> +}
> +
> +static void *_ima_measurements_next(struct seq_file *m, void *v, loff_t *pos,
> + struct list_head *head)
> {
> struct ima_queue_entry *qe = v;
>
> @@ -103,7 +127,18 @@ static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
> rcu_read_unlock();
> (*pos)++;
>
> - return (&qe->later == &ima_measurements) ? NULL : qe;
> + return (&qe->later == head) ? NULL : qe;
> +}
> +
> +static void *ima_measurements_next(struct seq_file *m, void *v, loff_t *pos)
> +{
> + return _ima_measurements_next(m, v, pos, &ima_measurements);
> +}
> +
> +static void *ima_measurements_staged_next(struct seq_file *m, void *v,
> + loff_t *pos)
> +{
> + return _ima_measurements_next(m, v, pos, &ima_measurements_staged);
> }
>
> static void ima_measurements_stop(struct seq_file *m, void *v)
> @@ -202,16 +237,147 @@ static const struct seq_operations ima_measurments_seqops = {
> .show = ima_measurements_show
> };
>
> +static int _ima_measurements_open(struct inode *inode, struct file *file,
> + const struct seq_operations *seq_ops)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> + int ret;
> +
> + if (write && !capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + mutex_lock(&ima_measure_lock);
> + if ((write && ima_measure_users != 0) ||
> + (!write && ima_measure_users < 0)) {
> + mutex_unlock(&ima_measure_lock);
> + return -EBUSY;
> + }
> +
> + ret = seq_open(file, seq_ops);
> + if (ret < 0) {
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> + }
> +
> + if (write)
> + ima_measure_users--;
> + else
> + ima_measure_users++;
> +
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> +}
> +
> static int ima_measurements_open(struct inode *inode, struct file *file)
> {
> - return seq_open(file, &ima_measurments_seqops);
> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
> +}
> +
> +static int ima_measurements_release(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> + int ret;
> +
> + mutex_lock(&ima_measure_lock);
> + ret = seq_release(inode, file);
> + if (!ret) {
> + if (write)
> + ima_measure_users++;
> + else
> + ima_measure_users--;
> + }
> +
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> }
>
> static const struct file_operations ima_measurements_ops = {
> .open = ima_measurements_open,
> .read = seq_read,
> .llseek = seq_lseek,
> - .release = seq_release,
> + .release = ima_measurements_release,
> +};
> +
> +static const struct seq_operations ima_measurments_staged_seqops = {
> + .start = ima_measurements_staged_start,
> + .next = ima_measurements_staged_next,
> + .stop = ima_measurements_stop,
> + .show = ima_measurements_show
> +};
> +
> +static int ima_measurements_staged_open(struct inode *inode, struct file *file)
> +{
> + return _ima_measurements_open(inode, file,
> + &ima_measurments_staged_seqops);
> +}
> +
> +static ssize_t ima_measurements_staged_read(struct file *file, char __user *buf,
> + size_t size, loff_t *ppos)
> +{
> + if (!ima_measurements_staged_exist)
> + return -ENOENT;
> +
> + return seq_read(file, buf, size, ppos);
> +}
> +
> +static ssize_t ima_measurements_staged_write(struct file *file,
> + const char __user *buf,
> + size_t datalen, loff_t *ppos)
> +{
> + char req[STAGED_REQ_LENGTH], *req_ptr = req;
> + unsigned long req_value;
> + bool trim = false;
> + int ret;
> +
> + if (*ppos > 0 || datalen < 2 || datalen > STAGED_REQ_LENGTH)
> + return -EINVAL;
> +
> + if (copy_from_user(req, buf, datalen) != 0)
> + return -EFAULT;
> +
> + if (req[datalen - 1] != '\n')
> + return -EINVAL;
> +
> + req[datalen - 1] = '\0';
> + req_ptr = req;
> +
> + switch (req[0]) {
> + case 'A':
> + if (datalen != 2 || req[1] != '\0')
> + return -EINVAL;
> +
> + ret = ima_queue_stage_trim(LONG_MAX, false);
> + break;
> + case 'D':
> + if (datalen != 2 || req[1] != '\0')
> + return -EINVAL;
> +
> + ret = ima_queue_delete_staged_trimmed(false);
> + break;
> + case '-':
> + trim = true;
> + req_ptr++;
> + fallthrough;
> + default:
> + ret = kstrtoul(req_ptr, 0, &req_value);
> + if (ret < 0)
> + return ret;
> +
> + ret = ima_queue_stage_trim(req_value, trim);
> + }
> +
> + if (ret < 0)
> + return ret;
> +
> + return datalen;
> +}
> +
> +static const struct file_operations ima_measurements_staged_ops = {
> + .open = ima_measurements_staged_open,
> + .read = ima_measurements_staged_read,
> + .write = ima_measurements_staged_write,
> + .llseek = seq_lseek,
> + .release = ima_measurements_release,
> };
>
> void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
> @@ -279,14 +445,37 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
>
> static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
> {
> - return seq_open(file, &ima_ascii_measurements_seqops);
> + return _ima_measurements_open(inode, file,
> + &ima_ascii_measurements_seqops);
> }
>
> static const struct file_operations ima_ascii_measurements_ops = {
> .open = ima_ascii_measurements_open,
> .read = seq_read,
> .llseek = seq_lseek,
> - .release = seq_release,
> + .release = ima_measurements_release,
> +};
> +
> +static const struct seq_operations ima_ascii_measurements_staged_seqops = {
> + .start = ima_measurements_staged_start,
> + .next = ima_measurements_staged_next,
> + .stop = ima_measurements_stop,
> + .show = ima_ascii_measurements_show
> +};
> +
> +static int ima_ascii_measurements_staged_open(struct inode *inode,
> + struct file *file)
> +{
> + return _ima_measurements_open(inode, file,
> + &ima_ascii_measurements_staged_seqops);
> +}
> +
> +static const struct file_operations ima_ascii_measurements_staged_ops = {
> + .open = ima_ascii_measurements_staged_open,
> + .read = ima_measurements_staged_read,
> + .write = ima_measurements_staged_write,
> + .llseek = seq_lseek,
> + .release = ima_measurements_release,
> };
>
> static ssize_t ima_read_policy(char *path)
> @@ -419,6 +608,25 @@ static int __init create_securityfs_measurement_lists(void)
> &ima_measurements_ops);
> if (IS_ERR(dentry))
> return PTR_ERR(dentry);
> +
> + sprintf(file_name, "ascii_runtime_measurements_staged_%s",
> + hash_algo_name[algo]);
> + dentry = securityfs_create_file(file_name,
> + S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
> + ima_dir, (void *)(uintptr_t)i,
> + &ima_ascii_measurements_staged_ops);
> + if (IS_ERR(dentry))
> + return PTR_ERR(dentry);
> +
> + sprintf(file_name, "binary_runtime_measurements_staged_%s",
> + hash_algo_name[algo]);
> + dentry = securityfs_create_file(file_name,
> + S_IRUSR | S_IRGRP |
> + S_IWUSR | S_IWGRP,
> + ima_dir, (void *)(uintptr_t)i,
> + &ima_measurements_staged_ops);
> + if (IS_ERR(dentry))
> + return PTR_ERR(dentry);
> }
>
> return 0;
> @@ -528,6 +736,20 @@ int __init ima_fs_init(void)
> goto out;
> }
>
> + dentry = securityfs_create_symlink("binary_runtime_measurements_staged",
> + ima_dir, "binary_runtime_measurements_staged_sha1", NULL);
> + if (IS_ERR(dentry)) {
> + ret = PTR_ERR(dentry);
> + goto out;
> + }
> +
> + dentry = securityfs_create_symlink("ascii_runtime_measurements_staged",
> + ima_dir, "ascii_runtime_measurements_staged_sha1", NULL);
> + if (IS_ERR(dentry)) {
> + ret = PTR_ERR(dentry);
> + goto out;
> + }
> +
> dentry = securityfs_create_file("runtime_measurements_count",
> S_IRUSR | S_IRGRP, ima_dir, NULL,
> &ima_measurements_count_ops);
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 7362f68f2d8b..13c7e78aeefd 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -40,8 +40,8 @@ void ima_measure_kexec_event(const char *event_name)
> long len;
> int n;
>
> - buf_size = ima_get_binary_runtime_size();
> - len = atomic_long_read(&ima_htable.len);
> + buf_size = ima_get_binary_runtime_size(BINARY_SIZE_FULL);
> + len = atomic_long_read(&ima_htable.len[BINARY_SIZE_FULL]);
>
> n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
> "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
> @@ -78,6 +78,17 @@ static int ima_alloc_kexec_file_buf(size_t segment_size)
> return 0;
> }
>
> +static int ima_dump_measurement(struct ima_kexec_hdr *khdr,
> + struct ima_queue_entry *qe)
> +{
> + if (ima_kexec_file.count >= ima_kexec_file.size)
> + return -EINVAL;
> +
> + khdr->count++;
> + ima_measurements_show(&ima_kexec_file, qe);
> + return 0;
> +}
> +
> static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
> unsigned long segment_size)
> {
> @@ -93,17 +104,25 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
>
> memset(&khdr, 0, sizeof(khdr));
> khdr.version = 1;
> - /* This is an append-only list, no need to hold the RCU read lock */
> - list_for_each_entry_rcu(qe, &ima_measurements, later, true) {
> - if (ima_kexec_file.count < ima_kexec_file.size) {
> - khdr.count++;
> - ima_measurements_show(&ima_kexec_file, qe);
> - } else {
> - ret = -EINVAL;
> +
> + /* It can race with ima_queue_stage_trim(). */
> + mutex_lock(&ima_extend_list_mutex);
> +
> + list_for_each_entry(qe, &ima_measurements_staged, later) {
> + ret = ima_dump_measurement(&khdr, qe);
> + if (ret < 0)
> + break;
> + }
> +
> + list_for_each_entry(qe, &ima_measurements, later) {
> + if (!ret)
> + ret = ima_dump_measurement(&khdr, qe);
> + if (ret < 0)
> break;
> - }
> }
>
> + mutex_unlock(&ima_extend_list_mutex);
> +
> /*
> * fill in reserved space with some buffer details
> * (eg. version, buffer size, number of measurements)
> @@ -157,7 +176,8 @@ void ima_add_kexec_buffer(struct kimage *image)
> else
> extra_memory = CONFIG_IMA_KEXEC_EXTRA_MEMORY_KB * 1024;
>
> - binary_runtime_size = ima_get_binary_runtime_size() + extra_memory;
> + binary_runtime_size = ima_get_binary_runtime_size(BINARY_SIZE_STAGED) +
> + extra_memory;
>
> if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
> kexec_segment_size = ULONG_MAX;
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 590637e81ad1..7dfa24b8ae31 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -22,19 +22,32 @@
>
> #define AUDIT_CAUSE_LEN_MAX 32
>
> +bool ima_flush_htable;
> +static int __init ima_flush_htable_setup(char *str)
> +{
> + ima_flush_htable = true;
> + return 1;
> +}
> +__setup("ima_flush_htable", ima_flush_htable_setup);
> +
> /* pre-allocated array of tpm_digest structures to extend a PCR */
> static struct tpm_digest *digests;
>
> LIST_HEAD(ima_measurements); /* list of all measurements */
> +LIST_HEAD(ima_measurements_staged); /* list of staged measurements */
> +static LIST_HEAD(ima_measurements_trim); /* list of measurements to trim */
> +bool ima_measurements_staged_exist; /* If there are staged measurements */
> #ifdef CONFIG_IMA_KEXEC
> -static unsigned long binary_runtime_size;
> +static unsigned long binary_runtime_size[BINARY__LAST];
> #else
> -static unsigned long binary_runtime_size = ULONG_MAX;
> +static unsigned long binary_runtime_size[BINARY_SIZE] = ULONG_MAX;
> +static unsigned long binary_runtime_size[BINARY_SIZE_FULL] = ULONG_MAX;
> +static unsigned long binary_runtime_size[BINARY_SIZE_STAGED] = ULONG_MAX;
> #endif
>
> /* key: inode (before secure-hashing a file) */
> struct ima_h_table ima_htable = {
> - .len = ATOMIC_LONG_INIT(0),
> + .len = { ATOMIC_LONG_INIT(0) },
> .violations = ATOMIC_LONG_INIT(0),
> .queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
> };
> @@ -43,7 +56,7 @@ struct ima_h_table ima_htable = {
> * and extending the TPM PCR aggregate. Since tpm_extend can take
> * long (and the tpm driver uses a mutex), we can't use the spinlock.
> */
> -static DEFINE_MUTEX(ima_extend_list_mutex);
> +DEFINE_MUTEX(ima_extend_list_mutex);
>
> /*
> * Used internally by the kernel to suspend measurements.
> @@ -101,7 +114,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
> bool update_htable)
> {
> struct ima_queue_entry *qe;
> - unsigned int key;
> + unsigned int i, key;
>
> qe = kmalloc(sizeof(*qe), GFP_KERNEL);
> if (qe == NULL) {
> @@ -113,18 +126,23 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
> INIT_LIST_HEAD(&qe->later);
> list_add_tail_rcu(&qe->later, &ima_measurements);
>
> - atomic_long_inc(&ima_htable.len);
> + for (i = 0; i < BINARY__LAST; i++)
> + atomic_long_inc(&ima_htable.len[i]);
> +
> if (update_htable) {
> key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
> hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
> }
>
> - if (binary_runtime_size != ULONG_MAX) {
> + if (binary_runtime_size[BINARY_SIZE_FULL] != ULONG_MAX) {
> int size;
>
> size = get_binary_runtime_size(entry);
> - binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
> - binary_runtime_size + size : ULONG_MAX;
> +
> + for (i = 0; i < BINARY__LAST; i++)
> + binary_runtime_size[i] =
> + (binary_runtime_size[i] < ULONG_MAX - size) ?
> + binary_runtime_size[i] + size : ULONG_MAX;
> }
> return 0;
> }
> @@ -134,12 +152,18 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
> * entire binary_runtime_measurement list, including the ima_kexec_hdr
> * structure.
> */
> -unsigned long ima_get_binary_runtime_size(void)
> +unsigned long ima_get_binary_runtime_size(enum binary_size_types type)
> {
> - if (binary_runtime_size >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
> + unsigned long val;
> +
> + mutex_lock(&ima_extend_list_mutex);
> + val = binary_runtime_size[type];
> + mutex_unlock(&ima_extend_list_mutex);
> +
> + if (val >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
> return ULONG_MAX;
> else
> - return binary_runtime_size + sizeof(struct ima_kexec_hdr);
> + return val + sizeof(struct ima_kexec_hdr);
> }
>
> static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
> @@ -220,6 +244,127 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> return result;
> }
>
> +int ima_queue_stage_trim(unsigned long req_value, bool trim)
> +{
> + unsigned long req_value_copy = req_value, to_remove = 0;
> + struct list_head *moved = &ima_measurements_staged;
> + struct ima_queue_entry *qe;
> +
> + if (req_value == 0 || req_value > LONG_MAX)
> + return -EINVAL;
> +
> + if (ima_measurements_staged_exist)
> + return -EEXIST;
> +
> + if (trim)
> + moved = &ima_measurements_trim;
> +
> + mutex_lock(&ima_extend_list_mutex);
> + if (list_empty(&ima_measurements)) {
> + mutex_unlock(&ima_extend_list_mutex);
> + return -ENOENT;
> + }
> +
> + if (req_value == LONG_MAX) {
> + list_replace(&ima_measurements, moved);
> + INIT_LIST_HEAD(&ima_measurements);
> + atomic_long_set(&ima_htable.len[BINARY_SIZE], 0);
> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> + binary_runtime_size[BINARY_SIZE] = 0;
> +
> + if (trim) {
> + atomic_long_set(&ima_htable.len[BINARY_SIZE_STAGED], 0);
> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> + binary_runtime_size[BINARY_SIZE_STAGED] = 0;
> + }
> + } else {
> + list_for_each_entry(qe, &ima_measurements, later) {
> + to_remove += get_binary_runtime_size(qe->entry);
> + if (--req_value_copy == 0)
> + break;
> + }
> +
> + if (req_value_copy > 0) {
> + mutex_unlock(&ima_extend_list_mutex);
> + return -ENOENT;
> + }
> +
> + __list_cut_position(moved, &ima_measurements, &qe->later);
> + atomic_long_sub(req_value, &ima_htable.len[BINARY_SIZE]);
> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> + binary_runtime_size[BINARY_SIZE] -= to_remove;
> +
> + if (trim) {
> + atomic_long_sub(req_value,
> + &ima_htable.len[BINARY_SIZE_STAGED]);
> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> + binary_runtime_size[BINARY_SIZE_STAGED] -=
> + to_remove;
> + }
> + }
> +
> + if (ima_flush_htable)
> + /* Either staged/trimmed entries are removed from hash table. */
> + list_for_each_entry(qe, moved, later)
> + /* It can race with ima_lookup_digest_entry(). */
> + hlist_del_rcu(&qe->hnext);
> +
> + mutex_unlock(&ima_extend_list_mutex);
> + ima_measurements_staged_exist = true;
> +
> + if (ima_flush_htable)
> + synchronize_rcu();
> +
> + if (trim)
> + return ima_queue_delete_staged_trimmed(true);
> +
> + return 0;
> +}
> +
> +int ima_queue_delete_staged_trimmed(bool staged_moved)
> +{
> + struct ima_queue_entry *qe, *qe_tmp;
> + unsigned int i;
> +
> + if (!ima_measurements_staged_exist)
> + return -ENOENT;
> +
> + if (!staged_moved) {
> + mutex_lock(&ima_extend_list_mutex);
> + list_replace(&ima_measurements_staged, &ima_measurements_trim);
> + INIT_LIST_HEAD(&ima_measurements_staged);
> + atomic_long_set(&ima_htable.len[BINARY_SIZE_STAGED], 0);
> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> + binary_runtime_size[BINARY_SIZE_STAGED] = 0;
> +
> + mutex_unlock(&ima_extend_list_mutex);
> + }
> +
> + list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_trim, later) {
> + /*
> + * Ok because after list delete qe is only accessed by
> + * ima_lookup_digest_entry().
> + */
> + for (i = 0; i < qe->entry->template_desc->num_fields; i++) {
> + kfree(qe->entry->template_data[i].data);
> + qe->entry->template_data[i].data = NULL;
> + qe->entry->template_data[i].len = 0;
> + }
> +
> + list_del(&qe->later);
> +
> + /* No leak if !ima_flush_htable, referenced by ima_htable. */
> + if (ima_flush_htable) {
> + kfree(qe->entry->digests);
> + kfree(qe->entry);
> + kfree(qe);
> + }
> + }
> +
> + ima_measurements_staged_exist = false;
> + return 0;
> +}
> +
> int ima_restore_measurement_entry(struct ima_template_entry *entry)
> {
> int result = 0;
^ permalink raw reply
* Re: [PATCH v2 07/13] mm: update secretmem to use VMA flags on mmap_prepare
From: Lorenzo Stoakes @ 2026-01-28 16:44 UTC (permalink / raw)
To: Andrew Morton
Cc: Jarkko Sakkinen, Dave Hansen, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H . Peter Anvin, Arnd Bergmann,
Greg Kroah-Hartman, Dan Williams, Vishal Verma, Dave Jiang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Christian Koenig, Huang Rui, Matthew Auld,
Matthew Brost, Alexander Viro, Christian Brauner, Jan Kara,
Benjamin LaHaise, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
Sandeep Dhavale, Hongbo Li, Chunhai Guo, Theodore Ts'o,
Andreas Dilger, Muchun Song, Oscar Salvador, David Hildenbrand,
Konstantin Komarov, Mike Marshall, Martin Brandenburg, Tony Luck,
Reinette Chatre, Dave Martin, James Morse, Babu Moger,
Carlos Maiolino, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
Matthew Wilcox, Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Hugh Dickins, Baolin Wang,
Zi Yan, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Jann Horn, Pedro Falcato, David Howells, Paul Moore,
James Morris, Serge E . Hallyn, Yury Norov, Rasmus Villemoes,
linux-sgx, linux-kernel, nvdimm, linux-cxl, dri-devel, intel-gfx,
linux-fsdevel, linux-aio, linux-erofs, linux-ext4, linux-mm,
ntfs3, devel, linux-xfs, keyrings, linux-security-module,
Jason Gunthorpe
In-Reply-To: <a243a09b0a5d0581e963d696de1735f61f5b2075.1769097829.git.lorenzo.stoakes@oracle.com>
Hi Andrew,
Could you apply the below fix-patch to resolve the issue Chris's AI checks
detected, I missed out one caller of mlock_future_ok() (a very human mistake
;)).
Cheers, Lorenzo
----8<----
From 652146b4d93a31bb6f9da9428ddaab8a4a53e170 Mon Sep 17 00:00:00 2001
From: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Date: Wed, 28 Jan 2026 16:41:55 +0000
Subject: [PATCH] fix
Signed-off-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
---
mm/mmap.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mm/mmap.c b/mm/mmap.c
index 354479c95896..5dfe57b6d69a 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -108,7 +108,8 @@ static int check_brk_limits(unsigned long addr, unsigned long len)
if (IS_ERR_VALUE(mapped_addr))
return mapped_addr;
- return mlock_future_ok(current->mm, current->mm->def_flags, len)
+ return mlock_future_ok(current->mm,
+ current->mm->def_flags & VM_LOCKED, len)
? 0 : -EAGAIN;
}
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v2 07/13] mm: update secretmem to use VMA flags on mmap_prepare
From: Lorenzo Stoakes @ 2026-01-28 16:04 UTC (permalink / raw)
To: Chris Mason
Cc: Andrew Morton, Jarkko Sakkinen, Dave Hansen, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H . Peter Anvin, Arnd Bergmann,
Greg Kroah-Hartman, Dan Williams, Vishal Verma, Dave Jiang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Christian Koenig, Huang Rui, Matthew Auld,
Matthew Brost, Alexander Viro, Christian Brauner, Jan Kara,
Benjamin LaHaise, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
Sandeep Dhavale, Hongbo Li, Chunhai Guo, Theodore Ts'o,
Andreas Dilger, Muchun Song, Oscar Salvador, David Hildenbrand,
Konstantin Komarov, Mike Marshall, Martin Brandenburg, Tony Luck,
Reinette Chatre, Dave Martin, James Morse, Babu Moger,
Carlos Maiolino, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
Matthew Wilcox, Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Hugh Dickins, Baolin Wang,
Zi Yan, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Jann Horn, Pedro Falcato, David Howells, Paul Moore,
James Morris, Serge E . Hallyn, Yury Norov, Rasmus Villemoes,
linux-sgx, linux-kernel, nvdimm, linux-cxl, dri-devel, intel-gfx,
linux-fsdevel, linux-aio, linux-erofs, linux-ext4, linux-mm,
ntfs3, devel, linux-xfs, keyrings, linux-security-module,
Jason Gunthorpe
In-Reply-To: <20260128121200.283932-1-clm@meta.com>
On Wed, Jan 28, 2026 at 04:08:36AM -0800, Chris Mason wrote:
> Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> > This patch updates secretmem to use the new vma_flags_t type which will
> > soon supersede vm_flags_t altogether.
> >
> > In order to make this change we also have to update mlock_future_ok(), we
> > replace the vm_flags_t parameter with a simple boolean is_vma_locked one,
> > which also simplifies the invocation here.
> >
> > This is laying the groundwork for eliminating the vm_flags_t in
> > vm_area_desc and more broadly throughout the kernel.
> >
>
> Hi everyone,
>
> Running mm-new through AI review flagged this one:
>
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -225,12 +225,12 @@ static inline unsigned long round_hint_to_min(unsigned long hint)
> > return hint;
> > }
> >
> > -bool mlock_future_ok(const struct mm_struct *mm, vm_flags_t vm_flags,
> > - unsigned long bytes)
> > +bool mlock_future_ok(const struct mm_struct *mm, bool is_vma_locked,
> > + unsigned long bytes)
> > {
> > unsigned long locked_pages, limit_pages;
> >
> > - if (!(vm_flags & VM_LOCKED) || capable(CAP_IPC_LOCK))
> > + if (!is_vma_locked || capable(CAP_IPC_LOCK))
> > return true;
>
> [ ... ]
>
> > @@ -416,7 +416,7 @@
> > if (!can_do_mlock())
> > return -EPERM;
> >
> > - if (!mlock_future_ok(mm, vm_flags, len))
> > + if (!mlock_future_ok(mm, vm_flags & VM_LOCKED, len))
> > return -EAGAIN;
>
> This looks correct - do_mmap() now extracts the VM_LOCKED bit from
> vm_flags and passes that to mlock_future_ok().
>
> However, it appears check_brk_limits() was not updated when
> mlock_future_ok() changed its signature. The function at mm/mmap.c:111
> still passes the raw def_flags value:
>
> return mlock_future_ok(current->mm, current->mm->def_flags, len)
> ? 0 : -EAGAIN;
Ack, the C 'type system' strikes again :) will send a fix-patch.
>
> When def_flags has any bit set, this converts to true, incorrectly
> triggering mlock limit checks even when VM_LOCKED is not set. Should
> this be passing (current->mm->def_flags & VM_LOCKED) instead?
>
> [ ... ]
>
> Additionally, the test stub at tools/testing/vma/vma_internal.h:1627
> still has the old signature (mm, vm_flags_t vm_flags, bytes) while the
> production code now uses (mm, bool is_vma_locked, bytes). This could
> cause compilation issues or mask bugs in the test suite.
Ack, I can fix that later. The VMA test headers have been split and it's too
much merge pain to address at this point given the tests aren't impacted by this
yet. Is on todo!
>
>
Cheers, Lorenzo
^ permalink raw reply
* Re: [PATCH v2 00/13] mm: add bitmap VMA flag helpers and convert all mmap_prepare to use them
From: Pedro Falcato @ 2026-01-28 15:50 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Yury Norov, Andrew Morton, Jarkko Sakkinen, Dave Hansen,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
H . Peter Anvin, Arnd Bergmann, Greg Kroah-Hartman, Dan Williams,
Vishal Verma, Dave Jiang, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, Tvrtko Ursulin, Christian Koenig,
Huang Rui, Matthew Auld, Matthew Brost, Alexander Viro,
Christian Brauner, Jan Kara, Benjamin LaHaise, Gao Xiang, Chao Yu,
Yue Hu, Jeffle Xu, Sandeep Dhavale, Hongbo Li, Chunhai Guo,
Theodore Ts'o, Andreas Dilger, Muchun Song, Oscar Salvador,
David Hildenbrand, Konstantin Komarov, Mike Marshall,
Martin Brandenburg, Tony Luck, Reinette Chatre, Dave Martin,
James Morse, Babu Moger, Carlos Maiolino, Damien Le Moal,
Naohiro Aota, Johannes Thumshirn, Matthew Wilcox,
Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Hugh Dickins, Baolin Wang,
Zi Yan, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Jann Horn, David Howells, Paul Moore, James Morris,
Serge E . Hallyn, Yury Norov, Rasmus Villemoes, linux-sgx,
linux-kernel, nvdimm, linux-cxl, dri-devel, intel-gfx,
linux-fsdevel, linux-aio, linux-erofs, linux-ext4, linux-mm,
ntfs3, devel, linux-xfs, keyrings, linux-security-module,
Jason Gunthorpe
In-Reply-To: <c7452c5f-e42f-4595-8680-bd1d5726be38@lucifer.local>
On Wed, Jan 28, 2026 at 09:33:44AM +0000, Lorenzo Stoakes wrote:
> On Tue, Jan 27, 2026 at 04:36:44PM -0500, Yury Norov wrote:
> > On Tue, Jan 27, 2026 at 02:40:03PM +0000, Lorenzo Stoakes wrote:
> > > On Tue, Jan 27, 2026 at 08:53:44AM -0500, Yury Norov wrote:
> > > > On Thu, Jan 22, 2026 at 04:06:09PM +0000, Lorenzo Stoakes wrote:
> >
> > ...
> >
> > > > Even if you expect adding more flags, u128 would double your capacity,
> > > > and people will still be able to use language-supported operation on
> > > > the bits in flag. Which looks simpler to me...
> > >
> > > u128 isn't supported on all architectures, VMA flags have to have absolutely
> >
> > What about big integers?
> >
> > typedef unsigned _BitInt(VMA_FLAGS_COUNT) vma_flags_t
>
> There is no use of _BitInt anywhere in the kernel. That seems to be a
> C23-only feature with limited compiler support that we simply couldn't use
> yet.
>
> https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2025/p3639r0.html tells
> me that it's supported in clang 16+ and gcc 14+.
>
> We cannot put such a restriction on compilers in the kernel, obviously.
>
> >
> > > We want to be able to arbitrarily extend this as we please in the future. So
> > > using u64 wouldn't buy us _anything_ except getting the 32-bit kernels in line.
> >
> > So enabling 32-bit arches is a big deal, even if it's a temporary
> > solution. Again, how many flags in your opinion are blocked because of
> > 32-bit integer limitation? How soon 64-bit capacity will get fully
> > used?
>
> In my opinion? I'm not sure where my opinion comes into this? There are 43 VMA
> flags and 32-bits available in 32-bit kernels.
>
> As I said to you before Yury, when adding new flags you have to add a whole
> load of mess of #ifdef CONFIG_64BIT ... #endif etc. around things that have
> nothing to do with 64-bit vs 32-bit architecture as a result.
>
> It's a mess, we've run out.
>
> Also something that might not have occurred to you - there is a chilling
> effect of limited VMA flag availability - the bar to adding flags is
> higher, and features that might have used VMA flags but need general kernel
> support (incl. 32-bit) have to find other ways to store state like this.
>
For the record, I fully agree with all of the points you made. I don't think
it makes sense to hold this change back waiting for a feature that right
now is relatively unobtainable (also IIRC the ABI around _BitInt was a bit
unstable and confusing in general, I don't know if that changed).
The goals are to:
1) get more than sizeof(unsigned long) * 8 flags so we don't have to uglify
and gatekeep things behind 64-bit. Also letting us use VMA flags more freely.
2) not cause any performance/codegen regression
Yes, the current patchset (and the current state of things too) uglifies
things a bit, but it also provides things like type safety which are sorely
needed here. And which 128-bit integers, or N-bit integers would not provide.
And if any of the above suddenly become available to us in the future, it
will be trivial to change because the VMA flags types will be fully
encapsulated with proper accessors.
Or perhaps we'll rewrite it all in rust by the end of the decade and this is
all a moot point, who knows ;)
--
Pedro
^ permalink raw reply
* Re: [PATCH v2 07/13] mm: update secretmem to use VMA flags on mmap_prepare
From: Chris Mason @ 2026-01-28 12:08 UTC (permalink / raw)
To: Lorenzo Stoakes
Cc: Andrew Morton, Jarkko Sakkinen, Dave Hansen, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H . Peter Anvin, Arnd Bergmann,
Greg Kroah-Hartman, Dan Williams, Vishal Verma, Dave Jiang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Christian Koenig, Huang Rui, Matthew Auld,
Matthew Brost, Alexander Viro, Christian Brauner, Jan Kara,
Benjamin LaHaise, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
Sandeep Dhavale, Hongbo Li, Chunhai Guo, Theodore Ts'o,
Andreas Dilger, Muchun Song, Oscar Salvador, David Hildenbrand,
Konstantin Komarov, Mike Marshall, Martin Brandenburg, Tony Luck,
Reinette Chatre, Dave Martin, James Morse, Babu Moger,
Carlos Maiolino, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
Matthew Wilcox, Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Hugh Dickins, Baolin Wang,
Zi Yan, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Jann Horn, Pedro Falcato, David Howells, Paul Moore,
James Morris, Serge E . Hallyn, Yury Norov, Rasmus Villemoes,
linux-sgx, linux-kernel, nvdimm, linux-cxl, dri-devel, intel-gfx,
linux-fsdevel, linux-aio, linux-erofs, linux-ext4, linux-mm,
ntfs3, devel, linux-xfs, keyrings, linux-security-module,
Jason Gunthorpe
In-Reply-To: <a243a09b0a5d0581e963d696de1735f61f5b2075.1769097829.git.lorenzo.stoakes@oracle.com>
Lorenzo Stoakes <lorenzo.stoakes@oracle.com> wrote:
> This patch updates secretmem to use the new vma_flags_t type which will
> soon supersede vm_flags_t altogether.
>
> In order to make this change we also have to update mlock_future_ok(), we
> replace the vm_flags_t parameter with a simple boolean is_vma_locked one,
> which also simplifies the invocation here.
>
> This is laying the groundwork for eliminating the vm_flags_t in
> vm_area_desc and more broadly throughout the kernel.
>
Hi everyone,
Running mm-new through AI review flagged this one:
> diff --git a/mm/mmap.c b/mm/mmap.c
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -225,12 +225,12 @@ static inline unsigned long round_hint_to_min(unsigned long hint)
> return hint;
> }
>
> -bool mlock_future_ok(const struct mm_struct *mm, vm_flags_t vm_flags,
> - unsigned long bytes)
> +bool mlock_future_ok(const struct mm_struct *mm, bool is_vma_locked,
> + unsigned long bytes)
> {
> unsigned long locked_pages, limit_pages;
>
> - if (!(vm_flags & VM_LOCKED) || capable(CAP_IPC_LOCK))
> + if (!is_vma_locked || capable(CAP_IPC_LOCK))
> return true;
[ ... ]
> @@ -416,7 +416,7 @@
> if (!can_do_mlock())
> return -EPERM;
>
> - if (!mlock_future_ok(mm, vm_flags, len))
> + if (!mlock_future_ok(mm, vm_flags & VM_LOCKED, len))
> return -EAGAIN;
This looks correct - do_mmap() now extracts the VM_LOCKED bit from
vm_flags and passes that to mlock_future_ok().
However, it appears check_brk_limits() was not updated when
mlock_future_ok() changed its signature. The function at mm/mmap.c:111
still passes the raw def_flags value:
return mlock_future_ok(current->mm, current->mm->def_flags, len)
? 0 : -EAGAIN;
When def_flags has any bit set, this converts to true, incorrectly
triggering mlock limit checks even when VM_LOCKED is not set. Should
this be passing (current->mm->def_flags & VM_LOCKED) instead?
[ ... ]
Additionally, the test stub at tools/testing/vma/vma_internal.h:1627
still has the old signature (mm, vm_flags_t vm_flags, bytes) while the
production code now uses (mm, bool is_vma_locked, bytes). This could
cause compilation issues or mask bugs in the test suite.
^ permalink raw reply
* Re: [PATCH] xfrm: kill xfrm_dev_{state,policy}_flush_secctx_check()
From: Tetsuo Handa @ 2026-01-28 10:28 UTC (permalink / raw)
To: Paul Moore, SELinux, linux-security-module
Cc: Steffen Klassert, Herbert Xu, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Simon Horman, Network Development
In-Reply-To: <CAHC9VhQq6jY63kYEQCp2t89Vv+_PDqv54RV6TO_TePDQyU6Vug@mail.gmail.com>
On 2026/01/28 6:59, Paul Moore wrote:
> It sounds like we either need to confirm that
> security_xfrm_{policy,state}_delete() is already present in all code
> paths that result in SPD/SAD deletions (in a place that can safely
> fail and return an error),
Yes.
> or we need to place
> xfrm_dev_{policy,state}_flush_secctx_check() in a location that can
> safely fail.
Did you mean xfrm_{policy,state}_flush_secctx_check() ?
Regarding xfrm_policy_flush() as an example, we can observe that we are
calling LSM hooks for must-not-fail callers. If the task_valid argument was
meant to be interpreted as whether to call LSM hooks, xfrm_policy_flush() needs
below change.
int xfrm_policy_flush(struct net *net, u8 type, bool task_valid)
{
(...snipped...)
- err = xfrm_policy_flush_secctx_check(net, type, task_valid);
+ err = task_valid ? xfrm_policy_flush_secctx_check(net, type, task_valid) : 0;
(...snipped...)
}
> The patch doesn't relocate
> xfrm_dev_{policy,state}_flush_secctx_check() and I don't really see
> any mention about security_xfrm_{policy,state}_delete() in the patch
> or description;
> have you verified that the LSM xfrm hooks are still
> being called to authorize the removals?
Please distinguish caller for "delete" and caller for "release".
LSM xfrm hooks need to be called for "delete" callers, but
LSM xfrm hooks need to be bypassed for "release" callers.
Maybe task_valid == true is for "delete" callers and task_valid == false is
for "release" callers; though it seems that some locations are passing wrong
task_valid flag (e.g. xfrm_dev_down() is passing task_valid == true despite
it is a "release" caller).
If 'task_valid == true is for "delete" callers and task_valid == false is for
"release" callers' does not hold, we might need to add a "forced" flag
(which is interpreted as whether LSM hooks must be bypassed) like
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit?h=next-20260123&id=fc0f090e41e652d158f946c616cdd82baed3c8f4
does.
Anyway, this patch which kills xfrm_dev_{state,policy}_flush_secctx_check()
can be applied as-is because all callers are "release" callers.
^ permalink raw reply
* Re: [PATCH v2 00/13] mm: add bitmap VMA flag helpers and convert all mmap_prepare to use them
From: Lorenzo Stoakes @ 2026-01-28 9:33 UTC (permalink / raw)
To: Yury Norov
Cc: Andrew Morton, Jarkko Sakkinen, Dave Hansen, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, x86, H . Peter Anvin, Arnd Bergmann,
Greg Kroah-Hartman, Dan Williams, Vishal Verma, Dave Jiang,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Christian Koenig, Huang Rui, Matthew Auld,
Matthew Brost, Alexander Viro, Christian Brauner, Jan Kara,
Benjamin LaHaise, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
Sandeep Dhavale, Hongbo Li, Chunhai Guo, Theodore Ts'o,
Andreas Dilger, Muchun Song, Oscar Salvador, David Hildenbrand,
Konstantin Komarov, Mike Marshall, Martin Brandenburg, Tony Luck,
Reinette Chatre, Dave Martin, James Morse, Babu Moger,
Carlos Maiolino, Damien Le Moal, Naohiro Aota, Johannes Thumshirn,
Matthew Wilcox, Liam R . Howlett, Vlastimil Babka, Mike Rapoport,
Suren Baghdasaryan, Michal Hocko, Hugh Dickins, Baolin Wang,
Zi Yan, Nico Pache, Ryan Roberts, Dev Jain, Barry Song,
Lance Yang, Jann Horn, Pedro Falcato, David Howells, Paul Moore,
James Morris, Serge E . Hallyn, Yury Norov, Rasmus Villemoes,
linux-sgx, linux-kernel, nvdimm, linux-cxl, dri-devel, intel-gfx,
linux-fsdevel, linux-aio, linux-erofs, linux-ext4, linux-mm,
ntfs3, devel, linux-xfs, keyrings, linux-security-module,
Jason Gunthorpe
In-Reply-To: <aXkv7DSUbdY-RD5d@yury>
On Tue, Jan 27, 2026 at 04:36:44PM -0500, Yury Norov wrote:
> On Tue, Jan 27, 2026 at 02:40:03PM +0000, Lorenzo Stoakes wrote:
> > On Tue, Jan 27, 2026 at 08:53:44AM -0500, Yury Norov wrote:
> > > On Thu, Jan 22, 2026 at 04:06:09PM +0000, Lorenzo Stoakes wrote:
>
> ...
>
> > > Even if you expect adding more flags, u128 would double your capacity,
> > > and people will still be able to use language-supported operation on
> > > the bits in flag. Which looks simpler to me...
> >
> > u128 isn't supported on all architectures, VMA flags have to have absolutely
>
> What about big integers?
>
> typedef unsigned _BitInt(VMA_FLAGS_COUNT) vma_flags_t
There is no use of _BitInt anywhere in the kernel. That seems to be a
C23-only feature with limited compiler support that we simply couldn't use
yet.
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2025/p3639r0.html tells
me that it's supported in clang 16+ and gcc 14+.
We cannot put such a restriction on compilers in the kernel, obviously.
>
> > We want to be able to arbitrarily extend this as we please in the future. So
> > using u64 wouldn't buy us _anything_ except getting the 32-bit kernels in line.
>
> So enabling 32-bit arches is a big deal, even if it's a temporary
> solution. Again, how many flags in your opinion are blocked because of
> 32-bit integer limitation? How soon 64-bit capacity will get fully
> used?
In my opinion? I'm not sure where my opinion comes into this? There are 43 VMA
flags and 32-bits available in 32-bit kernels.
As I said to you before Yury, when adding new flags you have to add a whole
load of mess of #ifdef CONFIG_64BIT ... #endif etc. around things that have
nothing to do with 64-bit vs 32-bit architecture as a result.
It's a mess, we've run out.
Also something that might not have occurred to you - there is a chilling
effect of limited VMA flag availability - the bar to adding flags is
higher, and features that might have used VMA flags but need general kernel
support (incl. 32-bit) have to find other ways to store state like this.
>
> > Using an integral value doesn't give us any kind of type safety, nor does it
> > give us as easy a means to track what users are doing with flags - both
> > additional benefits of this change.
>
> I tried the below, and it works OK for me with i386:
>
> $ cat bi.c
> #include <stdio.h>
> #include <limits.h>
>
> int main() {
> unsigned _BitInt(128) a = (_BitInt(128))1 << 65;
> unsigned _BitInt(128) b = (_BitInt(128))1 << 66;
>
> printf("a | b == %llx\n", (unsigned long long)((a | b)>>64));
> printf("BITINT_MAXWIDTH == 0x%x\n", BITINT_MAXWIDTH);
>
> return 0;
> }
>
> $ clang -m32 -std=c2x bi.c
> $ ./a.out
> a | b == 6
> BITINT_MAXWIDTH == 0x800000
I'm not sure why you're replying to my points about type safety,
traceability with this program but OK?
I mean thanks for that, I wasn't aware of the c23 standard (proposal?)
_BitInt(). It's useful to know that.
We can't use it right now, but it's good to know for the future.
>
> I didn't make GCC building it, at least out of the box. So the above
> question about 64-bit capacity has a practical meaning. If we've got a
> few years to let GCC fully support big integers as clang does, we don't
> have to wish anything else.
As long as you assume that all architectures will be supported, all
compilers used by users to build the kernel will support it, and Linus will
be fine with us using this.
That could be years, that could be never.
Also - and here's a really important point - the underlying implementation
_doesn't matter_.
Right now it's bitmaps.
By abstracting the VMA flags into an opaque type and providing helper
functions we also enable the ability to _change the implementation_.
So if this time comes, we can simply switch everything over. Job done.
Your suggested 'do nothing and hope' approach achieves none of this.
>
> I'd like to put it right. I maintain bitmaps, and I like it widely
> adopted. But when it comes to flags, being able to use plain logic
> operations looks so important to me so I'd like to make sure that
> switching to bitmaps is the only working option.
I'm not sure you're making a technical argument here?
From the point of view of mm, the ability to arbitrarily manipulate VMA
flags is a bug not a feature. The other part of this series is to make
changes to the f_op->mmap_prepare feature, which was explicitly implemented
in order to avoid such arbitrarily behaviour from drivers.
It's actually hugely valueable to make this change in such a way as we can
trace, with type safety, VMA flag usage throughout the kernel, and know
that for instance - on mmap setup - we don't need to worry about VMA
stabilisation - which feeds into other work re: killable VMA locks.
In summary, this series represents a workable and sensible means of
addressing all of these issues in one fell swoop, similar to the means
through which mm flags were extended across both 32-bit and 64-bit kernels.
We can in future choose to use _BitInt(), u128, or whatever we please
underneath, and which makes sense to use should conditions change and we
choose to do so for good technical reasons.
Any argument on the basis of 'allow the flags to continue to be manipulated
as they are' is I think mistaken.
Keep in mind that bitmap VMA flags are _already_ a merged feature in the
kernel, and this series only works to add sensible helper functions to
manipulate these flags and moves mmap_prepare to using them exclusively.
If we find cases where somehow the compiler does the wrong thing, we
already have functions in mm_types.h that allow us to address the first
system-word bits of the underlying type and can restrict core flags to
being at system word count or less.
I hope that won't ever be necessary, but we have means of adressing that
should any issue arise like that.
Thanks, Lorenzo
^ permalink raw reply
* Re: [PATCH] ipc: don't audit capability check in ipc_permissions()
From: Serge E. Hallyn @ 2026-01-28 3:26 UTC (permalink / raw)
To: Paul Moore
Cc: Serge E. Hallyn, Ondrej Mosnacek, Andrew Morton,
Eric W . Biederman, Alexey Gladkov, linux-kernel,
linux-security-module, selinux
In-Reply-To: <CAHC9VhSLi2-TBUyayML+tAuC+XF7jCAAL48oCB4qQqTrGXcMyA@mail.gmail.com>
On Tue, Jan 27, 2026 at 05:06:47PM -0500, Paul Moore wrote:
> On Mon, Jan 26, 2026 at 9:01 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> > On Mon, Jan 26, 2026 at 05:50:12PM -0500, Paul Moore wrote:
> > > On Thu, Jan 22, 2026 at 9:56 AM Ondrej Mosnacek <omosnace@redhat.com> wrote:
> > > >
> > > > The IPC sysctls implement the ctl_table_root::permissions hook and
> > > > they override the file access mode based on the CAP_CHECKPOINT_RESTORE
> > > > capability, which is being checked regardless of whether any access is
> > > > actually denied or not, so if an LSM denies the capability, an audit
> > > > record may be logged even when access is in fact granted.
> > > >
> > > > It wouldn't be viable to restructure the sysctl permission logic to only
> > > > check the capability when the access would be actually denied if it's
> > > > not granted. Thus, do the same as in net_ctl_permissions()
> > > > (net/sysctl_net.c) - switch from ns_capable() to ns_capable_noaudit(),
> > > > so that the check never emits an audit record.
> > > >
> > > > Fixes: 0889f44e2810 ("ipc: Check permissions for checkpoint_restart sysctls at open time")
> > > > Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
> > > > ---
> > > > include/linux/capability.h | 6 ++++++
> > > > ipc/ipc_sysctl.c | 2 +-
> > > > 2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > This change seems reasonable to me, but I would make sure Serge has a
> > > chance to review/ACK this patch as it has a capability impact.
> >
> > Acked-by: Serge Hallyn <serge@hallyn.com>
> >
> > Thanks - looks good to me.
>
> I don't see a dedicated IPC maintainer/tree, do you want to take this
> via the capabilities tree Serge?
Will do.
-serge
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox