* Re: [PATCH] keys: Pin request_key_auth payload in instantiate paths
From: Jarkko Sakkinen @ 2026-06-08 5:29 UTC (permalink / raw)
To: Shaomin Chen
Cc: keyrings, linux-security-module, linux-kernel, David Howells,
Paul Moore, James Morris, Serge E. Hallyn
In-Reply-To: <aiYynOxFk3LuK4LA@kernel.org>
On Mon, Jun 08, 2026 at 06:10:23AM +0300, Jarkko Sakkinen wrote:
> On Mon, Jun 08, 2026 at 06:06:21AM +0300, Jarkko Sakkinen wrote:
> > On Tue, May 26, 2026 at 10:48:38AM +0800, Shaomin Chen wrote:
> > > keyctl_instantiate_key_common() reads request_key_auth from the assumed
> > > auth key before copying an instantiation payload from userspace. The copy
> > > can fault and sleep. If the request completes and revokes the auth key in
> > > that window, the auth payload can be detached and freed before the
> > > instantiate path uses it again.
> > >
> > > A request-key helper reproducer can trigger this race. One helper child
> > > blocks in KEYCTL_INSTANTIATE_IOV while the original helper instantiates the
> > > requested key and returns. KASAN then reports a use-after-free from the
> > > stale request_key_auth payload in keyctl_instantiate_key_common().
> > >
> > > Give request_key_auth payloads a refcount. Take a payload reference while
> >
> > Please, name concrete things accurately. I.e. 'usage' in this case. If
> > you have a name, use it instead of obfuscating generalizations.
> >
> > > authkey->sem stabilizes the payload and revocation state. Hold that
> > > reference across the instantiate and reject paths. Drop the auth key
> > > owning reference from revoke and destroy.
> > >
> > > Reported-by: Shaomin Chen <eeesssooo020@gmail.com>
> > > Closes: https://lore.kernel.org/r/20260519144403.436694-1-eeesssooo020@gmail.com
> > > Signed-off-by: Shaomin Chen <eeesssooo020@gmail.com>
> > > ---
> > > include/keys/request_key_auth-type.h | 2 ++
> > > security/keys/internal.h | 2 ++
> > > security/keys/keyctl.c | 24 +++++++++++++++-----
> > > security/keys/request_key_auth.c | 33 ++++++++++++++++++++++++++--
> > > 4 files changed, 53 insertions(+), 8 deletions(-)
> >
> > So first, couple of things.
> >
> > I'm not going to test not that well documented involving OOT driver.
>
> Oops, sorry typo. "not that well documented reproducer" :-)
>
> But it is cool we just then need to draw the picture.
I think I got this:
A: request_key() B: KEYCTL_INSTANTIATE_IOV
---------------- -------------------------
create auth key
store rka in auth key
wait for helper
get auth key
load rka from auth key
copy user payload
sleep on #PF
helper completed
detach and free rka
destroy auth key
wake up
use rka->target_key
**USE-AFTER-FREE**
So nothing really complicated here, is there?
`
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: allow request-key path to be configured via Kconfig
From: Jarkko Sakkinen @ 2026-06-08 4:59 UTC (permalink / raw)
To: Gary Guo
Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <aiZJ94eugtNHcILD@kernel.org>
On Mon, Jun 08, 2026 at 07:50:03AM +0300, Jarkko Sakkinen wrote:
> On Sun, Jun 07, 2026 at 02:49:27PM +0100, Gary Guo wrote:
> > From: Gary Guo <gary@garyguo.net>
> >
> > Some Linux distributions (e.g. NixOS) does not have /sbin present, and they
> > currently carry patches to replace /sbin/request-key to some other path.
>
> Sorry but no configuration for introducing API divergence.
Not sure right now but one option might kernel command-line. Then it is
known at run-time, can be signed etc. Compiled value has no identity in
the same way.
And I don't care if NixOS has such a problem as I've not have any stake
making of those decisions.
You really should explain why it makes sense to have such feature i.e.,
why is it useful. And if NixOS considered, why is it useful for NixOS.
This all should be in the commit message.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: allow request-key path to be configured via Kconfig
From: Jarkko Sakkinen @ 2026-06-08 4:49 UTC (permalink / raw)
To: Gary Guo
Cc: David Howells, Paul Moore, James Morris, Serge E. Hallyn,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260607134928.2832202-1-gary@kernel.org>
On Sun, Jun 07, 2026 at 02:49:27PM +0100, Gary Guo wrote:
> From: Gary Guo <gary@garyguo.net>
>
> Some Linux distributions (e.g. NixOS) does not have /sbin present, and they
> currently carry patches to replace /sbin/request-key to some other path.
Sorry but no configuration for introducing API divergence.
>
> Follow the way modprobe handles this by making this a Kconfig option which
> defaults to the current /sbin/request-key.
>
> Also changed "char const" to "const char" as checkpatch complains
> otherwise.
>
> Link: https://github.com/NixOS/nixpkgs/blob/6b316287bae2ee04c9b93c8c858d930fd07d7338/pkgs/os-specific/linux/kernel/request-key-helper.patch
> Signed-off-by: Gary Guo <gary@garyguo.net>
> ---
> I did not update mentions of /sbin/request-key in documentation and
> elsewhere, as "/sbin/request-key" is concise while "request-key UMH" is
> more mouthful and less clear.
>
> Number of distros that doesn't have /sbin is limited so I think it wouldn't
> create much confusion. Similarly, there are a lot of places where
> /sbin/modprobe is mentioned despite it is technically configurable.
> ---
> security/keys/Kconfig | 9 +++++++++
> security/keys/request_key.c | 2 +-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> index f4510d8cb485..ee3c3d85fc03 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -40,6 +40,15 @@ config KEYS_REQUEST_CACHE
> key. Pathwalk will call multiple methods for each dentry traversed
> (permission, d_revalidate, lookup, getxattr, getacl, ...).
>
> +config REQUEST_KEY_PATH
> + string "Path to the request-key binary"
> + default "/sbin/request-key"
> + help
> + Path of the request-key usermode helper binary.
> +
> + This program is invoked by the kernel when the kernel is asked for
> + a key that it doesn't have immediately available.
> +
> config PERSISTENT_KEYRINGS
> bool "Enable register of persistent per-UID keyrings"
> help
> diff --git a/security/keys/request_key.c b/security/keys/request_key.c
> index a7673ad86d18..ac8f9d1a87ad 100644
> --- a/security/keys/request_key.c
> +++ b/security/keys/request_key.c
> @@ -117,7 +117,7 @@ static int call_usermodehelper_keys(const char *path, char **argv, char **envp,
> */
> static int call_sbin_request_key(struct key *authkey, void *aux)
> {
> - static char const request_key[] = "/sbin/request-key";
> + static const char request_key[] = CONFIG_REQUEST_KEY_PATH;
> struct request_key_auth *rka = get_request_key_auth(authkey);
> const struct cred *cred = current_cred();
> key_serial_t prkey, sskey;
>
> base-commit: 6e845bcb78c95af935094040bd4edc3c2b6dd784
> --
> 2.54.0
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH next] keys: Replace strcpy(derived_buf, "AUTH_KEY") with strscpy(..., HASH_SIZE)
From: Jarkko Sakkinen @ 2026-06-08 4:46 UTC (permalink / raw)
To: david.laight.linux
Cc: Kees Cook, linux-hardening, Arnd Bergmann, keyrings,
linux-integrity, linux-kernel, linux-security-module,
David Howells, James Morris, Mimi Zohar, Paul Moore,
Serge E. Hallyn
In-Reply-To: <20260606202633.5018-9-david.laight.linux@gmail.com>
On Sat, Jun 06, 2026 at 09:26:03PM +0100, david.laight.linux@gmail.com wrote:
> From: David Laight <david.laight.linux@gmail.com>
>
> derived_buf is guaranteed to be HASH_SIZE - and it is more than enough.
> The strscpy() degenerates into an memcpy() (as did the strcpy()).
> Do the same for the associated "ENC_KEY" copy.
>
> Removes a possibly unbounded strcpy().
>
> Signed-off-by: David Laight <david.laight.linux@gmail.com>
> ---
> This is one of a group of patches that remove potentially unbounded
> strcpy() calls.
>
> They are mostly replaced by strscpy() or, when strlen() has just been
> called, with memcpy() (usually including the '\0').
>
> Calls with copy string literals into arrays are left unchanged.
> They are safe and easily detected as such.
>
> The changes were made by getting the compiler to detect the calls and
> then fixing the code by hand.
>
> Note that all the changes are only compile tested.
>
> Some Makefiles were changed to allow files to contain strcpy().
> As well as 'difficult to fix' files, this included 'show' functions
> as they really need to use sysfs_emit() or seq_printf().
>
> All the patches are being sent individually to avoid very long cc lists.
> Apologies for the terse commit messages and likely unexpected tags.
> (There are about 100 patches in total.)
>
> security/keys/encrypted-keys/encrypted.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
> index 56b531587a1e..59cb77b237b3 100644
> --- a/security/keys/encrypted-keys/encrypted.c
> +++ b/security/keys/encrypted-keys/encrypted.c
> @@ -343,9 +343,9 @@ static int get_derived_key(u8 *derived_key, enum derived_key_type key_type,
> return -ENOMEM;
>
> if (key_type)
> - strcpy(derived_buf, "AUTH_KEY");
> + strscpy(derived_buf, "AUTH_KEY", HASH_SIZE);
> else
> - strcpy(derived_buf, "ENC_KEY");
> + strscpy(derived_buf, "ENC_KEY", HASH_SIZE);
>
> memcpy(derived_buf + strlen(derived_buf) + 1, master_key,
> master_keylen);
> --
> 2.39.5
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v6 4/4] tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in
From: Jarkko Sakkinen @ 2026-06-08 4:45 UTC (permalink / raw)
To: Yeoreum Yun
Cc: linux-security-module, linux-kernel, linux-integrity, paul, zohar,
roberto.sassu, noodles, sudeep.holla, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <20260605144325.434436-5-yeoreum.yun@arm.com>
On Fri, Jun 05, 2026 at 03:43:25PM +0100, Yeoreum Yun wrote:
> commit 746d9e9f62a6 ("tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's built-in")
> probe tpm_crb_ffa forcefully when it's built-in to integrate with IMA.
>
> However, IMA now provides the IMA_INIT_LATE_SYNC build option, which
> initialises IMA at the late_initcall_sync level, so this change is no
> longer required.
>
> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> drivers/char/tpm/tpm_crb_ffa.c | 18 +++---------------
> 1 file changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
> index 99f1c1e5644b..025c4d4b17ca 100644
> --- a/drivers/char/tpm/tpm_crb_ffa.c
> +++ b/drivers/char/tpm/tpm_crb_ffa.c
> @@ -177,23 +177,13 @@ static int tpm_crb_ffa_to_linux_errno(int errno)
> */
> int tpm_crb_ffa_init(void)
> {
> - int ret = 0;
> -
> - if (!IS_MODULE(CONFIG_TCG_ARM_CRB_FFA)) {
> - ret = ffa_register(&tpm_crb_ffa_driver);
> - if (ret) {
> - tpm_crb_ffa = ERR_PTR(-ENODEV);
> - return ret;
> - }
> - }
> -
> if (!tpm_crb_ffa)
> - ret = -ENOENT;
> + return -ENOENT;
>
> if (IS_ERR_VALUE(tpm_crb_ffa))
> - ret = -ENODEV;
> + return -ENODEV;
>
> - return ret;
> + return 0;
> }
> EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
>
> @@ -405,9 +395,7 @@ static struct ffa_driver tpm_crb_ffa_driver = {
> .id_table = tpm_crb_ffa_device_id,
> };
>
> -#ifdef MODULE
> module_ffa_driver(tpm_crb_ffa_driver);
> -#endif
>
> MODULE_AUTHOR("Arm");
> MODULE_DESCRIPTION("TPM CRB FFA driver");
> --
> LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
>
Is this different I applied?
If yes, I'll swap (if mandatory).
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: prevent slab cache merging for key_jar
From: Jarkko Sakkinen @ 2026-06-08 4:27 UTC (permalink / raw)
To: Vlastimil Babka (SUSE)
Cc: Mohammed EL Kadiri, David Howells, Paul Moore, James Morris,
Serge E . Hallyn, Kees Cook, Vlastimil Babka, keyrings,
linux-security-module, linux-hardening, linux-kernel
In-Reply-To: <19fb9d38-fe02-4653-bbad-58806e55ca84@kernel.org>
On Fri, Jun 05, 2026 at 02:16:00PM +0200, Vlastimil Babka (SUSE) wrote:
> On 6/4/26 14:50, Mohammed EL Kadiri wrote:
> > The key_jar slab cache holds struct key objects containing cryptographic
> > keys, authentication tokens, and keyring linkage. This cache currently
> > lacks merge prevention, allowing the SLUB allocator to merge it with
> > other similarly-sized caches.
> >
> > On a default Ubuntu 6.17.0-23-generic system, key_jar has 5 aliases,
> > meaning 5 unrelated object types share its slab pages. struct key is
> > 224 bytes, placed in 256-byte slabs alongside biovec-16, maple_node,
> > ip6_dst_cache, task_delay_info, and kmalloc-256 users.
> >
> > Cross-cache heap exploitation is a well-documented attack class
> > (CVE-2022-29582, CVE-2022-2588, CVE-2021-22555) where slab cache
> > merging enables type confusion between unrelated kernel objects. A
> > use-after-free in any subsystem sharing slab pages with key_jar could
> > allow an attacker to reclaim a freed slot as a struct key, or corrupt
> > an existing key through a dangling pointer to a different type.
> >
> > Add SLAB_NO_MERGE to ensure key_jar receives dedicated slab pages,
> > eliminating cross-cache attacks targeting struct key. The memory
> > overhead is minimal: with 32 objects per slab page and typical key
> > usage bounded by system keyring size, the cost of dedicated pages is
> > negligible. There is zero performance impact on the allocation hot
> > path.
> >
> > This follows the precedent set by skbuff_head_cache (net/core/skbuff.c)
> > which uses SLAB_NO_MERGE for similar isolation requirements.
> >
> > Signed-off-by: Mohammed EL Kadiri <med08elkadiri@gmail.com>
>
> Acked-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>
Thank you.
has been applied
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: prevent slab cache merging for key_jar
From: Jarkko Sakkinen @ 2026-06-08 4:22 UTC (permalink / raw)
To: Mohammed EL Kadiri
Cc: David Howells, Paul Moore, James Morris, Serge E . Hallyn,
Kees Cook, Vlastimil Babka, keyrings, linux-security-module,
linux-hardening, linux-kernel
In-Reply-To: <20260604125034.13757-1-med08elkadiri@gmail.com>
On Thu, Jun 04, 2026 at 01:50:34PM +0100, Mohammed EL Kadiri wrote:
> The key_jar slab cache holds struct key objects containing cryptographic
> keys, authentication tokens, and keyring linkage. This cache currently
> lacks merge prevention, allowing the SLUB allocator to merge it with
> other similarly-sized caches.
>
> On a default Ubuntu 6.17.0-23-generic system, key_jar has 5 aliases,
> meaning 5 unrelated object types share its slab pages. struct key is
> 224 bytes, placed in 256-byte slabs alongside biovec-16, maple_node,
> ip6_dst_cache, task_delay_info, and kmalloc-256 users.
>
> Cross-cache heap exploitation is a well-documented attack class
> (CVE-2022-29582, CVE-2022-2588, CVE-2021-22555) where slab cache
> merging enables type confusion between unrelated kernel objects. A
> use-after-free in any subsystem sharing slab pages with key_jar could
> allow an attacker to reclaim a freed slot as a struct key, or corrupt
> an existing key through a dangling pointer to a different type.
>
> Add SLAB_NO_MERGE to ensure key_jar receives dedicated slab pages,
> eliminating cross-cache attacks targeting struct key. The memory
> overhead is minimal: with 32 objects per slab page and typical key
> usage bounded by system keyring size, the cost of dedicated pages is
> negligible. There is zero performance impact on the allocation hot
> path.
>
> This follows the precedent set by skbuff_head_cache (net/core/skbuff.c)
> which uses SLAB_NO_MERGE for similar isolation requirements.
>
~/work/kernel.org/jarkko/linux-tpmdd master*
❯ git log --oneline -1 d0bf7d5759c1d89fb013aa41cca5832e00b9632a
d0bf7d5759c1 mm/slab: introduce kmem_cache flag SLAB_NO_MERGE
~/work/kernel.org/jarkko/linux-tpmdd master*
❯ git describe --contains d0bf7d5759c1d89fb013aa41cca5832e00b9632a
v6.5-rc1~137^2^3~1
So we could probably forward to stable's starting from v6.6+ if that
is necessary / makes sense?
It's not a bug fix but kind of still I think would be a change that
stable kernels are better off with it than without it.
What do you think?
> Signed-off-by: Mohammed EL Kadiri <med08elkadiri@gmail.com>
> ---
> security/keys/key.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/keys/key.c b/security/keys/key.c
> index 3bbdde778631..592b65cf8539 100644
> --- a/security/keys/key.c
> +++ b/security/keys/key.c
> @@ -1275,7 +1275,7 @@ void __init key_init(void)
> {
> /* allocate a slab in which we can store keys */
> key_jar = kmem_cache_create("key_jar", sizeof(struct key),
> - 0, SLAB_HWCACHE_ALIGN|SLAB_PANIC, NULL);
> + 0, SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_NO_MERGE, NULL);
>
> /* add the special key types */
> list_add_tail(&key_type_keyring.link, &key_types_list);
> --
> 2.43.0
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v5 4/4] tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in
From: Jarkko Sakkinen @ 2026-06-08 4:01 UTC (permalink / raw)
To: Yeoreum Yun
Cc: linux-security-module, linux-kernel, linux-integrity, paul, zohar,
roberto.sassu, noodles, sudeep.holla, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <ah7TAk3iItltddzT@e129823.arm.com>
On Tue, Jun 02, 2026 at 01:56:34PM +0100, Yeoreum Yun wrote:
> > On Mon, Jun 01, 2026 at 03:27:49PM +0100, Yeoreum Yun wrote:
> > > commit 746d9e9f62a6 ("tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's build_in")
> > > probe tpm_crb_ffa forcefully when it's built-in to integrate with IMA.
> > >
> > > However, IMA now provides the IMA_INIT_LATE_SYNC build option, which
> > > initialises IMA at the late_initcall_sync level, so this change is no
> > > longer required.
> > >
> > > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> >
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> Might be rb tag?. Thanks!
>
> --
> Sincerely,
> Yeoreum Yun
SOB is enough information as it is a small change. I pushed it to
for-next-tpm.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v4 3/3] tpm: tpm_crb_ffa: revert defered_probed when tpm_crb_ffa is built-in
From: Jarkko Sakkinen @ 2026-06-08 3:54 UTC (permalink / raw)
To: Sudeep Holla
Cc: Yeoreum Yun, linux-security-module, linux-kernel, linux-integrity,
paul, zohar, roberto.sassu, noodles, jmorris, serge,
dmitry.kasatkin, eric.snowberg, jgg
In-Reply-To: <20260602-vague-proficient-gibbon-b005c0@sudeepholla>
On Tue, Jun 02, 2026 at 10:57:25AM +0100, Sudeep Holla wrote:
> On Tue, Jun 02, 2026 at 04:50:42AM +0300, Jarkko Sakkinen wrote:
> > On Mon, Jun 01, 2026 at 09:54:08AM +0100, Sudeep Holla wrote:
> > > On Mon, Jun 01, 2026 at 08:17:13AM +0100, Yeoreum Yun wrote:
> > > > Hi Jarkko,
> > > >
> > > > Sorry for late answer.
> > > >
> > > > > On Mon, May 25, 2026 at 08:54:04AM +0100, Yeoreum Yun wrote:
> > > > > > commit 746d9e9f62a6 ("tpm: tpm_crb_ffa: try to probe tpm_crb_ffa when it's build_in")
> > > > > > probe tpm_crb_ffa forcefully when it's built-in to integrate with IMA.
> > > > > >
> > > > > > However, IMA now provides the IMA_INIT_LATE_SYNC build option, which
> > > > > > initialises IMA at the late_initcall_sync level, so this change is no
> > > > > > longer required.
> > > > > >
> > > > > > Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> > > > > > ---
> > > > > > drivers/char/tpm/tpm_crb_ffa.c | 18 +++---------------
> > > > > > 1 file changed, 3 insertions(+), 15 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
> > > > > > index 99f1c1e5644b..025c4d4b17ca 100644
> > > > > > --- a/drivers/char/tpm/tpm_crb_ffa.c
> > > > > > +++ b/drivers/char/tpm/tpm_crb_ffa.c
> > > > > > @@ -177,23 +177,13 @@ static int tpm_crb_ffa_to_linux_errno(int errno)
> > > > > > */
> > > > > > int tpm_crb_ffa_init(void)
> > > > > > {
> > > > > > - int ret = 0;
> > > > > > -
> > > > > > - if (!IS_MODULE(CONFIG_TCG_ARM_CRB_FFA)) {
> > > > > > - ret = ffa_register(&tpm_crb_ffa_driver);
> > > > > > - if (ret) {
> > > > > > - tpm_crb_ffa = ERR_PTR(-ENODEV);
> > > > > > - return ret;
> > > > > > - }
> > > > > > - }
> > > > > > -
> > > > > > if (!tpm_crb_ffa)
> > > > > > - ret = -ENOENT;
> > > > > > + return -ENOENT;
> > > > > >
> > > > > > if (IS_ERR_VALUE(tpm_crb_ffa))
> > > > > > - ret = -ENODEV;
> > > > > > + return -ENODEV;
> > > > > >
> > > > > > - return ret;
> > > > > > + return 0;
> > > > > > }
> > > > > > EXPORT_SYMBOL_GPL(tpm_crb_ffa_init);
> > > > > >
> > > > > > @@ -405,9 +395,7 @@ static struct ffa_driver tpm_crb_ffa_driver = {
> > > > > > .id_table = tpm_crb_ffa_device_id,
> > > > > > };
> > > > > >
> > > > > > -#ifdef MODULE
> > > > > > module_ffa_driver(tpm_crb_ffa_driver);
> > > > > > -#endif
> > > > > >
> > > > > > MODULE_AUTHOR("Arm");
> > > > > > MODULE_DESCRIPTION("TPM CRB FFA driver");
> > > > > > --
> > > > > > LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
> > > > > >
> > > > >
> > > > > How we would sync up this patch? Through which tree etc.
> > > >
> > > > IMHO, the IMA relevant thing would be into IMA tree,
> > > > However I think this patch would be much easier to sync into Sudeep's
> > > > FF-A tree where ff-a initilisation is reverted to device_initcall
> > > > unless you're uncomfortable.
> > > >
> > > > For this, It might be better to split this patch from this series
> > > > since by above and defer probe of ff-a would make a register failure
> > > > of registering tpm_crb_ffa driver which is built-in.
> > > >
> > > > @Sudeep what do you think?
> > > >
> > >
> > > IIRC, there is/was no dependency between these and FF-A patches that are
> > > queued in terms of build. I agree there may be dependency to get all the
> > > functionality but we can resort to linux-next for that. FF-A is not enabled
> > > in the defconfig, so anyone working on FF-A + TPM must enable then and can
> > > rely on -next IMHO.
> > >
> > > That said, I have already sent PR for FF-A to SoC team and it is already
> > > queued for v7.2. I don't have any other plans unless they are fixes.
> >
> > Works for me.
> >
>
> Sorry if I was not clear. I haven't included any TPM patches in this series
> as part of my FF-A pull request queued for v7.2. So I was asking to route this
> via your tree.
It's now in 'for-next-tpm'. Just pushed.
>
> --
> Regards,
> Sudeep
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] KEYS: Use acquire when reading state in keyring search
From: Jarkko Sakkinen @ 2026-06-08 3:48 UTC (permalink / raw)
To: Gui-Dong Han
Cc: dhowells, keyrings, ebiggers, linux-security-module, linux-kernel,
baijiaju1990
In-Reply-To: <CALbr=LYaca4+=hTsZJORQfrtzyUAGo8c+4ZEgkzgkFWQDWNJBA@mail.gmail.com>
On Tue, Jun 02, 2026 at 05:42:26PM +0800, Gui-Dong Han wrote:
> On Sat, May 30, 2026 at 9:02 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > On Fri, May 29, 2026 at 11:34:06AM +0800, Gui-Dong Han wrote:
> > > The negative-key race fix added release/acquire ordering for key use.
> > >
> > > Publish payload before state; read state before payload.
> > >
> > > keyring_search_iterator() still uses READ_ONCE() before match callbacks.
> > > An asymmetric match callback calls asymmetric_key_ids(), which reads
> > > key->payload.data[asym_key_ids].
> > >
> > > Use key_read_state() there to complete that ordering.
> >
> > OK, so... I'm having a bit trouble understanding the exact concurrency
> > scenario you're trying to describe despite I think I get the fix itself
> > i.e. it is not pairing with mark_key_instantiated?
>
> Yes, it is intended to pair with mark_key_instantiated().
OK, right. I'll apply this.
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: Pin request_key_auth payload in instantiate paths
From: Jarkko Sakkinen @ 2026-06-08 3:10 UTC (permalink / raw)
To: Shaomin Chen
Cc: keyrings, linux-security-module, linux-kernel, David Howells,
Paul Moore, James Morris, Serge E. Hallyn
In-Reply-To: <aiYxqTyfHwrDOTCs@kernel.org>
On Mon, Jun 08, 2026 at 06:06:21AM +0300, Jarkko Sakkinen wrote:
> On Tue, May 26, 2026 at 10:48:38AM +0800, Shaomin Chen wrote:
> > keyctl_instantiate_key_common() reads request_key_auth from the assumed
> > auth key before copying an instantiation payload from userspace. The copy
> > can fault and sleep. If the request completes and revokes the auth key in
> > that window, the auth payload can be detached and freed before the
> > instantiate path uses it again.
> >
> > A request-key helper reproducer can trigger this race. One helper child
> > blocks in KEYCTL_INSTANTIATE_IOV while the original helper instantiates the
> > requested key and returns. KASAN then reports a use-after-free from the
> > stale request_key_auth payload in keyctl_instantiate_key_common().
> >
> > Give request_key_auth payloads a refcount. Take a payload reference while
>
> Please, name concrete things accurately. I.e. 'usage' in this case. If
> you have a name, use it instead of obfuscating generalizations.
>
> > authkey->sem stabilizes the payload and revocation state. Hold that
> > reference across the instantiate and reject paths. Drop the auth key
> > owning reference from revoke and destroy.
> >
> > Reported-by: Shaomin Chen <eeesssooo020@gmail.com>
> > Closes: https://lore.kernel.org/r/20260519144403.436694-1-eeesssooo020@gmail.com
> > Signed-off-by: Shaomin Chen <eeesssooo020@gmail.com>
> > ---
> > include/keys/request_key_auth-type.h | 2 ++
> > security/keys/internal.h | 2 ++
> > security/keys/keyctl.c | 24 +++++++++++++++-----
> > security/keys/request_key_auth.c | 33 ++++++++++++++++++++++++++--
> > 4 files changed, 53 insertions(+), 8 deletions(-)
>
> So first, couple of things.
>
> I'm not going to test not that well documented involving OOT driver.
Oops, sorry typo. "not that well documented reproducer" :-)
But it is cool we just then need to draw the picture.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH] keys: Pin request_key_auth payload in instantiate paths
From: Jarkko Sakkinen @ 2026-06-08 3:06 UTC (permalink / raw)
To: Shaomin Chen
Cc: keyrings, linux-security-module, linux-kernel, David Howells,
Paul Moore, James Morris, Serge E. Hallyn
In-Reply-To: <20260526024838.3368409-1-eeesssooo020@gmail.com>
On Tue, May 26, 2026 at 10:48:38AM +0800, Shaomin Chen wrote:
> keyctl_instantiate_key_common() reads request_key_auth from the assumed
> auth key before copying an instantiation payload from userspace. The copy
> can fault and sleep. If the request completes and revokes the auth key in
> that window, the auth payload can be detached and freed before the
> instantiate path uses it again.
>
> A request-key helper reproducer can trigger this race. One helper child
> blocks in KEYCTL_INSTANTIATE_IOV while the original helper instantiates the
> requested key and returns. KASAN then reports a use-after-free from the
> stale request_key_auth payload in keyctl_instantiate_key_common().
>
> Give request_key_auth payloads a refcount. Take a payload reference while
Please, name concrete things accurately. I.e. 'usage' in this case. If
you have a name, use it instead of obfuscating generalizations.
> authkey->sem stabilizes the payload and revocation state. Hold that
> reference across the instantiate and reject paths. Drop the auth key
> owning reference from revoke and destroy.
>
> Reported-by: Shaomin Chen <eeesssooo020@gmail.com>
> Closes: https://lore.kernel.org/r/20260519144403.436694-1-eeesssooo020@gmail.com
> Signed-off-by: Shaomin Chen <eeesssooo020@gmail.com>
> ---
> include/keys/request_key_auth-type.h | 2 ++
> security/keys/internal.h | 2 ++
> security/keys/keyctl.c | 24 +++++++++++++++-----
> security/keys/request_key_auth.c | 33 ++++++++++++++++++++++++++--
> 4 files changed, 53 insertions(+), 8 deletions(-)
So first, couple of things.
I'm not going to test not that well documented involving OOT driver.
That does necessarily mean same as NAK but we need much more clarity
here.
You should start off not mentioning 'reproducer'. If it does not
exist in public, it does not exist. It's not good for anything
here.
Then, put a concurrency scenario with A/B sidecars.
Some comments below.
>
> diff --git a/include/keys/request_key_auth-type.h b/include/keys/request_key_auth-type.h
> index 36b89a933310..01e42ee5f409 100644
> --- a/include/keys/request_key_auth-type.h
> +++ b/include/keys/request_key_auth-type.h
> @@ -9,12 +9,14 @@
> #define _KEYS_REQUEST_KEY_AUTH_TYPE_H
>
> #include <linux/key.h>
> +#include <linux/refcount.h>
BTW, did you test compilation w/o adding this? Just potential
low-hanging fruit diff to remove.
>
> /*
> * Authorisation record for request_key().
> */
> struct request_key_auth {
> struct rcu_head rcu;
> + refcount_t usage;
> struct key *target_key;
> struct key *dest_keyring;
> const struct cred *cred;
> diff --git a/security/keys/internal.h b/security/keys/internal.h
> index 2cffa6dc8255..b7b622bc36a1 100644
> --- a/security/keys/internal.h
> +++ b/security/keys/internal.h
> @@ -208,6 +208,8 @@ extern struct key *request_key_auth_new(struct key *target,
> const void *callout_info,
> size_t callout_len,
> struct key *dest_keyring);
> +struct request_key_auth *request_key_auth_get(struct key *authkey);
> +void request_key_auth_put(struct request_key_auth *rka);
>
> extern struct key *key_get_instantiation_authkey(key_serial_t target_id);
>
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index ef855d69c97a..d14ace88e529 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -1197,9 +1197,13 @@ static long keyctl_instantiate_key_common(key_serial_t id,
> if (!instkey)
> goto error;
>
> - rka = instkey->payload.data[0];
> - if (rka->target_key->serial != id)
> + rka = request_key_auth_get(instkey);
> + if (!rka) {
> + ret = -EKEYREVOKED;
> goto error;
> + }
> + if (rka->target_key->serial != id)
> + goto error_put_rka;
This label should not be introduced and generally please do everything
not add extra fat to diff. This is already fat for a claiming to be
bug fix.
>
> /* pull the payload in if one was supplied */
> payload = NULL;
> @@ -1208,7 +1212,7 @@ static long keyctl_instantiate_key_common(key_serial_t id,
> ret = -ENOMEM;
> payload = kvmalloc(plen, GFP_KERNEL);
> if (!payload)
> - goto error;
> + goto error_put_rka;
>
> ret = -EFAULT;
> if (!copy_from_iter_full(payload, plen, from))
> @@ -1234,6 +1238,8 @@ static long keyctl_instantiate_key_common(key_serial_t id,
>
> error2:
> kvfree_sensitive(payload, plen);
> +error_put_rka:
> + request_key_auth_put(rka);
> error:
> return ret;
> }
> @@ -1358,15 +1364,19 @@ long keyctl_reject_key(key_serial_t id, unsigned timeout, unsigned error,
> if (!instkey)
> goto error;
>
> - rka = instkey->payload.data[0];
> - if (rka->target_key->serial != id)
> + rka = request_key_auth_get(instkey);
> + if (!rka) {
> + ret = -EKEYREVOKED;
> goto error;
> + }
> + if (rka->target_key->serial != id)
> + goto error_put_rka;
>
> /* find the destination keyring if present (which must also be
> * writable) */
> ret = get_instantiation_keyring(ringid, rka, &dest_keyring);
> if (ret < 0)
> - goto error;
> + goto error_put_rka;
>
> /* instantiate the key and link it into a keyring */
> ret = key_reject_and_link(rka->target_key, timeout, error,
> @@ -1379,6 +1389,8 @@ long keyctl_reject_key(key_serial_t id, unsigned timeout, unsigned error,
> if (ret == 0)
> keyctl_change_reqkey_auth(NULL);
>
> +error_put_rka:
> + request_key_auth_put(rka);
> error:
> return ret;
> }
> diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
> index a7d7538c1f70..282e09d8fa46 100644
> --- a/security/keys/request_key_auth.c
> +++ b/security/keys/request_key_auth.c
> @@ -23,6 +23,7 @@ static void request_key_auth_describe(const struct key *, struct seq_file *);
> static void request_key_auth_revoke(struct key *);
> static void request_key_auth_destroy(struct key *);
> static long request_key_auth_read(const struct key *, char *, size_t);
> +static void request_key_auth_rcu_disposal(struct rcu_head *);
>
> /*
> * The request-key authorisation key type definition.
> @@ -115,6 +116,31 @@ static void free_request_key_auth(struct request_key_auth *rka)
> kfree(rka);
> }
>
> +/*
> + * Take a reference to the request-key authorisation payload so callers can
> + * drop authkey->sem before doing operations that may sleep.
> + */
> +struct request_key_auth *request_key_auth_get(struct key *authkey)
> +{
> + struct request_key_auth *rka;
> +
> + down_read(&authkey->sem);
> + rka = dereference_key_locked(authkey);
> + if (rka && !test_bit(KEY_FLAG_REVOKED, &authkey->flags))
> + refcount_inc(&rka->usage);
> + else
> + rka = NULL;
> + up_read(&authkey->sem);
> +
> + return rka;
> +}
Instead add fast-path return here. Then request_key_auth can be called
called in keyctl_instantiate_key_common and all that extra cruft in
this patch won't be needed.
> +
> +void request_key_auth_put(struct request_key_auth *rka)
> +{
> + if (rka && refcount_dec_and_test(&rka->usage))
> + call_rcu(&rka->rcu, request_key_auth_rcu_disposal);
> +}
> +
> /*
> * Dispose of the request_key_auth record under RCU conditions
> */
> @@ -136,8 +162,10 @@ static void request_key_auth_revoke(struct key *key)
> struct request_key_auth *rka = dereference_key_locked(key);
>
> kenter("{%d}", key->serial);
> + if (!rka)
> + return;
> rcu_assign_keypointer(key, NULL);
> - call_rcu(&rka->rcu, request_key_auth_rcu_disposal);
> + request_key_auth_put(rka);
> }
>
> /*
> @@ -150,7 +178,7 @@ static void request_key_auth_destroy(struct key *key)
> kenter("{%d}", key->serial);
> if (rka) {
> rcu_assign_keypointer(key, NULL);
> - call_rcu(&rka->rcu, request_key_auth_rcu_disposal);
> + request_key_auth_put(rka);
> }
> }
>
> @@ -174,6 +202,7 @@ struct key *request_key_auth_new(struct key *target, const char *op,
> rka = kzalloc_obj(*rka);
> if (!rka)
> goto error;
> + refcount_set(&rka->usage, 1);
> rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
> if (!rka->callout_info)
> goto error_free_rka;
> --
> 2.47.3
Looking forward for v2. These requests are for being able to make
reasonable review for the actual logical change. Now there's
that and "decorations".
Thanks!
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v10 6/9] selftests/landlock: add tests for quiet flag with fs rules
From: Tingmao Wang @ 2026-06-08 1:31 UTC (permalink / raw)
To: Justin Suess
Cc: Mickaël Salaün, Günther Noack, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <aiMciEe6zVnisbQJ@zenbox>
On 6/5/26 20:04, Justin Suess wrote:
> On Mon, Jun 01, 2026 at 01:00:40AM +0100, Tingmao Wang wrote:
>> [...]
>> + if (target->expect_ioctl_allowed)
>> + ASSERT_NE(EACCES, ret);
>> + else
>> + ASSERT_EQ(EACCES, ret);
> This doesn't compile.
>
> make: Entering directory '/home/justin/Code/linux-next/tools/testing/selftests'
> CC fs_test
> fs_test.c: In function ‘audit_quiet_layout1_test_body’:
> fs_test.c:8456:33: error: expected ‘}’ before ‘else’
> 8456 | else
> | ^~~~
> fs_test.c: At top level:
> fs_test.c:8474:1: error: expected identifier or ‘(’ before ‘}’ token
> 8474 | }
> | ^
> make[1]: *** [../lib.mk:225: /home/justin/Code/linux-next/.out-landlock_archlinux-base-devel/kselftest/landlock/fs_test] Error 1
>
> The ASSERT_* macros doen't properly handle braceless if statements...
>
> (easy to miss if you forget to recompile after clang format and/or
> checkpatch --fix...)
>
> Adding braces to this as with previous versions of this series should
> fix it.
>
> Justin
Thanks for spotting! I forgot and was wondering why I didn't fix this
checkpatch lint earlier, turns out there is a reason why :)
Will fix in next version, after Mickaël's review.
^ permalink raw reply
* Re: [PATCH] keys: allow request-key path to be configured via Kconfig
From: Serge E. Hallyn @ 2026-06-07 19:55 UTC (permalink / raw)
To: Gary Guo
Cc: David Howells, Jarkko Sakkinen, Paul Moore, James Morris,
Serge E. Hallyn, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260607134928.2832202-1-gary@kernel.org>
On Sun, Jun 07, 2026 at 02:49:27PM +0100, Gary Guo wrote:
> From: Gary Guo <gary@garyguo.net>
>
> Some Linux distributions (e.g. NixOS) does not have /sbin present, and they
> currently carry patches to replace /sbin/request-key to some other path.
>
> Follow the way modprobe handles this by making this a Kconfig option which
> defaults to the current /sbin/request-key.
>
> Also changed "char const" to "const char" as checkpatch complains
> otherwise.
>
> Link: https://github.com/NixOS/nixpkgs/blob/6b316287bae2ee04c9b93c8c858d930fd07d7338/pkgs/os-specific/linux/kernel/request-key-helper.patch
> Signed-off-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
> ---
> I did not update mentions of /sbin/request-key in documentation and
> elsewhere, as "/sbin/request-key" is concise while "request-key UMH" is
> more mouthful and less clear.
>
> Number of distros that doesn't have /sbin is limited so I think it wouldn't
> create much confusion. Similarly, there are a lot of places where
> /sbin/modprobe is mentioned despite it is technically configurable.
> ---
> security/keys/Kconfig | 9 +++++++++
> security/keys/request_key.c | 2 +-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> index f4510d8cb485..ee3c3d85fc03 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -40,6 +40,15 @@ config KEYS_REQUEST_CACHE
> key. Pathwalk will call multiple methods for each dentry traversed
> (permission, d_revalidate, lookup, getxattr, getacl, ...).
>
> +config REQUEST_KEY_PATH
> + string "Path to the request-key binary"
> + default "/sbin/request-key"
> + help
> + Path of the request-key usermode helper binary.
> +
> + This program is invoked by the kernel when the kernel is asked for
> + a key that it doesn't have immediately available.
> +
> config PERSISTENT_KEYRINGS
> bool "Enable register of persistent per-UID keyrings"
> help
> diff --git a/security/keys/request_key.c b/security/keys/request_key.c
> index a7673ad86d18..ac8f9d1a87ad 100644
> --- a/security/keys/request_key.c
> +++ b/security/keys/request_key.c
> @@ -117,7 +117,7 @@ static int call_usermodehelper_keys(const char *path, char **argv, char **envp,
> */
> static int call_sbin_request_key(struct key *authkey, void *aux)
> {
> - static char const request_key[] = "/sbin/request-key";
> + static const char request_key[] = CONFIG_REQUEST_KEY_PATH;
> struct request_key_auth *rka = get_request_key_auth(authkey);
> const struct cred *cred = current_cred();
> key_serial_t prkey, sskey;
>
> base-commit: 6e845bcb78c95af935094040bd4edc3c2b6dd784
> --
> 2.54.0
^ permalink raw reply
* [PATCH] keys: allow request-key path to be configured via Kconfig
From: Gary Guo @ 2026-06-07 13:49 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen, Paul Moore, James Morris,
Serge E. Hallyn
Cc: Gary Guo, keyrings, linux-security-module, linux-kernel
From: Gary Guo <gary@garyguo.net>
Some Linux distributions (e.g. NixOS) does not have /sbin present, and they
currently carry patches to replace /sbin/request-key to some other path.
Follow the way modprobe handles this by making this a Kconfig option which
defaults to the current /sbin/request-key.
Also changed "char const" to "const char" as checkpatch complains
otherwise.
Link: https://github.com/NixOS/nixpkgs/blob/6b316287bae2ee04c9b93c8c858d930fd07d7338/pkgs/os-specific/linux/kernel/request-key-helper.patch
Signed-off-by: Gary Guo <gary@garyguo.net>
---
I did not update mentions of /sbin/request-key in documentation and
elsewhere, as "/sbin/request-key" is concise while "request-key UMH" is
more mouthful and less clear.
Number of distros that doesn't have /sbin is limited so I think it wouldn't
create much confusion. Similarly, there are a lot of places where
/sbin/modprobe is mentioned despite it is technically configurable.
---
security/keys/Kconfig | 9 +++++++++
security/keys/request_key.c | 2 +-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index f4510d8cb485..ee3c3d85fc03 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -40,6 +40,15 @@ config KEYS_REQUEST_CACHE
key. Pathwalk will call multiple methods for each dentry traversed
(permission, d_revalidate, lookup, getxattr, getacl, ...).
+config REQUEST_KEY_PATH
+ string "Path to the request-key binary"
+ default "/sbin/request-key"
+ help
+ Path of the request-key usermode helper binary.
+
+ This program is invoked by the kernel when the kernel is asked for
+ a key that it doesn't have immediately available.
+
config PERSISTENT_KEYRINGS
bool "Enable register of persistent per-UID keyrings"
help
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index a7673ad86d18..ac8f9d1a87ad 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -117,7 +117,7 @@ static int call_usermodehelper_keys(const char *path, char **argv, char **envp,
*/
static int call_sbin_request_key(struct key *authkey, void *aux)
{
- static char const request_key[] = "/sbin/request-key";
+ static const char request_key[] = CONFIG_REQUEST_KEY_PATH;
struct request_key_auth *rka = get_request_key_auth(authkey);
const struct cred *cred = current_cred();
key_serial_t prkey, sskey;
base-commit: 6e845bcb78c95af935094040bd4edc3c2b6dd784
--
2.54.0
^ permalink raw reply related
* [PATCH next] keys: Replace strcpy(derived_buf, "AUTH_KEY") with strscpy(..., HASH_SIZE)
From: david.laight.linux @ 2026-06-06 20:26 UTC (permalink / raw)
To: Kees Cook, linux-hardening, Arnd Bergmann, keyrings,
linux-integrity, linux-kernel, linux-security-module
Cc: David Howells, James Morris, Jarkko Sakkinen, Mimi Zohar,
Paul Moore, Serge E. Hallyn, David Laight
From: David Laight <david.laight.linux@gmail.com>
derived_buf is guaranteed to be HASH_SIZE - and it is more than enough.
The strscpy() degenerates into an memcpy() (as did the strcpy()).
Do the same for the associated "ENC_KEY" copy.
Removes a possibly unbounded strcpy().
Signed-off-by: David Laight <david.laight.linux@gmail.com>
---
This is one of a group of patches that remove potentially unbounded
strcpy() calls.
They are mostly replaced by strscpy() or, when strlen() has just been
called, with memcpy() (usually including the '\0').
Calls with copy string literals into arrays are left unchanged.
They are safe and easily detected as such.
The changes were made by getting the compiler to detect the calls and
then fixing the code by hand.
Note that all the changes are only compile tested.
Some Makefiles were changed to allow files to contain strcpy().
As well as 'difficult to fix' files, this included 'show' functions
as they really need to use sysfs_emit() or seq_printf().
All the patches are being sent individually to avoid very long cc lists.
Apologies for the terse commit messages and likely unexpected tags.
(There are about 100 patches in total.)
security/keys/encrypted-keys/encrypted.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 56b531587a1e..59cb77b237b3 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -343,9 +343,9 @@ static int get_derived_key(u8 *derived_key, enum derived_key_type key_type,
return -ENOMEM;
if (key_type)
- strcpy(derived_buf, "AUTH_KEY");
+ strscpy(derived_buf, "AUTH_KEY", HASH_SIZE);
else
- strcpy(derived_buf, "ENC_KEY");
+ strscpy(derived_buf, "ENC_KEY", HASH_SIZE);
memcpy(derived_buf + strlen(derived_buf) + 1, master_key,
master_keylen);
--
2.39.5
^ permalink raw reply related
* Re: [PATCH] apparmor: Constify 'nulldfa_src' and 'stacksplitdfa_src' arrays
From: Len Bao @ 2026-06-06 17:18 UTC (permalink / raw)
To: John Johansen, Paul Moore, James Morris, Serge E. Hallyn,
Kees Cook
Cc: Len Bao, apparmor, linux-security-module, linux-hardening,
linux-kernel
In-Reply-To: <20260524113412.48050-1-len.bao@gmx.us>
Hi,
On Sun, May 24, 2026 at 11:34:11AM +0000, Len Bao wrote:
> The 'nulldfa_src' and 'stacksplitdfa_src' arrays are initialized in
> their declarations and never changed. So, constify them to reduce the
> attack surface.
>
> To make this possible, it is also necessary to change the 'unpack_table'
> and 'aa_dfa_unpack' function prototypes to pass, as a first argument, a
> pointer to a 'const' blob. At the same type, define the blob exact
> pointer type (pointer to const char) since all the calls to the
> mentioned functions use this same type.
>
> Before the patch (size lsm.o):
>
> text data bss dec hex
> 128768 28028 704 157500 2673c
>
> After the patch (size lsm.o):
>
> text data bss dec hex
> 131264 25532 704 157500 2673c
>
> Signed-off-by: Len Bao <len.bao@gmx.us>
> ---
Friendly ping.
Any comments are welcome.
Regards,
Len
> security/apparmor/include/match.h | 2 +-
> security/apparmor/lsm.c | 4 ++--
> security/apparmor/match.c | 6 +++---
> 3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/security/apparmor/include/match.h b/security/apparmor/include/match.h
> index 7accb1c39..4a92cd044 100644
> --- a/security/apparmor/include/match.h
> +++ b/security/apparmor/include/match.h
> @@ -125,7 +125,7 @@ static inline size_t table_size(size_t len, size_t el_size)
>
> #define aa_state_t unsigned int
>
> -struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags);
> +struct aa_dfa *aa_dfa_unpack(const char *blob, size_t size, int flags);
> aa_state_t aa_dfa_match_len(struct aa_dfa *dfa, aa_state_t start,
> const char *str, int len);
> aa_state_t aa_dfa_match(struct aa_dfa *dfa, aa_state_t start,
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index 3491e9f60..3f995b6a7 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -2432,12 +2432,12 @@ static int __init apparmor_nf_ip_init(void)
> }
> #endif
>
> -static char nulldfa_src[] __aligned(8) = {
> +static const char nulldfa_src[] __aligned(8) = {
> #include "nulldfa.in"
> };
> static struct aa_dfa *nulldfa;
>
> -static char stacksplitdfa_src[] __aligned(8) = {
> +static const char stacksplitdfa_src[] __aligned(8) = {
> #include "stacksplitdfa.in"
> };
> struct aa_dfa *stacksplitdfa;
> diff --git a/security/apparmor/match.c b/security/apparmor/match.c
> index 3a2c6cf02..c6f7bea1e 100644
> --- a/security/apparmor/match.c
> +++ b/security/apparmor/match.c
> @@ -31,7 +31,7 @@
> *
> * NOTE: must be freed by kvfree (not kfree)
> */
> -static struct table_header *unpack_table(char *blob, size_t bsize)
> +static struct table_header *unpack_table(const char *blob, size_t bsize)
> {
> struct table_header *table = NULL;
> struct table_header th;
> @@ -311,11 +311,11 @@ static struct table_header *remap_data16_to_data32(struct table_header *old)
> *
> * Returns: an unpacked dfa ready for matching or ERR_PTR on failure
> */
> -struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags)
> +struct aa_dfa *aa_dfa_unpack(const char *blob, size_t size, int flags)
> {
> int hsize;
> int error = -ENOMEM;
> - char *data = blob;
> + const char *data = blob;
> struct table_header *table = NULL;
> struct aa_dfa *dfa = kzalloc_obj(struct aa_dfa);
> if (!dfa)
> --
> 2.43.0
>
^ permalink raw reply
* Re: [PATCH v4 2/7] landlock: Add UDP connect() access control
From: Matthieu Buffet @ 2026-06-06 17:11 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Mikhail Ivanov,
konstantin.meskhidze, Tingmao Wang
In-Reply-To: <20260522.reif5feiX0Ce@digikod.net>
On 5/22/2026 11:10 PM, Mickaël Salaün wrote:
>> + * Note: socket is not locked, so another thread could do an
>> + * explicit bind(!=0) on this socket, changing inet_num to non-zero
>> + * after we read it, but this would only have us enforce an
>> + * additional bind(0) access check and would not bypass policy.
>> + */
>> + if (inet_sk(sock->sk)->inet_num != 0)
>
> There is still a race condition, this should fix it:
>
> *
> * Hold the socket lock around the inet_num read to exclude
> * udp_lib_get_port()'s transient inet_num = snum write that is reverted
> * to 0 on a failing reuseport bind.
> */
> if (inet_sk(sock->sk)->inet_num != 0)
> slow = lock_sock_fast(sock->sk);
> num = inet_sk(sock->sk)->inet_num;
> unlock_sock_fast(sock->sk, slow);
> if (num != 0)
> return 0;
Oh wow, nice catch. The transient write in udp_lib_get_port() indeed
requires locking, a READ_ONCE would not even be enough.
>> + port0.ss_family = sock->sk->__sk_common.skc_family;
>
> Looking at net/, __sk_common.skc_family (and other __sk_common.* fields)
> should be replaced by sk_family (see #define in include/net/sock.h).
> Same for all other __sk_common.
>
> In fact, it should be READ_ONCE(sock->sk->sk_family) for consistency
> with the net/ code and because the socket are not locked when the LSM
> hooks are called. I realized that the existing Landlock code have the
> same issue... Could you please add a new patch (to be backported) to
> always use this pattern?
Indeed, setsockopt(IPV6_ADDRFORM) could mess with these too, I can send
a separate fix patch for that.
Thanks!
--
Matthieu
^ permalink raw reply
* Re: [PATCH v4 0/7] landlock: Add UDP access control support
From: Matthieu Buffet @ 2026-06-06 17:01 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Mikhail Ivanov,
konstantin.meskhidze, Tingmao Wang
In-Reply-To: <20260522.saibiuZ5ailo@digikod.net>
Hi Mickaël, Günther,
Thank you both for your reviews, I will follow up with these last fixes
in a v5.
On 5/22/2026 11:08 PM, Mickaël Salaün wrote:
>> I'm just not super happy about the clarity of logs generated for denied
>> autobinds ("domain=xxxxxx blockers=net.bind_udp"), due to the fact that
>> addresses and ports are currently only logged if they are non-0. A later
>> (coordinated LSM-wide) patch could improve readability by replacing != 0
>> checks with new booleans in struct lsm_network_audit.
>
> Do you plan to send such patch after this series? I guess we could add
> has_{port,addr} fields to lsm_network_audit and handle AF_UNSPEC too?
I have not come up with anything better than adding boolean fields, so
if you're in, I will draft a proposition along these lines (and cc: LSM
subsystem maintainers to synchronize the change across LSMs, I guess)
>> I'm also not
>> exactly happy with the integration in existing TCP selftests, but
>> refactoring them has already been discussed earlier.
>
> Can you remind us what was your concern and the potential fix?
Regarding TCP selftests, I was referencing that discussion about
readability (length, and usage of conditionals in what are already test
variants) :
https://lore.kernel.org/linux-security-module/22dcebae-dc5d-0bf1-c686-d2f444558106@huawei-partners.com/
Nothing blocking, refactoring can be done when things are less busy.
--
Matthieu
^ permalink raw reply
* Re: [PATCH v4 2/7] landlock: Add UDP connect() access control
From: Matthieu Buffet @ 2026-06-06 17:04 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, Mikhail Ivanov,
konstantin.meskhidze, Tingmao Wang
In-Reply-To: <20260522.AhMei2meelee@digikod.net>
On 5/22/2026 11:18 PM, Mickaël Salaün wrote:
>> + /*
>> + * Construct a struct sockaddr* with port 0 to pretend the
>> + * process tried to bind() on that address.
>> + */
>> + port0.ss_family = sock->sk->__sk_common.skc_family;
>> + switch (port0.ss_family) {
>> + case AF_INET: {
>> + ((struct sockaddr_in *)&port0)->sin_port = 0;
>
> Why is this useful? The struct is already initialized to 0.
Indeed, I will only leave a proper initialization of the necessary
fields instead.
--
Matthieu
^ permalink raw reply
* [PATCH] cred: prevent slab cache merging for cred_jar
From: Mohammed EL Kadiri @ 2026-06-06 14:25 UTC (permalink / raw)
To: Paul Moore
Cc: Serge Hallyn, Vlastimil Babka, Kees Cook, linux-security-module,
linux-hardening, linux-kernel, Mohammed EL Kadiri
The cred_jar slab cache holds struct cred objects, which contain
process credentials: uid, gid, euid, egid, and capability sets.
Overwriting any of these fields is sufficient for privilege escalation.
On a default Ubuntu 6.17.0-23-generic system, cred_jar (named "cred"
in sysfs) has 2 aliases, meaning 2 unrelated object types share its
slab pages (object_size=184, objs_per_slab=42).
Cross-cache heap exploitation relies on slab cache merging to achieve
type confusion between unrelated kernel objects. CVE-2022-29582
demonstrates this technique: an io_uring use-after-free is leveraged
across cache boundaries through page-level reallocation, ultimately
achieving root. struct cred is a primary target in this class of
attacks due to the direct privilege escalation that results from
corrupting any of its identity or capability fields.
Add SLAB_NO_MERGE to ensure cred_jar receives dedicated slab pages,
so that freed credential slots can only be reallocated as struct cred
objects. The memory overhead is minimal: one struct cred exists per
task, and with 42 objects per slab page, the cost of dedicated pages
is negligible. There is zero performance impact on the allocation
hot path.
This follows the precedent set by skbuff_head_cache (net/core/skbuff.c)
and key_jar (security/keys/key.c) which use SLAB_NO_MERGE for similar
isolation requirements.
Signed-off-by: Mohammed EL Kadiri <med08elkadiri@gmail.com>
---
kernel/cred.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/cred.c b/kernel/cred.c
index 9676965c0981..0e4ee60a5acd 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -557,7 +557,7 @@ void __init cred_init(void)
{
/* allocate a slab in which we can store credentials */
cred_jar = KMEM_CACHE(cred,
- SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT);
+ SLAB_HWCACHE_ALIGN | SLAB_PANIC | SLAB_ACCOUNT | SLAB_NO_MERGE);
}
/**
--
2.43.0
^ permalink raw reply related
* Re: [net v3] netlabel: validate unlabeled address and mask attribute lengths
From: patchwork-bot+netdevbpf @ 2026-06-06 2:10 UTC (permalink / raw)
To: Chenguang Zhao
Cc: paul, davem, edumazet, kuba, pabeni, horms, netdev,
linux-security-module
In-Reply-To: <20260603011353.101776-1-zhaochenguang@kylinos.cn>
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Wed, 3 Jun 2026 09:13:53 +0800 you wrote:
> netlbl_unlabel_addrinfo_get() used the address attribute length to
> determine whether the attribute data could be read as an IPv4 or IPv6
> address, but did not independently validate the corresponding mask
> attribute length. A crafted Generic Netlink request could therefore
> provide a valid IPv4/IPv6 address attribute with a shorter mask
> attribute, which would later be read as a full struct in_addr or
> struct in6_addr.
>
> [...]
Here is the summary with links:
- [net,v3] netlabel: validate unlabeled address and mask attribute lengths
https://git.kernel.org/netdev/net/c/9772589b57e4
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH v10 6/9] selftests/landlock: add tests for quiet flag with fs rules
From: Justin Suess @ 2026-06-05 19:04 UTC (permalink / raw)
To: Tingmao Wang
Cc: Mickaël Salaün, Günther Noack, Jan Kara,
Abhinav Saxena, linux-security-module
In-Reply-To: <9208d19c3aeba778f317defcfee9c62c44600e3b.1780272022.git.m@maowtm.org>
On Mon, Jun 01, 2026 at 01:00:40AM +0100, Tingmao Wang wrote:
> Test various interactions of the quiet flag with filesystem rules:
> - Non-optional access (tested with open and rename).
> - Optional access (tested with truncate and ioctl).
> - Behaviour around mounts matches with normal Landlock rules.
> - Behaviour around disconnected directories matches with normal Landlock
> rules (test expected behaviour of 9a868cdbe66a ("landlock: Fix handling of
> disconnected directories") applied to the collected quiet flag).
> - Multiple layers works as expected.
>
> Assisted-by: GitHub Copilot:claude-opus-4.6 copilot-review
> Signed-off-by: Tingmao Wang <m@maowtm.org>
> ---
>
> Changes in v10:
> - Fix grammar in some comments
> - if brackets
>
> Changes in v8:
> - Rebase, resolve conflict, then clang-format
> - Remove previously added comment about domain allocation record leakage -
> this is now documented properly by 239fd9a6f948 ("selftests/landlock:
> Drain stale audit records on init")
> - Fix missing EXPECT_EQ on audit_count_records() return value
>
> Changes in v6:
> - Change quiet bool argument of add_path_beneath into a __u32 flags
> (suggested by Justin Suess)
> - Rename quiet_behind_mountpoint_ignored_disconnected to
> quiet_behind_mountpoint_disconnected and fix test due to disconnected
> directory handling changes
>
> Changes in v5:
> - Add quiet_two_layers_different_handled_{1,2,3} variants.
>
> Changes in v3:
> - New patch
>
> tools/testing/selftests/landlock/fs_test.c | 2447 +++++++++++++++++++-
> 1 file changed, 2438 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 10d9355ade5f..5f5d75fabe07 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -720,7 +720,7 @@ TEST_F_FORK(layout1, rule_with_unhandled_access)
>
> static void add_path_beneath(struct __test_metadata *const _metadata,
> const int ruleset_fd, const __u64 allowed_access,
> - const char *const path)
> + const char *const path, __u32 flags)
> {
> struct landlock_path_beneath_attr path_beneath = {
> .allowed_access = allowed_access,
> @@ -733,7 +733,7 @@ static void add_path_beneath(struct __test_metadata *const _metadata,
> strerror(errno));
> }
> ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
> - &path_beneath, 0))
> + &path_beneath, flags))
> {
> TH_LOG("Failed to update the ruleset with \"%s\": %s", path,
> strerror(errno));
> @@ -780,7 +780,7 @@ static int create_ruleset(struct __test_metadata *const _metadata,
> continue;
>
> add_path_beneath(_metadata, ruleset_fd, rules[i].access,
> - rules[i].path);
> + rules[i].path, 0);
> }
> return ruleset_fd;
> }
> @@ -1310,7 +1310,7 @@ TEST_F_FORK(layout1, inherit_subset)
> * ANDed with the previous ones.
> */
> add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_WRITE_FILE,
> - dir_s1d2);
> + dir_s1d2, 0);
> /*
> * According to ruleset_fd, dir_s1d2 should now have the
> * LANDLOCK_ACCESS_FS_READ_FILE and LANDLOCK_ACCESS_FS_WRITE_FILE
> @@ -1342,7 +1342,7 @@ TEST_F_FORK(layout1, inherit_subset)
> * Try to get more privileges by adding new access rights to the parent
> * directory: dir_s1d1.
> */
> - add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1);
> + add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, 0);
> enforce_ruleset(_metadata, ruleset_fd);
>
> /* Same tests and results as above. */
> @@ -1365,7 +1365,7 @@ TEST_F_FORK(layout1, inherit_subset)
> * that there was no rule tied to it before.
> */
> add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_WRITE_FILE,
> - dir_s1d3);
> + dir_s1d3, 0);
> enforce_ruleset(_metadata, ruleset_fd);
> ASSERT_EQ(0, close(ruleset_fd));
>
> @@ -1417,7 +1417,7 @@ TEST_F_FORK(layout1, inherit_superset)
> add_path_beneath(_metadata, ruleset_fd,
> LANDLOCK_ACCESS_FS_READ_FILE |
> LANDLOCK_ACCESS_FS_READ_DIR,
> - dir_s1d2);
> + dir_s1d2, 0);
> enforce_ruleset(_metadata, ruleset_fd);
> EXPECT_EQ(0, close(ruleset_fd));
>
> @@ -3970,7 +3970,7 @@ static int ioctl_error(struct __test_metadata *const _metadata, int fd,
> unsigned int cmd)
> {
> char buf[128]; /* sufficiently large */
> - int res, stdinbak_fd;
> + int res, stdinbak_fd, err;
>
> /*
> * Depending on the IOCTL command, parts of the zeroed-out buffer might
> @@ -3985,13 +3985,14 @@ static int ioctl_error(struct __test_metadata *const _metadata, int fd,
> /* Invokes the IOCTL with a zeroed-out buffer. */
> bzero(&buf, sizeof(buf));
> res = ioctl(fd, cmd, &buf);
> + err = errno;
>
> /* Restores the old FD 0 and closes the backup FD. */
> ASSERT_EQ(0, dup2(stdinbak_fd, 0));
> ASSERT_EQ(0, close(stdinbak_fd));
>
> if (res < 0)
> - return errno;
> + return err;
>
> return 0;
> }
> @@ -4789,6 +4790,7 @@ FIXTURE(layout1_bind) {};
>
> static const char bind_dir_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3";
> static const char bind_file1_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3/f1";
> +static const char bind_file2_s1d3[] = TMP_DIR "/s2d1/s2d2/s1d3/f2";
>
> /* Move targets for disconnected path tests. */
> static const char dir_s4d1[] = TMP_DIR "/s4d1";
> @@ -7764,4 +7766,2431 @@ TEST_F(audit_layout1, mount)
> EXPECT_EQ(1, records.domain);
> }
>
> +static bool debug_quiet_tests;
> +
> +FIXTURE(audit_quiet_layout1)
> +{
> + struct audit_filter audit_filter;
> + int audit_fd;
> +};
> +
> +FIXTURE_SETUP(audit_quiet_layout1)
> +{
> + prepare_layout(_metadata);
> + create_layout1(_metadata);
> +
> + set_cap(_metadata, CAP_AUDIT_CONTROL);
> + self->audit_fd = audit_init_with_exe_filter(&self->audit_filter);
> + EXPECT_LE(0, self->audit_fd);
> + clear_cap(_metadata, CAP_AUDIT_CONTROL);
> +
> + if (getenv("DEBUG_QUIET_TESTS"))
> + debug_quiet_tests = true;
> +}
> +
> +FIXTURE_TEARDOWN_PARENT(audit_quiet_layout1)
> +{
> + remove_layout1(_metadata);
> + cleanup_layout(_metadata);
> +
> + set_cap(_metadata, CAP_AUDIT_CONTROL);
> + EXPECT_EQ(0, audit_cleanup(-1, NULL));
> + clear_cap(_metadata, CAP_AUDIT_CONTROL);
> +}
> +
> +struct a_rule {
> + const char *path;
> + __u64 access;
> + bool quiet;
> +};
> +
> +struct a_layer {
> + __u64 handled_access_fs;
> + __u64 quiet_access_fs;
> + struct a_rule rules[6];
> + __u64 restrict_flags;
> +};
> +
> +struct a_target {
> + /* File/dir to try open. */
> + const char *target;
> + /* Open mode (one of O_RDONLY, O_WRONLY, or O_RDWR). */
> + int open_mode;
> + /* Should open succeed? */
> + bool expect_open_success;
> + /* If open fails, whether to expect an audit log for read. */
> + bool audit_read_blocked;
> + /* If open fails, whether to expect an audit log for write. */
> + bool audit_write_blocked;
> + /* If ftruncate() is expected to be allowed. */
> + bool expect_truncate_success;
> + /* If ftruncate fails, whether to expect an audit log. */
> + bool audit_truncate;
> + /*
> + * If ioctl() is expected to be allowed (ioctl not attempted if
> + * neither this nor expect_ioctl_denied is set).
> + */
> + bool expect_ioctl_allowed;
> + /* If ioctl() is expected to be denied. */
> + bool expect_ioctl_denied;
> + /* If ioctl fails, whether to expect an audit log. */
> + bool audit_ioctl;
> +};
> +
> +#define AUDIT_QUIET_MAX_TARGETS 10
> +
> +FIXTURE_VARIANT(audit_quiet_layout1)
> +{
> + struct a_layer layers[3];
> + struct a_target targets[AUDIT_QUIET_MAX_TARGETS];
> +};
> +
> +#define FS_R LANDLOCK_ACCESS_FS_READ_FILE
> +#define FS_W LANDLOCK_ACCESS_FS_WRITE_FILE
> +#define FS_TRUNC LANDLOCK_ACCESS_FS_TRUNCATE
> +#define FS_IOCTL LANDLOCK_ACCESS_FS_IOCTL_DEV
> +
> +static int sprint_access_bits(char *buf, size_t buflen, __u64 access)
> +{
> + size_t offset = 0;
> +
> + if (buflen < strlen("rwti make_reg remove_file refer") + 1)
> + abort();
> +
> + buf[0] = '\0';
> + if (access & FS_R)
> + offset += snprintf(buf + offset, buflen - offset, "r");
> + if (access & FS_W)
> + offset += snprintf(buf + offset, buflen - offset, "w");
> + if (access & FS_TRUNC)
> + offset += snprintf(buf + offset, buflen - offset, "t");
> + if (access & FS_IOCTL)
> + offset += snprintf(buf + offset, buflen - offset, "i");
> + if (access & LANDLOCK_ACCESS_FS_MAKE_REG)
> + offset += snprintf(buf + offset, buflen - offset, ",make_reg");
> + if (access & LANDLOCK_ACCESS_FS_REMOVE_FILE)
> + offset +=
> + snprintf(buf + offset, buflen - offset, ",remove_file");
> + if (access & LANDLOCK_ACCESS_FS_REFER)
> + offset += snprintf(buf + offset, buflen - offset, ",refer");
> +
> + if (buf[0] == ',') {
> + offset--;
> + memmove(buf, buf + 1, offset);
> + buf[offset] = '\0';
> + }
> +
> + return offset;
> +}
> +
> +static int apply_a_layer(struct __test_metadata *const _metadata,
> + const struct a_layer *l)
> +{
> + struct landlock_ruleset_attr rs_attr = {
> + .handled_access_fs = l->handled_access_fs,
> + .quiet_access_fs = l->quiet_access_fs,
> + };
> + int rs_fd;
> + int i;
> + const struct a_rule *r;
> + char handled_access_s[33], quiet_access_s[33], rule_access_s[33];
> +
> + if (!l->handled_access_fs)
> + return 0;
> +
> + rs_fd = landlock_create_ruleset(&rs_attr, sizeof(rs_attr), 0);
> + ASSERT_LE(0, rs_fd);
> +
> + for (i = 0; i < ARRAY_SIZE(l->rules); i++) {
> + r = &l->rules[i];
> + if (!r->path)
> + continue;
> +
> + add_path_beneath(_metadata, rs_fd, r->access, r->path,
> + r->quiet ? LANDLOCK_ADD_RULE_QUIET : 0);
> + }
> +
> + ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
> + ASSERT_EQ(0, landlock_restrict_self(rs_fd, l->restrict_flags))
> + {
> + TH_LOG("Failed to enforce ruleset: %s", strerror(errno));
> + }
> + ASSERT_EQ(0, close(rs_fd));
> +
> + if (debug_quiet_tests) {
> + sprint_access_bits(handled_access_s, sizeof(handled_access_s),
> + l->handled_access_fs);
> + sprint_access_bits(quiet_access_s, sizeof(quiet_access_s),
> + l->quiet_access_fs);
> + TH_LOG("applied layer: handled=%s quiet=%s restrict_flags=0x%llx",
> + handled_access_s, quiet_access_s,
> + (unsigned long long)l->restrict_flags);
> + for (i = 0; i < ARRAY_SIZE(l->rules); i++) {
> + r = &l->rules[i];
> + if (!r->path)
> + continue;
> +
> + sprint_access_bits(rule_access_s, sizeof(rule_access_s),
> + r->access);
> + TH_LOG(" rule[%d]: path=%s access=%s quiet=%d", i,
> + r->path, rule_access_s, r->quiet);
> + }
> + }
> + return 0;
> +}
> +
> +void audit_quiet_layout1_test_body(struct __test_metadata *const _metadata,
> + FIXTURE_DATA(audit_quiet_layout1) * self,
> + const struct a_target *targets)
> +{
> + struct audit_records records = {};
> + int i;
> + const struct a_target *target;
> + int fd = -1;
> + int open_mode;
> + int ret;
> + bool expect_audit;
> + const char *blocker;
> +
> + for (i = 0; i < AUDIT_QUIET_MAX_TARGETS; i++) {
> + target = &targets[i];
> + if (!target->target)
> + continue;
> +
> + open_mode = target->open_mode & (O_RDONLY | O_WRONLY | O_RDWR);
> +
> + EXPECT_TRUE(open_mode == O_RDONLY || open_mode == O_WRONLY ||
> + open_mode == O_RDWR);
> +
> + if (target->expect_open_success) {
> + EXPECT_FALSE(target->audit_read_blocked);
> + EXPECT_FALSE(target->audit_write_blocked);
> + }
> + if (target->expect_truncate_success)
> + EXPECT_TRUE(target->expect_open_success &&
> + !target->audit_truncate);
> +
> + if (debug_quiet_tests)
> + TH_LOG("Try open \"%s\" with %s%s", target->target,
> + open_mode != O_WRONLY ? "r" : "",
> + open_mode != O_RDONLY ? "w" : "");
> +
> + fd = openat(AT_FDCWD, target->target, open_mode | O_CLOEXEC);
> + if (target->expect_open_success) {
> + ASSERT_LE(0, fd)
> + {
> + TH_LOG("Failed to open \"%s\": %s",
> + target->target, strerror(errno));
> + };
> + } else {
> + ASSERT_EQ(-1, fd);
> + ASSERT_EQ(EACCES, errno);
> + }
> +
> + expect_audit = true;
> +
> + if (target->audit_read_blocked && target->audit_write_blocked)
> + blocker = "fs\\.write_file,fs\\.read_file";
> + else if (target->audit_read_blocked)
> + blocker = "fs\\.read_file";
> + else if (target->audit_write_blocked)
> + blocker = "fs\\.write_file";
> + else
> + expect_audit = false;
> +
> + if (expect_audit)
> + ASSERT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
> + blocker, target->target));
> +
> + /* Check that we see no (other) logs. */
> + EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> + ASSERT_EQ(0, records.access);
> +
> + if (target->expect_open_success && fd >= 0) {
> + if (debug_quiet_tests)
> + TH_LOG("Try ftruncate \"%s\"", target->target);
> +
> + ret = ftruncate(fd, 0);
> + if (target->expect_truncate_success) {
> + ASSERT_EQ(0, ret);
> + } else {
> + ASSERT_EQ(-1, ret);
> + if (open_mode != O_RDONLY)
> + ASSERT_EQ(EACCES, errno);
> + }
> +
> + if (target->audit_truncate)
> + ASSERT_EQ(0, matches_log_fs(_metadata,
> + self->audit_fd,
> + "fs\\.truncate",
> + target->target));
> +
> + if (target->expect_ioctl_allowed ||
> + target->expect_ioctl_denied) {
> + if (debug_quiet_tests)
> + TH_LOG("Try ioctl FIONREAD on \"%s\"",
> + target->target);
> +
> + ret = ioctl_error(_metadata, fd, FIONREAD);
> + if (target->expect_ioctl_allowed)
> + ASSERT_NE(EACCES, ret);
> + else
> + ASSERT_EQ(EACCES, ret);
This doesn't compile.
make: Entering directory '/home/justin/Code/linux-next/tools/testing/selftests'
CC fs_test
fs_test.c: In function ‘audit_quiet_layout1_test_body’:
fs_test.c:8456:33: error: expected ‘}’ before ‘else’
8456 | else
| ^~~~
fs_test.c: At top level:
fs_test.c:8474:1: error: expected identifier or ‘(’ before ‘}’ token
8474 | }
| ^
make[1]: *** [../lib.mk:225: /home/justin/Code/linux-next/.out-landlock_archlinux-base-devel/kselftest/landlock/fs_test] Error 1
The ASSERT_* macros doen't properly handle braceless if statements...
(easy to miss if you forget to recompile after clang format and/or
checkpatch --fix...)
Adding braces to this as with previous versions of this series should
fix it.
Justin
^ permalink raw reply
* Re: -next status as at v7.1-rc6
From: James Bottomley @ 2026-06-05 18:29 UTC (permalink / raw)
To: Serge E. Hallyn, Linus Torvalds
Cc: Paul Moore, linux-security-module, Mark Brown, Blaise Boscaccy,
Alexei Starovoitov, linux-next, linux-kernel
In-Reply-To: <aiLS4wOtpqrpw4pA@mail.hallyn.com>
On Fri, 2026-06-05 at 08:45 -0500, Serge E. Hallyn wrote:
> On Thu, Jun 04, 2026 at 04:18:46PM -0700, Linus Torvalds wrote:
> > On Thu, 4 Jun 2026 at 15:23, Paul Moore <paul@paul-moore.com>
> > wrote:
> > >
> > > While you didn't reply to any of my comments explaining how
> > > Hornet works, specifically how it ties into the kernel, I'm
> > > assuming you've read the overview. Can you help those of us in
> > > the LSM space understand why a BPF dev's NACK on code that lives
> > > strictly under security/ is sufficient grounds to reject an LSM
> > > patch?
> >
> > Honestly, I'm not competent to make a judgment call between two
> > different models for hash chain verification, so I basically *have*
> > to go by maintainer opinions.
> >
> > And the discussions I have been cc'd on have not been what I'd call
> > enlightening.
> >
> > But people have pointed out that the LSM code mucks around with bpf
> > internals, and those NAK's have had reasons for them.
> >
> > And honestly, I don't understand *why* Hornet does what it does -
> > and does it in ways that obviously annoy the bpf people. There is
> > no *reason* to look at the bpf maps that I can see, and from my
> > understanding of Alexei's arguments (which may be lacking), the
> > fact that Hornet does that is the main reason for the NAK.
>
> The two most useful threads I believe were from a year ago,
> 20250502184421.1424368-1-bboscaccy@linux.microsoft.com
> and
> 20250528215037.2081066-1-bboscaccy@linux.microsoft.com
> which includes the proposal by the BPF side:
> https://lore.kernel.org/linux-security-module/CACYkzJ6VQUExfyt0=-FmXz46GHJh3d=FXh5j4KfexcEFbHV-vg@mail.gmail.com/
>
> There were 2 or three objections from each side iiuc, but the main
> ones that stuck in my mind were
>
> 1. whether it is ok to rely on a signed userspace bpf verifier
> program to
> verify the signature.
> 2. objection by James Bottomley
>
> (2f71d6c03698eb17d51f7247efde777627ee578a.camel@HansenPartnership.com
> )
> about the verifiability of the hash chain link.
My objection to the upstream signature scheme in that email was that if
you hand me a lskel consisting of a loader and a map and ask me to sign
it, how do I know the loader actually verifies the hash of the map
without having to disassemble and analyze it.
I think the second patch set (not yet upstream) makes that task easier
because it now calls out to the bpf_loader_verify_metadata() kfunc to
verify the map. That means I can run the loader in a sandbox and see
if it makes the call, which is way easier than having to disassemble
and analyse its byte code. Note: it's still not as easy as the hornet
method of simply having the map hash as a signed attribute I can
extract and verify, though. However, the required task has gone from
pretty impossible to doable.
Regards,
James
^ permalink raw reply
* Re: [PATCH v2 1/9] security: add LSM blob and hooks for namespaces
From: Paul Moore @ 2026-06-05 18:08 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Christian Brauner, Günther Noack, Serge E . Hallyn,
Daniel Durning, Jonathan Corbet, Justin Suess, Lennart Poettering,
Mikhail Ivanov, Nicolas Bouchinet, Shervin Oloumi, Tingmao Wang,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module
In-Reply-To: <20260605.ieZ1hei8Ahna@digikod.net>
On Fri, Jun 5, 2026 at 11:07 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> Paul, could you please take a look at this patch and the next one? I'd
> like to push it to linux-next to get more feedback.
I saw the patches Mickaël, they are in the queue and will be reviewed
in due time. Considering the delay in getting this revision out after
my last review, another week or two shouldn't pose a problem.
--
paul-moore.com
^ 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