* [PATCH 1/2] treewide: Add a flag to detect the Apple T2 chip
From: Atharva Tiwari @ 2026-07-19 18:03 UTC (permalink / raw)
Cc: Atharva Tiwari, Bjorn Helgaas, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Andreas Noever,
Mika Westerberg, Yehezkel Bernat, Hans de Goede,
Ilpo Järvinen, Jarkko Sakkinen, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris,
Serge E. Hallyn, linux-pci, linux-kernel, linux-usb,
platform-driver-x86, linux-integrity, keyrings,
linux-security-module
In-Reply-To: <20260719180308.1398-1-atharvatiwarilinuxdev@gmail.com>
Add a flag to detect Apple T2 chips on Intel Macs.
Cache the result to avoid repeated checks. That will
be used in upcoming patches.
Signed-off-by: Atharva Tiwari <atharvatiwarilinuxdev@gmail.com>
---
arch/x86/pci/fixup.c | 10 +++++
include/linux/platform_data/x86/apple.h | 5 +++
security/integrity/platform_certs/load_uefi.c | 38 ++++---------------
3 files changed, 23 insertions(+), 30 deletions(-)
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index b301c6c8df75..fbd204ca4c6d 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -7,6 +7,7 @@
#include <linux/delay.h>
#include <linux/dmi.h>
#include <linux/pci.h>
+#include <linux/platform_data/x86/apple.h>
#include <linux/suspend.h>
#include <linux/vgaarb.h>
#include <asm/amd/node.h>
@@ -995,6 +996,15 @@ static void asus_disable_nvme_d3cold(struct pci_dev *pdev)
}
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x9a09, asus_disable_nvme_d3cold);
+bool has_t2_chip;
+EXPORT_SYMBOL(has_t2_chip);
+
+static void apple_has_t2_chip(struct pci_dev *pdev)
+{
+ has_t2_chip = true;
+}
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_APPLE, 0x1801, apple_has_t2_chip);
+
#ifdef CONFIG_SUSPEND
/*
* Root Ports on some AMD SoCs advertise PME_Support for D3hot and D3cold, but
diff --git a/include/linux/platform_data/x86/apple.h b/include/linux/platform_data/x86/apple.h
index 079e816c3c21..a2ab63bd1eac 100644
--- a/include/linux/platform_data/x86/apple.h
+++ b/include/linux/platform_data/x86/apple.h
@@ -6,8 +6,13 @@
* x86_apple_machine - whether the machine is an x86 Apple Macintosh
*/
extern bool x86_apple_machine;
+/**
+ * has_t2_chip - whether the machine has the Apple T2 chip
+ */
+extern bool has_t2_chip;
#else
#define x86_apple_machine false
+#define has_t2_chip false
#endif
#endif
diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index c0d6948446c3..da57ac322e72 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -3,42 +3,16 @@
#include <linux/kernel.h>
#include <linux/sched.h>
#include <linux/cred.h>
-#include <linux/dmi.h>
#include <linux/err.h>
#include <linux/efi.h>
#include <linux/slab.h>
#include <linux/ima.h>
+#include <linux/platform_data/x86/apple.h>
#include <keys/asymmetric-type.h>
#include <keys/system_keyring.h>
#include "../integrity.h"
#include "keyring_handler.h"
-/*
- * On T2 Macs reading the db and dbx efi variables to load UEFI Secure Boot
- * certificates causes occurrence of a page fault in Apple's firmware and
- * a crash disabling EFI runtime services. The following quirk skips reading
- * these variables.
- */
-static const struct dmi_system_id uefi_skip_cert[] = {
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,1") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,2") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,3") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro15,4") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro16,1") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro16,2") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro16,3") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookPro16,4") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookAir8,1") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookAir8,2") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacBookAir9,1") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "Macmini8,1") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "MacPro7,1") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "iMac20,1") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "iMac20,2") },
- { UEFI_QUIRK_SKIP_CERT("Apple Inc.", "iMacPro1,1") },
- { }
-};
-
/*
* Look to see if a UEFI variable called MokIgnoreDB exists and return true if
* it does.
@@ -165,10 +139,14 @@ static int __init load_uefi_certs(void)
unsigned long dbsize = 0, dbxsize = 0, mokxsize = 0;
efi_status_t status;
int rc = 0;
- const struct dmi_system_id *dmi_id;
- dmi_id = dmi_first_match(uefi_skip_cert);
- if (dmi_id) {
+ /*
+ * On T2 Macs reading the db and dbx efi variables to load UEFI Secure Boot
+ * certificates causes occurrence of a page fault in Apple's firmware and
+ * a crash disabling EFI runtime services. The following quirk skips reading
+ * these variables.
+ */
+ if (has_t2_chip) {
pr_err("Reading UEFI Secure Boot Certs is not supported on T2 Macs.\n");
return false;
}
--
2.43.0
^ permalink raw reply related
* [PATCH 0/2] Add Apple T2 NHI device links
From: Atharva Tiwari @ 2026-07-19 18:03 UTC (permalink / raw)
Cc: Atharva Tiwari, Bjorn Helgaas, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Andreas Noever,
Mika Westerberg, Yehezkel Bernat, Hans de Goede,
Ilpo Järvinen, Jarkko Sakkinen, Mimi Zohar, Roberto Sassu,
Dmitry Kasatkin, Eric Snowberg, Paul Moore, James Morris,
Serge E. Hallyn, linux-pci, linux-kernel, linux-usb,
platform-driver-x86, linux-integrity, keyrings,
linux-security-module
This series adds a cached flag to detect Apple T2 systems, then uses it
to create device links for the Thunderbolt NHI.
Atharva Tiwari (2):
treewide: Add a flag to detect the Apple T2 chip
thunderbolt: Add device links for Apple T2 NHI
arch/x86/pci/fixup.c | 10 ++++
drivers/thunderbolt/tb.c | 49 +++++++++++++++++++
include/linux/platform_data/x86/apple.h | 5 ++
security/integrity/platform_certs/load_uefi.c | 38 +++-----------
4 files changed, 72 insertions(+), 30 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v3 3/3] assoc_array: trim the final shortcut word using the current chunk end
From: Michael Bommarito @ 2026-07-19 16:15 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen
Cc: Andrew Morton, Paul Moore, James Morris, Serge E . Hallyn,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260719161505.2423935-1-michael.bommarito@gmail.com>
assoc_array_walk() masks off the bits past shortcut->skip_to_level in the
word that contains skip_to_level, gated on
round_up(sc_level, ASSOC_ARRAY_KEY_CHUNK_SIZE) > skip_to_level.
That guard is wrong in two opposite ways:
- When sc_level is word-aligned (every word after the first) round_up()
is a no-op, so the guard is sc_level > skip_to_level and never fires for
the word that holds skip_to_level. A shortcut that spans more than one
word and ends in the middle of its last word leaves that word untrimmed,
and its stale high bits leak into the dissimilarity word and can steer
the walk down the wrong descendant.
- When sc_level is unaligned (the first word) and skip_to_level sits on
the next chunk boundary, sc_level + CHUNK would exceed skip_to_level and
fire the trim with shift = skip_to_level & CHUNK_MASK == 0, which clears
the whole dissimilarity word and makes a differing shortcut compare
equal.
Use the end of the chunk that contains sc_level instead:
skip_to_level < round_down(sc_level, CHUNK) + CHUNK
For an aligned sc_level whose word holds skip_to_level this now fires (the
first bug); for an unaligned sc_level with skip_to_level on the following
boundary it does not, so shift is never 0 when the branch runs and the trim
never clears the whole word.
Fixes: 3cb989501c26 ("Add a generic associative array implementation.")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
lib/assoc_array.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/lib/assoc_array.c b/lib/assoc_array.c
index bcc6e0a013eb8..b6c9723e12ced 100644
--- a/lib/assoc_array.c
+++ b/lib/assoc_array.c
@@ -255,7 +255,8 @@ assoc_array_walk(const struct assoc_array *array,
sc_segments = shortcut->index_key[sc_level >> ASSOC_ARRAY_KEY_CHUNK_SHIFT];
dissimilarity = segments ^ sc_segments;
- if (round_up(sc_level, ASSOC_ARRAY_KEY_CHUNK_SIZE) > shortcut->skip_to_level) {
+ if (shortcut->skip_to_level < round_down(sc_level,
+ ASSOC_ARRAY_KEY_CHUNK_SIZE) + ASSOC_ARRAY_KEY_CHUNK_SIZE) {
/* Trim segments that are beyond the shortcut */
int shift = shortcut->skip_to_level & ASSOC_ARRAY_KEY_CHUNK_MASK;
dissimilarity &= ~(ULONG_MAX << shift);
--
2.53.0
^ permalink raw reply related
* [PATCH v3 2/3] keys: make keyring key-chunk byte order agree with keyring_diff_objects()
From: Michael Bommarito @ 2026-07-19 16:15 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen
Cc: Andrew Morton, Paul Moore, James Morris, Serge E . Hallyn,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260719161505.2423935-1-michael.bommarito@gmail.com>
keyring_get_key_chunk() loads description bytes into the index chunk low
address first, while keyring_diff_objects() numbers the first differing
bit from the low end and folds the absolute byte index into the level
without removing the inline-prefix offset the level already carries.
The two disagree on byte order and bit position, so the array can be
told two keys first differ at a bit that does not differ in the chunk
the walker uses, letting crafted descriptions collide into one node.
Load the chunk in the order keyring_diff_objects() assumes and drop the
inline-prefix length when folding the byte index into the level. This
only changes the in-memory ordering used to place keys within a keyring;
add, search and read of non-colliding keys are unaffected.
Fixes: f771fde82051 ("keys: Simplify key description management")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
security/keys/keyring.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 085f7a743354c..15bf4af8f2821 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -293,9 +293,10 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
desc_len -= offset;
if (desc_len > n)
desc_len = n;
+ d += desc_len;
do {
chunk <<= 8;
- chunk |= *d++;
+ chunk |= *--d;
} while (--desc_len > 0);
return chunk;
}
@@ -376,7 +377,7 @@ static int keyring_diff_objects(const void *object, const void *data)
return -1;
differ_plus_i:
- level += i;
+ level += i - (int)sizeof(a->desc);
differ:
i = level * 8 + __ffs(seg_a ^ seg_b);
return i;
--
2.53.0
^ permalink raw reply related
* [PATCH v3 1/3] keys: fix out-of-bounds read in keyring_get_key_chunk()
From: Michael Bommarito @ 2026-07-19 16:15 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen
Cc: Andrew Morton, Paul Moore, James Morris, Serge E . Hallyn,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260719161505.2423935-1-michael.bommarito@gmail.com>
For description-level chunks keyring_get_key_chunk() advances the read
pointer by level * sizeof(long) past the inline prefix but only
bounds-checks the prefix, so a long enough key description is read past
its kmemdup(desc, desc_len + 1) allocation. Compute the full byte
offset and bounds-check the description against it before reading.
The walk only reaches a description-level chunk when two keys collide
through the hash, x, type and domain_tag chunks, so this is reached from
an unprivileged add_key(2) with a crafted pair of same-type keys whose
index hashes collide; KASAN reports a slab-out-of-bounds read.
Fixes: f771fde82051 ("keys: Simplify key description management")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
security/keys/keyring.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 7a2ee0ded7c93..085f7a743354c 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -271,6 +271,7 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
unsigned long chunk = 0;
const u8 *d;
int desc_len = index_key->desc_len, n = sizeof(chunk);
+ unsigned int offset;
level /= ASSOC_ARRAY_KEY_CHUNK_SIZE;
switch (level) {
@@ -284,12 +285,12 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
return (unsigned long)index_key->domain_tag;
default:
level -= 4;
- if (desc_len <= sizeof(index_key->desc))
+ offset = sizeof(index_key->desc) + level * sizeof(long);
+ if (desc_len <= offset)
return 0;
- d = index_key->description + sizeof(index_key->desc);
- d += level * sizeof(long);
- desc_len -= sizeof(index_key->desc);
+ d = index_key->description + offset;
+ desc_len -= offset;
if (desc_len > n)
desc_len = n;
do {
--
2.53.0
^ permalink raw reply related
* [PATCH v3 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency
From: Michael Bommarito @ 2026-07-19 16:15 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen
Cc: Andrew Morton, Paul Moore, James Morris, Serge E . Hallyn,
keyrings, linux-security-module, linux-kernel
An unprivileged keyring whose keys collide through the description-chunk
path can drive assoc_array node splitting into an out-of-bounds slot write.
Patch 1 stops the out-of-bounds read in keyring_get_key_chunk(); patch 2
makes the chunk byte order agree with keyring_diff_objects(); patch 3 fixes
the shortcut-walk trim so the walk cannot be steered down the wrong
descendant.
v3 changes (patch 1 only; patches 2 and 3 are unchanged):
Per Jarkko's review, patch 1 no longer extends the existing
keyring_get_key_chunk() declaration line; the new offset is declared on its
own line as unsigned int. No functional change.
Patches 2 and 3 are unchanged from v2 and carry Jarkko's Reviewed-by.
v2: https://lore.kernel.org/keyrings/20260714115451.3773164-1-michael.bommarito@gmail.com/
v1: https://lore.kernel.org/keyrings/20260712014500.480410-1-michael.bommarito@gmail.com/
Michael Bommarito (3):
keys: fix out-of-bounds read in keyring_get_key_chunk()
keys: make keyring key-chunk byte order agree with
keyring_diff_objects()
assoc_array: trim the final shortcut word using the current chunk end
lib/assoc_array.c | 3 ++-
security/keys/keyring.c | 14 ++++++++------
2 files changed, 10 insertions(+), 7 deletions(-)
base-commit: 2c7c88a412aa6d09cd04b414211b4ef8553b5309
--
2.53.0
^ permalink raw reply
* Re: unix_stream_connect and socket address resolution
From: John Ericson @ 2026-07-19 15:37 UTC (permalink / raw)
To: David Laight
Cc: Kuniyuki Iwashima, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Cong Wang, Simon Horman, Christian Brauner,
David Rheinsberg, Andy Lutomirski, Sergei Zimmerman, network dev,
Mickaël Salaün, Günther Noack, Paul Moore,
linux-security-module, LKML
In-Reply-To: <20260718215855.07284fb1@pumpkin>
Hi David,
On Sat, Jul 18, 2026, at 4:58 PM, David Laight wrote:
> My $0.02
Thanks for weighing in here.
> If you assume that the client isn't responsible for restarting the server,
> then there is no strong timing relation between creating a new server
> (by any means) and the connect request from the client.
> In other words both the above are very similar to the client being
> preempted at the start of the connect() system call.
>
> What you need to do is hard link foo to foo1, create the new
> socket at foo2/bar then mv foo2 to foo so that it is atomic.
> But I suspect hard links to directories aren't allowed any more :-(
> (Creating 'random' hard links to directories used to be 'fun',
> you could get 'find' in a right mess.)
>
> David
I think I am a little confused by your answer. I am not trying to do
anything in particular in userland relating to dying and restarting
servers. Rather, I am wondering why it was decided (long ago, pre the
current repo's git history) for connect to re-resolve the path every
loop iteration.
I am working on a series of related af_unix refactors, and it would
simplify things a lot if I could make `unix_stream_connect` just resolve
the path once before the loop, but I do not know if that is an
acceptable change in behavior.
I was justifying the change in terms of the resolve-once behavior being
less surprising in my original email, since that reason stands on its
own, with or without my other planned work, but the truth is both that
and the ease of refactoring with that change are my motivations.
Hope that clarifies things,
John
^ permalink raw reply
* Re: [PATCH v2 3/3] assoc_array: trim the final shortcut word using the current chunk end
From: Jarkko Sakkinen @ 2026-07-18 22:18 UTC (permalink / raw)
To: Michael Bommarito
Cc: David Howells, Andrew Morton, Paul Moore, James Morris,
Serge E . Hallyn, keyrings, linux-security-module, linux-kernel
In-Reply-To: <CAJJ9bXyHopjhwusu=yK5HzrF31N5QX0uU-WTRN0g=aSMc9HJmw@mail.gmail.com>
On Sat, Jul 18, 2026 at 04:35:17PM -0400, Michael Bommarito wrote:
> On Sat, Jul 18, 2026 at 2:46 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > Were you able to reproduce this with basic command-line tools? The
> > patches are verifiable by reading the code but asking this just in
> > case if you had a snippet at hand (not interested on complex
> > reproducers).
>
> No, it's a little tricky to hit, definitely not just a few keyctl
> commands. I can send offlist if you'd like but it requires at least
> ~50 or so lines of C/python
Hey, please do, I'm happy to at least try to test this.
>
> PS - I'll send v3 with the nit fixed and carry your tags today/tomorrow
>
> Thanks,
> Mike
BR, Jarkko
^ permalink raw reply
* Re: unix_stream_connect and socket address resolution
From: David Laight @ 2026-07-18 20:58 UTC (permalink / raw)
To: John Ericson
Cc: Kuniyuki Iwashima, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Cong Wang, Simon Horman, Christian Brauner,
David Rheinsberg, Andy Lutomirski, Sergei Zimmerman, network dev,
Mickaël Salaün, Günther Noack, Paul Moore,
linux-security-module, LKML
In-Reply-To: <e6f54c5b-2c92-4772-b2f9-5c21e2ec50a9@app.fastmail.com>
On Sat, 18 Jul 2026 15:55:12 -0400
"John Ericson" <mail@johnericson.me> wrote:
> In [1] I observed what I considered some odd behavior in unix_stream_connect():
>
> > I was hoping this was going to be a simple matter of factoring out the
> > back half of `unix_stream_connect`. No such luck was had, because
> > actually instead of `unix_stream_connect` looking up the socket from the
> > VFS once, it does it repeatedly in the same loop that is used to deal
> > with full listening queues.
> >
> > (This behavior is rather surprising to me, because it would allow a
> > deleted and recreated socket to be picked up on the next loop iteration.
> > But, I don't want to make any UAPI-visible changes in this patch series,
> > so I did not consider changing it.)
>
> I had said I didn't want to consider changing this yet in my patch
> series, but based on the feedback I received for a second version of
> that patch series, I now actually think it is a good idea after all to
> discuss this first, and see if it should be changed prior to my patch
> series. (This discussion will inform what the code looks like before I
> do my v2 patch series, and how big or small that patch series is.)
>
> Here are two scenarios where the current behavior of
> `unix_stream_connect` is surprising:
>
> File system version:
>
> 1. server binds socket `/foo/bar`
>
> 2. clients fill up the accept queue, begin looping
>
> 3. `mv /foo /foo2; mkdir /foo`
>
> 4. another server binds `/foo/bar`
>
> 5. clients connect to the second server instead
>
> The loop in question is within `unix_stream_connect` itself, not in
> user code. I consider it very surprising that `/foo/bar` is looked up
> multiple times during a single system call.
>
> Abstract socket version:
>
> 1. server binds abstract socket `@foo`
>
> 2. clients fill up the accept queue, begin looping
>
> 3. server closes its socket (or exits), releasing the abstract name
>
> 4. another server binds `@foo`
>
> 5. clients connect to the second server instead
>
> For abstract sockets we cannot play tricks with `mv`: the first server
> does need to relinquish `@foo` itself. But still, the result is the same
> where a different socket is resolved on the next loop iteration in
> `unix_stream_connect`. I am not sure it is fair to call this a TOCTOU
> issue exactly, but it feels very similar to one.
>
> The more natural semantics in my view would be to first resolve the
> address to a socket, and then loop holding that resolved socket
> constant. With these semantics:
>
> - In the `mv` case, the retrying clients continue to try connecting to
> the original socket, now at `/foo2/bar`.
>
> - In the close case, the retrying clients fail, and do not connect to
> any new socket at the same path or abstract name.
>
> What do you all think? Is this better? If so, is this a security fix
> which can be made unconditionally, or, absent a real concrete attack
> vector, is this a UAPI-breaking change which is automatically out of
> scope, and would thus need an explicit opt-in mechanism?
>
> Looking forward to feedback,
My $0.02
If you assume that the client isn't responsible for restarting the server,
then there is no strong timing relation between creating a new server
(by any means) and the connect request from the client.
In other words both the above are very similar to the client being
preempted at the start of the connect() system call.
What you need to do is hard link foo to foo1, create the new
socket at foo2/bar then mv foo2 to foo so that it is atomic.
But I suspect hard links to directories aren't allowed any more :-(
(Creating 'random' hard links to directories used to be 'fun',
you could get 'find' in a right mess.)
David
>
> John
>
> [1]: https://lore.kernel.org/all/20260703073948.2541875-3-John.Ericson@Obsidian.Systems/
>
^ permalink raw reply
* Re: [PATCH v2 3/3] assoc_array: trim the final shortcut word using the current chunk end
From: Michael Bommarito @ 2026-07-18 20:35 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: David Howells, Andrew Morton, Paul Moore, James Morris,
Serge E . Hallyn, keyrings, linux-security-module, linux-kernel
In-Reply-To: <alvKED66fSFFcl1Z@kernel.org>
On Sat, Jul 18, 2026 at 2:46 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> Were you able to reproduce this with basic command-line tools? The
> patches are verifiable by reading the code but asking this just in
> case if you had a snippet at hand (not interested on complex
> reproducers).
No, it's a little tricky to hit, definitely not just a few keyctl
commands. I can send offlist if you'd like but it requires at least
~50 or so lines of C/python
PS - I'll send v3 with the nit fixed and carry your tags today/tomorrow
Thanks,
Mike
^ permalink raw reply
* unix_stream_connect and socket address resolution
From: John Ericson @ 2026-07-18 19:55 UTC (permalink / raw)
To: Kuniyuki Iwashima, David S . Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni
Cc: Cong Wang, Simon Horman, Christian Brauner, David Rheinsberg,
Andy Lutomirski, Sergei Zimmerman, network dev,
Mickaël Salaün, Günther Noack, Paul Moore,
linux-security-module, LKML
In-Reply-To: <20260703073948.2541875-3-John.Ericson@Obsidian.Systems>
In [1] I observed what I considered some odd behavior in unix_stream_connect():
> I was hoping this was going to be a simple matter of factoring out the
> back half of `unix_stream_connect`. No such luck was had, because
> actually instead of `unix_stream_connect` looking up the socket from the
> VFS once, it does it repeatedly in the same loop that is used to deal
> with full listening queues.
>
> (This behavior is rather surprising to me, because it would allow a
> deleted and recreated socket to be picked up on the next loop iteration.
> But, I don't want to make any UAPI-visible changes in this patch series,
> so I did not consider changing it.)
I had said I didn't want to consider changing this yet in my patch
series, but based on the feedback I received for a second version of
that patch series, I now actually think it is a good idea after all to
discuss this first, and see if it should be changed prior to my patch
series. (This discussion will inform what the code looks like before I
do my v2 patch series, and how big or small that patch series is.)
Here are two scenarios where the current behavior of
`unix_stream_connect` is surprising:
File system version:
1. server binds socket `/foo/bar`
2. clients fill up the accept queue, begin looping
3. `mv /foo /foo2; mkdir /foo`
4. another server binds `/foo/bar`
5. clients connect to the second server instead
The loop in question is within `unix_stream_connect` itself, not in
user code. I consider it very surprising that `/foo/bar` is looked up
multiple times during a single system call.
Abstract socket version:
1. server binds abstract socket `@foo`
2. clients fill up the accept queue, begin looping
3. server closes its socket (or exits), releasing the abstract name
4. another server binds `@foo`
5. clients connect to the second server instead
For abstract sockets we cannot play tricks with `mv`: the first server
does need to relinquish `@foo` itself. But still, the result is the same
where a different socket is resolved on the next loop iteration in
`unix_stream_connect`. I am not sure it is fair to call this a TOCTOU
issue exactly, but it feels very similar to one.
The more natural semantics in my view would be to first resolve the
address to a socket, and then loop holding that resolved socket
constant. With these semantics:
- In the `mv` case, the retrying clients continue to try connecting to
the original socket, now at `/foo2/bar`.
- In the close case, the retrying clients fail, and do not connect to
any new socket at the same path or abstract name.
What do you all think? Is this better? If so, is this a security fix
which can be made unconditionally, or, absent a real concrete attack
vector, is this a UAPI-breaking change which is automatically out of
scope, and would thus need an explicit opt-in mechanism?
Looking forward to feedback,
John
[1]: https://lore.kernel.org/all/20260703073948.2541875-3-John.Ericson@Obsidian.Systems/
^ permalink raw reply
* Re: [PATCH v2 3/3] assoc_array: trim the final shortcut word using the current chunk end
From: Jarkko Sakkinen @ 2026-07-18 18:46 UTC (permalink / raw)
To: Michael Bommarito
Cc: David Howells, Andrew Morton, Paul Moore, James Morris,
Serge E . Hallyn, keyrings, linux-security-module, linux-kernel
In-Reply-To: <alvIBm3Po_1gX6g_@kernel.org>
On Sat, Jul 18, 2026 at 09:38:01PM +0300, Jarkko Sakkinen wrote:
> On Tue, Jul 14, 2026 at 07:54:51AM -0400, Michael Bommarito wrote:
> > assoc_array_walk() masks off the bits past shortcut->skip_to_level in the
> > word that contains skip_to_level, gated on
> > round_up(sc_level, ASSOC_ARRAY_KEY_CHUNK_SIZE) > skip_to_level.
> >
> > That guard is wrong in two opposite ways:
> >
> > - When sc_level is word-aligned (every word after the first) round_up()
> > is a no-op, so the guard is sc_level > skip_to_level and never fires for
> > the word that holds skip_to_level. A shortcut that spans more than one
> > word and ends in the middle of its last word leaves that word untrimmed,
> > and its stale high bits leak into the dissimilarity word and can steer
> > the walk down the wrong descendant.
> >
> > - When sc_level is unaligned (the first word) and skip_to_level sits on
> > the next chunk boundary, sc_level + CHUNK would exceed skip_to_level and
> > fire the trim with shift = skip_to_level & CHUNK_MASK == 0, which clears
> > the whole dissimilarity word and makes a differing shortcut compare
> > equal.
> >
> > Use the end of the chunk that contains sc_level instead:
> >
> > skip_to_level < round_down(sc_level, CHUNK) + CHUNK
> >
> > For an aligned sc_level whose word holds skip_to_level this now fires (the
> > first bug); for an unaligned sc_level with skip_to_level on the following
> > boundary it does not, so shift is never 0 when the branch runs and the trim
> > never clears the whole word.
> >
> > Fixes: 3cb989501c26 ("Add a generic associative array implementation.")
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> > ---
> > lib/assoc_array.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/assoc_array.c b/lib/assoc_array.c
> > index bcc6e0a013eb8..b6c9723e12ced 100644
> > --- a/lib/assoc_array.c
> > +++ b/lib/assoc_array.c
> > @@ -255,7 +255,8 @@ assoc_array_walk(const struct assoc_array *array,
> > sc_segments = shortcut->index_key[sc_level >> ASSOC_ARRAY_KEY_CHUNK_SHIFT];
> > dissimilarity = segments ^ sc_segments;
> >
> > - if (round_up(sc_level, ASSOC_ARRAY_KEY_CHUNK_SIZE) > shortcut->skip_to_level) {
> > + if (shortcut->skip_to_level < round_down(sc_level,
> > + ASSOC_ARRAY_KEY_CHUNK_SIZE) + ASSOC_ARRAY_KEY_CHUNK_SIZE) {
> > /* Trim segments that are beyond the shortcut */
> > int shift = shortcut->skip_to_level & ASSOC_ARRAY_KEY_CHUNK_MASK;
> > dissimilarity &= ~(ULONG_MAX << shift);
> > --
> > 2.53.0
> >
>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> BR, Jarkko
Were you able to reproduce this with basic command-line tools? The
patches are verifiable by reading the code but asking this just in
case if you had a snippet at hand (not interested on complex
reproducers).
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2 3/3] assoc_array: trim the final shortcut word using the current chunk end
From: Jarkko Sakkinen @ 2026-07-18 18:37 UTC (permalink / raw)
To: Michael Bommarito
Cc: David Howells, Andrew Morton, Paul Moore, James Morris,
Serge E . Hallyn, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260714115451.3773164-4-michael.bommarito@gmail.com>
On Tue, Jul 14, 2026 at 07:54:51AM -0400, Michael Bommarito wrote:
> assoc_array_walk() masks off the bits past shortcut->skip_to_level in the
> word that contains skip_to_level, gated on
> round_up(sc_level, ASSOC_ARRAY_KEY_CHUNK_SIZE) > skip_to_level.
>
> That guard is wrong in two opposite ways:
>
> - When sc_level is word-aligned (every word after the first) round_up()
> is a no-op, so the guard is sc_level > skip_to_level and never fires for
> the word that holds skip_to_level. A shortcut that spans more than one
> word and ends in the middle of its last word leaves that word untrimmed,
> and its stale high bits leak into the dissimilarity word and can steer
> the walk down the wrong descendant.
>
> - When sc_level is unaligned (the first word) and skip_to_level sits on
> the next chunk boundary, sc_level + CHUNK would exceed skip_to_level and
> fire the trim with shift = skip_to_level & CHUNK_MASK == 0, which clears
> the whole dissimilarity word and makes a differing shortcut compare
> equal.
>
> Use the end of the chunk that contains sc_level instead:
>
> skip_to_level < round_down(sc_level, CHUNK) + CHUNK
>
> For an aligned sc_level whose word holds skip_to_level this now fires (the
> first bug); for an unaligned sc_level with skip_to_level on the following
> boundary it does not, so shift is never 0 when the branch runs and the trim
> never clears the whole word.
>
> Fixes: 3cb989501c26 ("Add a generic associative array implementation.")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> lib/assoc_array.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/assoc_array.c b/lib/assoc_array.c
> index bcc6e0a013eb8..b6c9723e12ced 100644
> --- a/lib/assoc_array.c
> +++ b/lib/assoc_array.c
> @@ -255,7 +255,8 @@ assoc_array_walk(const struct assoc_array *array,
> sc_segments = shortcut->index_key[sc_level >> ASSOC_ARRAY_KEY_CHUNK_SHIFT];
> dissimilarity = segments ^ sc_segments;
>
> - if (round_up(sc_level, ASSOC_ARRAY_KEY_CHUNK_SIZE) > shortcut->skip_to_level) {
> + if (shortcut->skip_to_level < round_down(sc_level,
> + ASSOC_ARRAY_KEY_CHUNK_SIZE) + ASSOC_ARRAY_KEY_CHUNK_SIZE) {
> /* Trim segments that are beyond the shortcut */
> int shift = shortcut->skip_to_level & ASSOC_ARRAY_KEY_CHUNK_MASK;
> dissimilarity &= ~(ULONG_MAX << shift);
> --
> 2.53.0
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2 2/3] keys: make keyring key-chunk byte order agree with keyring_diff_objects()
From: Jarkko Sakkinen @ 2026-07-18 18:36 UTC (permalink / raw)
To: Michael Bommarito
Cc: David Howells, Andrew Morton, Paul Moore, James Morris,
Serge E . Hallyn, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260714115451.3773164-3-michael.bommarito@gmail.com>
On Tue, Jul 14, 2026 at 07:54:50AM -0400, Michael Bommarito wrote:
> keyring_get_key_chunk() loads description bytes into the index chunk low
> address first, while keyring_diff_objects() numbers the first differing
> bit from the low end and folds the absolute byte index into the level
> without removing the inline-prefix offset the level already carries.
> The two disagree on byte order and bit position, so the array can be
> told two keys first differ at a bit that does not differ in the chunk
> the walker uses, letting crafted descriptions collide into one node.
>
> Load the chunk in the order keyring_diff_objects() assumes and drop the
> inline-prefix length when folding the byte index into the level. This
> only changes the in-memory ordering used to place keys within a keyring;
> add, search and read of non-colliding keys are unaffected.
>
> Fixes: f771fde82051 ("keys: Simplify key description management")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> security/keys/keyring.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index 1739373172ad5..e7066893e6ffc 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -292,9 +292,10 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
> desc_len -= offset;
> if (desc_len > n)
> desc_len = n;
> + d += desc_len;
> do {
> chunk <<= 8;
> - chunk |= *d++;
> + chunk |= *--d;
> } while (--desc_len > 0);
> return chunk;
> }
> @@ -375,7 +376,7 @@ static int keyring_diff_objects(const void *object, const void *data)
> return -1;
>
> differ_plus_i:
> - level += i;
> + level += i - (int)sizeof(a->desc);
> differ:
> i = level * 8 + __ffs(seg_a ^ seg_b);
> return i;
> --
> 2.53.0
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v2 1/3] keys: fix out-of-bounds read in keyring_get_key_chunk()
From: Jarkko Sakkinen @ 2026-07-18 18:36 UTC (permalink / raw)
To: Michael Bommarito
Cc: David Howells, Andrew Morton, Paul Moore, James Morris,
Serge E . Hallyn, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260714115451.3773164-2-michael.bommarito@gmail.com>
On Tue, Jul 14, 2026 at 07:54:49AM -0400, Michael Bommarito wrote:
> For description-level chunks keyring_get_key_chunk() advances the read
> pointer by level * sizeof(long) past the inline prefix but only
> bounds-checks the prefix, so a long enough key description is read past
> its kmemdup(desc, desc_len + 1) allocation. Compute the full byte
> offset and bounds-check the description against it before reading.
>
> The walk only reaches a description-level chunk when two keys collide
> through the hash, x, type and domain_tag chunks, so this is reached from
> an unprivileged add_key(2) with a crafted pair of same-type keys whose
> index hashes collide; KASAN reports a slab-out-of-bounds read.
>
> Fixes: f771fde82051 ("keys: Simplify key description management")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> security/keys/keyring.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> KASAN, x86_64: add_key(2) of a crafted hash-colliding "user"-key pair
> (~63-byte descriptions) reports
>
> BUG: KASAN: slab-out-of-bounds in keyring_get_key_chunk
> keyring_get_key_chunk <- assoc_array_insert <- __key_link_begin
> <- __do_sys_add_key
>
> reading one byte past the description allocation; the same trigger is
> KASAN-clean with this patch. On a kernel built without init-on-alloc,
> reading the colliding keyring back with KEYCTL_READ returns
> uninitialized slab until patches 2 and 3 are applied too. Trigger
> available off-list.
>
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index 7a2ee0ded7c93..1739373172ad5 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -270,7 +270,7 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
> const struct keyring_index_key *index_key = data;
> unsigned long chunk = 0;
> const u8 *d;
> - int desc_len = index_key->desc_len, n = sizeof(chunk);
> + int desc_len = index_key->desc_len, n = sizeof(chunk), offset;
>
> level /= ASSOC_ARRAY_KEY_CHUNK_SIZE;
> switch (level) {
> @@ -284,12 +284,12 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
> return (unsigned long)index_key->domain_tag;
> default:
> level -= 4;
> - if (desc_len <= sizeof(index_key->desc))
> + offset = sizeof(index_key->desc) + level * sizeof(long);
> + if (desc_len <= offset)
> return 0;
>
> - d = index_key->description + sizeof(index_key->desc);
> - d += level * sizeof(long);
> - desc_len -= sizeof(index_key->desc);
> + d = index_key->description + offset;
> + desc_len -= offset;
> if (desc_len > n)
> desc_len = n;
> do {
> --
> 2.53.0
>
Same feedback as before.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH 3/3] assoc_array: trim the final shortcut word when skip_to_level is chunk-aligned
From: Jarkko Sakkinen @ 2026-07-18 18:34 UTC (permalink / raw)
To: Michael Bommarito
Cc: David Howells, Paul Moore, James Morris, Serge E . Hallyn,
Andrew Morton, keyrings, linux-security-module, linux-kernel
In-Reply-To: <alvGyuUIvjJiRjPk@kernel.org>
On Sat, Jul 18, 2026 at 09:32:46PM +0300, Jarkko Sakkinen wrote:
> On Sat, Jul 11, 2026 at 09:45:00PM -0400, Michael Bommarito wrote:
> > assoc_array_walk() masks off the bits past shortcut->skip_to_level in
> > the final word of a shortcut before testing it, gated on
> > round_up(sc_level, chunk_size) > skip_to_level. Once sc_level is
> > word-aligned (every word after the first) round_up() is a no-op and the
> > guard never fires for the word that contains skip_to_level, so its stale
> > high bits leak into the dissimilarity word and can steer the walk down
> > the wrong descendant.
> >
> > Test sc_level + ASSOC_ARRAY_KEY_CHUNK_SIZE > skip_to_level directly; an
> > exact-multiple skip_to_level ends on the boundary and stays untrimmed.
> >
> > Fixes: 3cb989501c26 ("Add a generic associative array implementation.")
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> > ---
> > lib/assoc_array.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/assoc_array.c b/lib/assoc_array.c
> > index bcc6e0a013eb8..1de2c337f8fcf 100644
> > --- a/lib/assoc_array.c
> > +++ b/lib/assoc_array.c
> > @@ -255,7 +255,7 @@ assoc_array_walk(const struct assoc_array *array,
> > sc_segments = shortcut->index_key[sc_level >> ASSOC_ARRAY_KEY_CHUNK_SHIFT];
> > dissimilarity = segments ^ sc_segments;
> >
> > - if (round_up(sc_level, ASSOC_ARRAY_KEY_CHUNK_SIZE) > shortcut->skip_to_level) {
> > + if (sc_level + ASSOC_ARRAY_KEY_CHUNK_SIZE > shortcut->skip_to_level) {
> > /* Trim segments that are beyond the shortcut */
> > int shift = shortcut->skip_to_level & ASSOC_ARRAY_KEY_CHUNK_MASK;
> > dissimilarity &= ~(ULONG_MAX << shift);
> > --
> > 2.53.0
> >
>
> Ditto.
>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> BR, Jarkko
On holiday up until end of this month so expect some delay in my
responses.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH 3/3] assoc_array: trim the final shortcut word when skip_to_level is chunk-aligned
From: Jarkko Sakkinen @ 2026-07-18 18:32 UTC (permalink / raw)
To: Michael Bommarito
Cc: David Howells, Paul Moore, James Morris, Serge E . Hallyn,
Andrew Morton, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260712014500.480410-4-michael.bommarito@gmail.com>
On Sat, Jul 11, 2026 at 09:45:00PM -0400, Michael Bommarito wrote:
> assoc_array_walk() masks off the bits past shortcut->skip_to_level in
> the final word of a shortcut before testing it, gated on
> round_up(sc_level, chunk_size) > skip_to_level. Once sc_level is
> word-aligned (every word after the first) round_up() is a no-op and the
> guard never fires for the word that contains skip_to_level, so its stale
> high bits leak into the dissimilarity word and can steer the walk down
> the wrong descendant.
>
> Test sc_level + ASSOC_ARRAY_KEY_CHUNK_SIZE > skip_to_level directly; an
> exact-multiple skip_to_level ends on the boundary and stays untrimmed.
>
> Fixes: 3cb989501c26 ("Add a generic associative array implementation.")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> lib/assoc_array.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/assoc_array.c b/lib/assoc_array.c
> index bcc6e0a013eb8..1de2c337f8fcf 100644
> --- a/lib/assoc_array.c
> +++ b/lib/assoc_array.c
> @@ -255,7 +255,7 @@ assoc_array_walk(const struct assoc_array *array,
> sc_segments = shortcut->index_key[sc_level >> ASSOC_ARRAY_KEY_CHUNK_SHIFT];
> dissimilarity = segments ^ sc_segments;
>
> - if (round_up(sc_level, ASSOC_ARRAY_KEY_CHUNK_SIZE) > shortcut->skip_to_level) {
> + if (sc_level + ASSOC_ARRAY_KEY_CHUNK_SIZE > shortcut->skip_to_level) {
> /* Trim segments that are beyond the shortcut */
> int shift = shortcut->skip_to_level & ASSOC_ARRAY_KEY_CHUNK_MASK;
> dissimilarity &= ~(ULONG_MAX << shift);
> --
> 2.53.0
>
Ditto.
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH 2/3] keys: make keyring key-chunk byte order agree with keyring_diff_objects()
From: Jarkko Sakkinen @ 2026-07-18 18:31 UTC (permalink / raw)
To: Michael Bommarito
Cc: David Howells, Paul Moore, James Morris, Serge E . Hallyn,
Andrew Morton, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260712014500.480410-3-michael.bommarito@gmail.com>
On Sat, Jul 11, 2026 at 09:44:59PM -0400, Michael Bommarito wrote:
> keyring_get_key_chunk() loads description bytes into the index chunk low
> address first, while keyring_diff_objects() numbers the first differing
> bit from the low end and folds the absolute byte index into the level
> without removing the inline-prefix offset the level already carries.
> The two disagree on byte order and bit position, so the array can be
> told two keys first differ at a bit that does not differ in the chunk
> the walker uses, letting crafted descriptions collide into one node.
>
> Load the chunk in the order keyring_diff_objects() assumes and drop the
> inline-prefix length when folding the byte index into the level. This
> only changes the in-memory ordering used to place keys within a keyring;
> add, search and read of non-colliding keys are unaffected.
>
> Fixes: f771fde82051 ("keys: Simplify key description management")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> security/keys/keyring.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index 1739373172ad5..e7066893e6ffc 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -292,9 +292,10 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
> desc_len -= offset;
> if (desc_len > n)
> desc_len = n;
> + d += desc_len;
> do {
> chunk <<= 8;
> - chunk |= *d++;
> + chunk |= *--d;
> } while (--desc_len > 0);
> return chunk;
> }
> @@ -375,7 +376,7 @@ static int keyring_diff_objects(const void *object, const void *data)
> return -1;
>
> differ_plus_i:
> - level += i;
> + level += i - (int)sizeof(a->desc);
> differ:
> i = level * 8 + __ffs(seg_a ^ seg_b);
> return i;
> --
> 2.53.0
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Add to +1 version.
BR, Jarkko
^ permalink raw reply
* Re: [PATCH 1/3] keys: fix out-of-bounds read in keyring_get_key_chunk()
From: Jarkko Sakkinen @ 2026-07-18 18:29 UTC (permalink / raw)
To: Michael Bommarito
Cc: David Howells, Paul Moore, James Morris, Serge E . Hallyn,
Andrew Morton, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260712014500.480410-2-michael.bommarito@gmail.com>
On Sat, Jul 11, 2026 at 09:44:58PM -0400, Michael Bommarito wrote:
> For description-level chunks keyring_get_key_chunk() advances the read
> pointer by level * sizeof(long) past the inline prefix but only
> bounds-checks the prefix, so a long enough key description is read past
> its kmemdup(desc, desc_len + 1) allocation. Compute the full byte
> offset and bounds-check the description against it before reading.
>
> The walk only reaches a description-level chunk when two keys collide
> through the hash, x, type and domain_tag chunks, so this is reached from
> an unprivileged add_key(2) with a crafted pair of same-type keys whose
> index hashes collide; KASAN reports a slab-out-of-bounds read.
>
> Fixes: f771fde82051 ("keys: Simplify key description management")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> security/keys/keyring.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> KASAN, x86_64: add_key(2) of a crafted hash-colliding "user"-key pair
> (~63-byte descriptions) reports
>
> BUG: KASAN: slab-out-of-bounds in keyring_get_key_chunk
> keyring_get_key_chunk <- assoc_array_insert <- __key_link_begin
> <- __do_sys_add_key
>
> reading one byte past the description allocation; the same trigger is
> KASAN-clean with this patch. On a kernel built without init-on-alloc,
> reading the colliding keyring back with KEYCTL_READ returns
> uninitialized slab until patches 2 and 3 are applied too. Trigger
> available off-list.
>
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index 7a2ee0ded7c93..1739373172ad5 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -270,7 +270,7 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
> const struct keyring_index_key *index_key = data;
> unsigned long chunk = 0;
> const u8 *d;
> - int desc_len = index_key->desc_len, n = sizeof(chunk);
> + int desc_len = index_key->desc_len, n = sizeof(chunk), offset;
Just a nut but this is quite nasty looking statement to begin with, so
at least I would not extend it.
I'd add "int offset" to its own line. Maybe for the sake of clarity it
should be also unsigned int unless negative values hold some merit.
>
> level /= ASSOC_ARRAY_KEY_CHUNK_SIZE;
> switch (level) {
> @@ -284,12 +284,12 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
> return (unsigned long)index_key->domain_tag;
> default:
> level -= 4;
> - if (desc_len <= sizeof(index_key->desc))
> + offset = sizeof(index_key->desc) + level * sizeof(long);
> + if (desc_len <= offset)
> return 0;
>
> - d = index_key->description + sizeof(index_key->desc);
> - d += level * sizeof(long);
> - desc_len -= sizeof(index_key->desc);
> + d = index_key->description + offset;
> + desc_len -= offset;
> if (desc_len > n)
> desc_len = n;
> do {
> --
> 2.53.0
>
BR, Jarkko
^ permalink raw reply
* Re: [PATCH 0/3] keys: fix keyring assoc-array out-of-bounds read and index inconsistency
From: Jarkko Sakkinen @ 2026-07-18 18:23 UTC (permalink / raw)
To: Michael Bommarito
Cc: David Howells, Paul Moore, James Morris, Serge E . Hallyn,
Andrew Morton, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20260712014500.480410-1-michael.bommarito@gmail.com>
On Sat, Jul 11, 2026 at 09:44:57PM -0400, Michael Bommarito wrote:
> keyring_get_key_chunk() advances the description read pointer by
> level * sizeof(long) past the inline prefix but only bounds-checks the
> prefix, so once the associative-array walk reaches a description-level
> chunk it reads past the kmemdup(desc, desc_len + 1) description
> allocation. Reaching that depth needs two keys that collide through the
> hash, x, type and domain_tag chunks, which an unprivileged add_key(2)
> can arrange with a crafted pair of same-type keys.
Thanks for the description. I fully get the scenario from this.
>
> An unprivileged user can thus read up to sizeof(long) bytes past a
> keyring key's description; on kernels built without init-on-alloc the
> same collision, read back with KEYCTL_READ, returns uninitialized kernel
> slab.
>
> Patch 1 is the memory-safety fix and stands alone. Patches 2 and 3 fix
> two index-key consistency bugs that let the crafted keys collide into a
> single malformed node in the first place, which is what enables the
> KEYCTL_READ disclosure.
>
> The KASAN reproduction is on patch 1. Trigger is available off-list.
>
> Michael Bommarito (3):
> keys: fix out-of-bounds read in keyring_get_key_chunk()
> keys: make keyring key-chunk byte order agree with
> keyring_diff_objects()
> assoc_array: trim the final shortcut word when skip_to_level is
> chunk-aligned
>
> lib/assoc_array.c | 2 +-
> security/keys/keyring.c | 15 ++++++++-------
> 2 files changed, 9 insertions(+), 8 deletions(-)
>
>
> base-commit: 2c7c88a412aa6d09cd04b414211b4ef8553b5309
> --
> 2.53.0
>
BR, Jarkko
^ permalink raw reply
* Re: Landlock: LANDLOCK_ACCESS_FS_IOCTL_DEV is bypassable via io_uring IORING_OP_URING_CMD (confirmed on real NVMe hardware)
From: Mickaël Salaün @ 2026-07-18 15:01 UTC (permalink / raw)
To: Vivek Parikh
Cc: Günther Noack, Paul Moore, Jens Axboe, linux-security-module,
io-uring
In-Reply-To: <20260718135650.380643-1-viv0411.parikh@gmail.com>
Hi Vivek,
Similar reports were already sent:
https://lore.kernel.org/all/20260616201633.275067-1-hexlabsecurity@proton.me/
Please take a look at the Landlock threat model:
https://lore.kernel.org/all/20260707210336.2060040-1-mic@digikod.net/
This is not a bypass.
Regards,
Mickaël
On Sat, Jul 18, 2026 at 07:26:48PM +0530, Vivek Parikh wrote:
> Hi Mickaël,
>
> Note: this was found with AI assistance, so I am treating it as public per
> Documentation/process/security-bugs.
>
> While continuing the LSM-mediation audit I found that Landlock's
> LANDLOCK_ACCESS_FS_IOCTL_DEV right can be bypassed with io_uring's
> IORING_OP_URING_CMD. Unlike the mount_setattr(2) gap I reported earlier,
> this one is fully unprivileged and squarely inside Landlock's documented
> model. I have verified it on real NVMe hardware (output below).
>
> The mechanism
> -------------
> Landlock enforces IOCTL_DEV through the file_ioctl / file_ioctl_compat LSM
> hooks (security/landlock/fs.c: LSM_HOOK_INIT(file_ioctl, ...) /
> file_ioctl_compat -> hook_file_ioctl_common ->
> LANDLOCK_ACCESS_FS_IOCTL_DEV, security/landlock/fs.c:1854/1865).
>
> io_uring's IORING_OP_URING_CMD dispatches driver passthrough commands
> through a *different* hook, security_uring_cmd(ioucmd)
> (io_uring/uring_cmd.c:249), before calling file->f_op->uring_cmd. Landlock
> implements no uring_cmd hook -- it has no io_uring hooks at all (only
> SELinux and Smack implement security_uring_cmd). So for the same device
> fd:
>
> ioctl(devfd, CMD, arg) -> security_file_ioctl -> Landlock: DENIED
> (IOCTL_DEV not granted)
> IORING_OP_URING_CMD(devfd) -> security_uring_cmd -> Landlock: NO HOOK
> -> f_op->uring_cmd runs
>
> uring_cmd is the async twin of the device ioctl. NVMe makes the
> equivalence explicit (drivers/nvme/host/ioctl.c): the ioctl path handles
> NVME_IOCTL_ADMIN_CMD and the uring_cmd path handles NVME_URING_CMD_ADMIN
> -- the same admin/IO passthrough commands. Both the controller char dev
> (nvme_dev_uring_cmd, core.c:3841) and the namespace char dev /dev/ngX
> (nvme_ns_chr_uring_cmd, core.c:3946) implement ->uring_cmd, as do ublk
> and drivers/char/mem.c.
>
> Impact
> ------
> A Landlock-sandboxed task that is denied IOCTL_DEV on a device but holds
> an fd to it (opened under a granted fs right, or inherited) can issue the
> equivalent device commands via IORING_OP_URING_CMD, defeating exactly what
> IOCTL_DEV exists to gate. On NVMe -- the most common storage device on
> Linux systems -- this means arbitrary admin passthrough: the same code
> path carries FORMAT NVM, SANITIZE, and firmware-download commands, not
> just reads/writes. The realistic scenario: a sandboxed storage workload
> is given an NVMe namespace fd for fast IO with the expectation "it can do
> IO but cannot send device-control commands" -- the expectation
> LANDLOCK_ACCESS_FS_IOCTL_DEV was added (ABI 5) to express. It is void.
>
> For ublk devices the gap is total: ublk's control plane is uring_cmd-only
> (no ioctl equivalent), so on ublk char devices IOCTL_DEV currently
> mediates nothing at all.
>
> Documentation/userspace-api/landlock.rst presents IOCTL_DEV as the control
> over device ioctls; a sandbox author reasonably expects withholding it to
> stop device control commands. io_uring is unprivileged, Landlock is
> unprivileged, and no capability is required anywhere.
>
> Affected versions
> -----------------
> This is not a regression -- it is a coverage gap that shipped with the
> IOCTL_DEV right. LANDLOCK_ACCESS_FS_IOCTL_DEV was added in v6.10
> (b25f7415eb41, May 2024); NVMe/ublk ->uring_cmd predate it (v6.0), so the
> first kernel to support IOCTL_DEV was already bypassable. It is present in
> every kernel from v6.10 through v7.2-rc3, including 6.12 LTS. (SELinux and
> Smack implement security_uring_cmd and are unaffected; Landlock and AppArmor
> do not, but only Landlock exposes IOCTL_DEV as a user-facing right.)
>
> For completeness on scope: exploitation requires a Landlock policy that
> handles IOCTL_DEV, a sandboxee that holds/opens a ->uring_cmd-capable device
> fd, and io_uring not otherwise blocked. Sandboxers that also seccomp-filter
> io_uring (e.g. Chromium) are not affected via this path; the exposure is for
> the growing set of tools that adopt IOCTL_DEV without blocking io_uring.
>
> Reproducer (confirmed on real hardware)
> ---------------------------------------
> Three self-contained PoCs (raw io_uring, no liburing) are available on
> request; I am not inlining them.
>
> PoC 1 targets /dev/null (null_fops has no ->unlocked_ioctl but has
> .uring_cmd = uring_cmd_null) and needs no special hardware:
>
> [*] Landlock enforced: READ/WRITE granted on /, IOCTL_DEV denied
> [1] ioctl(/dev/null, dev-cmd) = -1 (Permission denied) <- Landlock DENIED
> [2] io_uring URING_CMD(/dev/null) res = 0 (OK) <- Landlock did NOT mediate
>
> PoC 2 targets a real NVMe controller (/dev/nvme0, WD PC SN740) with
> IDENTIFY CONTROLLER (admin opcode 0x06, read-only), on
> 7.0.0-27-generic:
>
> [*] Landlock enforced: READ/WRITE granted on /, IOCTL_DEV denied
> [1] ioctl(NVME_IOCTL_ADMIN_CMD identify) = -1 (Permission denied) <- Landlock DENIED
> [2] URING_CMD(NVME_URING_CMD_ADMIN identify) res = 0 (OK)
> IDENTIFY data: vid=0xb715 model="WD PC SN740 SDDPMQD-512G-1101"
>
> The same PoC was also confirmed on AWS EC2 (Ubuntu 7.0.0-1008-aws, stock
> cloud image, default settings, real NVMe EBS volume):
>
> [1] ioctl(NVME_IOCTL_ADMIN_CMD identify) = -1 (Permission denied) <- Landlock DENIED
> [2] URING_CMD(NVME_URING_CMD_ADMIN identify) res = 0 (OK)
> IDENTIFY data: vid=0x0f1d model="Amazon Elastic Block Store"
>
> PoC 3 targets /dev/fuse (world-accessible, 0666) as a fully unprivileged
> user (fresh uid, no group memberships, Landlock ABI 8; also reproduced in
> a Docker container with seccomp relaxed). No fuse module parameter is
> needed for this signal: fuse_uring_cmd() (fs/fuse/dev_uring.c:1217) calls
> fuse_get_dev() *before* its enable_uring check, so a never-mounted fd
> returns -EPERM even with enable_uring=N (the default). The full
> queue-registration scenario does need enable_uring=1; NVMe and ublk have
> no such gate at all.
>
> [*] Landlock enforced: READ/WRITE granted on /, IOCTL_DEV denied
> [1] ioctl(/dev/fuse, FUSE_DEV_IOC_CLONE) = -1 (Permission denied) <- Landlock DENIED
> [2] URING_CMD(/dev/fuse, FUSE_IO_URING_CMD_REGISTER) res = -1 (EPERM)
> <- reached fuse_uring_cmd; Landlock did NOT mediate
>
> (-EPERM can only originate inside fuse_uring_cmd on a never-mounted
> fd, proving the call passed security_uring_cmd into the driver.)
>
> The ioctl admin passthrough is denied while the identical admin command
> executes via io_uring. (Note: NVMe uring passthrough requires
> SQE128+CQE32, per nvme_uring_cmd_checks.)
>
> Mitigations / not affected
> --------------------------
> The bypass is neutralised anywhere io_uring cannot be reached:
>
> - kernel.io_uring_disabled = 2 makes io_uring_setup() return -EPERM for all
> callers (io_uring_allowed(), io_uring/io_uring.c); value 1 restricts it to
> io_uring_group / CAP_SYS_ADMIN. Where an admin has set either, this path is
> blocked. This is a hardening knob, not a universal default: RHEL 9.3 / Rocky
> 9.3 ship io_uring *enabled* on the host (Red Hat documents the syscalls as
> succeeding or returning EPERM per configuration).
> - Many container runtimes block the io_uring syscalls in their default seccomp
> profile (e.g. Docker/Podman -> io_uring_setup fails with EPERM/ENOSYS inside
> the container, verified with Docker 29 on Fedora 42), so a sandboxee confined
> by such a runtime is protected. Likewise application sandboxers that
> seccomp-filter io_uring (e.g. Chromium) are not affected via this path.
> Additionally, on SELinux-enforcing hosts, containers without a relaxed label
> are denied io_uring_setup by selinux_uring_allowed -- a second independent
> gate (verified: the same container run fails with EACCES until
> --security-opt label=disable is given).
>
> So the exposed population is: io_uring-enabled kernels (the desktop default,
> and RHEL/Rocky on a bare host) running a Landlock sandbox that handles
> IOCTL_DEV without an io_uring seccomp block.
>
> Fix direction
> -------------
> security_uring_cmd(ioucmd) gives the LSM the io_uring_cmd (and thus the
> struct file). Landlock should implement a uring_cmd hook that, for a
> device file, requires LANDLOCK_ACCESS_FS_IOCTL_DEV -- mirroring
> hook_file_ioctl. The uring_cmd command is driver-specific rather than a
> standard ioctl cmd number, so the is_masked_device_ioctl() allow-list
> (FIONREAD etc.) does not apply; the safe behavior is to require IOCTL_DEV
> for any uring_cmd on a device file. I am happy to prepare that patch
> (LSM_HOOK_INIT(uring_cmd, ...) + a tools/testing/selftests/landlock test)
> if you agree with the direction.
>
> CCing Jens and io-uring, as the io_uring side is involved (a new LSM hook
> consumer, no io_uring behavior change expected).
>
> Thanks,
> Vivek
>
^ permalink raw reply
* Landlock: LANDLOCK_ACCESS_FS_IOCTL_DEV is bypassable via io_uring IORING_OP_URING_CMD (confirmed on real NVMe hardware)
From: Vivek Parikh @ 2026-07-18 13:56 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, Paul Moore, Jens Axboe, linux-security-module,
io-uring, viv0411.parikh
Hi Mickaël,
Note: this was found with AI assistance, so I am treating it as public per
Documentation/process/security-bugs.
While continuing the LSM-mediation audit I found that Landlock's
LANDLOCK_ACCESS_FS_IOCTL_DEV right can be bypassed with io_uring's
IORING_OP_URING_CMD. Unlike the mount_setattr(2) gap I reported earlier,
this one is fully unprivileged and squarely inside Landlock's documented
model. I have verified it on real NVMe hardware (output below).
The mechanism
-------------
Landlock enforces IOCTL_DEV through the file_ioctl / file_ioctl_compat LSM
hooks (security/landlock/fs.c: LSM_HOOK_INIT(file_ioctl, ...) /
file_ioctl_compat -> hook_file_ioctl_common ->
LANDLOCK_ACCESS_FS_IOCTL_DEV, security/landlock/fs.c:1854/1865).
io_uring's IORING_OP_URING_CMD dispatches driver passthrough commands
through a *different* hook, security_uring_cmd(ioucmd)
(io_uring/uring_cmd.c:249), before calling file->f_op->uring_cmd. Landlock
implements no uring_cmd hook -- it has no io_uring hooks at all (only
SELinux and Smack implement security_uring_cmd). So for the same device
fd:
ioctl(devfd, CMD, arg) -> security_file_ioctl -> Landlock: DENIED
(IOCTL_DEV not granted)
IORING_OP_URING_CMD(devfd) -> security_uring_cmd -> Landlock: NO HOOK
-> f_op->uring_cmd runs
uring_cmd is the async twin of the device ioctl. NVMe makes the
equivalence explicit (drivers/nvme/host/ioctl.c): the ioctl path handles
NVME_IOCTL_ADMIN_CMD and the uring_cmd path handles NVME_URING_CMD_ADMIN
-- the same admin/IO passthrough commands. Both the controller char dev
(nvme_dev_uring_cmd, core.c:3841) and the namespace char dev /dev/ngX
(nvme_ns_chr_uring_cmd, core.c:3946) implement ->uring_cmd, as do ublk
and drivers/char/mem.c.
Impact
------
A Landlock-sandboxed task that is denied IOCTL_DEV on a device but holds
an fd to it (opened under a granted fs right, or inherited) can issue the
equivalent device commands via IORING_OP_URING_CMD, defeating exactly what
IOCTL_DEV exists to gate. On NVMe -- the most common storage device on
Linux systems -- this means arbitrary admin passthrough: the same code
path carries FORMAT NVM, SANITIZE, and firmware-download commands, not
just reads/writes. The realistic scenario: a sandboxed storage workload
is given an NVMe namespace fd for fast IO with the expectation "it can do
IO but cannot send device-control commands" -- the expectation
LANDLOCK_ACCESS_FS_IOCTL_DEV was added (ABI 5) to express. It is void.
For ublk devices the gap is total: ublk's control plane is uring_cmd-only
(no ioctl equivalent), so on ublk char devices IOCTL_DEV currently
mediates nothing at all.
Documentation/userspace-api/landlock.rst presents IOCTL_DEV as the control
over device ioctls; a sandbox author reasonably expects withholding it to
stop device control commands. io_uring is unprivileged, Landlock is
unprivileged, and no capability is required anywhere.
Affected versions
-----------------
This is not a regression -- it is a coverage gap that shipped with the
IOCTL_DEV right. LANDLOCK_ACCESS_FS_IOCTL_DEV was added in v6.10
(b25f7415eb41, May 2024); NVMe/ublk ->uring_cmd predate it (v6.0), so the
first kernel to support IOCTL_DEV was already bypassable. It is present in
every kernel from v6.10 through v7.2-rc3, including 6.12 LTS. (SELinux and
Smack implement security_uring_cmd and are unaffected; Landlock and AppArmor
do not, but only Landlock exposes IOCTL_DEV as a user-facing right.)
For completeness on scope: exploitation requires a Landlock policy that
handles IOCTL_DEV, a sandboxee that holds/opens a ->uring_cmd-capable device
fd, and io_uring not otherwise blocked. Sandboxers that also seccomp-filter
io_uring (e.g. Chromium) are not affected via this path; the exposure is for
the growing set of tools that adopt IOCTL_DEV without blocking io_uring.
Reproducer (confirmed on real hardware)
---------------------------------------
Three self-contained PoCs (raw io_uring, no liburing) are available on
request; I am not inlining them.
PoC 1 targets /dev/null (null_fops has no ->unlocked_ioctl but has
.uring_cmd = uring_cmd_null) and needs no special hardware:
[*] Landlock enforced: READ/WRITE granted on /, IOCTL_DEV denied
[1] ioctl(/dev/null, dev-cmd) = -1 (Permission denied) <- Landlock DENIED
[2] io_uring URING_CMD(/dev/null) res = 0 (OK) <- Landlock did NOT mediate
PoC 2 targets a real NVMe controller (/dev/nvme0, WD PC SN740) with
IDENTIFY CONTROLLER (admin opcode 0x06, read-only), on
7.0.0-27-generic:
[*] Landlock enforced: READ/WRITE granted on /, IOCTL_DEV denied
[1] ioctl(NVME_IOCTL_ADMIN_CMD identify) = -1 (Permission denied) <- Landlock DENIED
[2] URING_CMD(NVME_URING_CMD_ADMIN identify) res = 0 (OK)
IDENTIFY data: vid=0xb715 model="WD PC SN740 SDDPMQD-512G-1101"
The same PoC was also confirmed on AWS EC2 (Ubuntu 7.0.0-1008-aws, stock
cloud image, default settings, real NVMe EBS volume):
[1] ioctl(NVME_IOCTL_ADMIN_CMD identify) = -1 (Permission denied) <- Landlock DENIED
[2] URING_CMD(NVME_URING_CMD_ADMIN identify) res = 0 (OK)
IDENTIFY data: vid=0x0f1d model="Amazon Elastic Block Store"
PoC 3 targets /dev/fuse (world-accessible, 0666) as a fully unprivileged
user (fresh uid, no group memberships, Landlock ABI 8; also reproduced in
a Docker container with seccomp relaxed). No fuse module parameter is
needed for this signal: fuse_uring_cmd() (fs/fuse/dev_uring.c:1217) calls
fuse_get_dev() *before* its enable_uring check, so a never-mounted fd
returns -EPERM even with enable_uring=N (the default). The full
queue-registration scenario does need enable_uring=1; NVMe and ublk have
no such gate at all.
[*] Landlock enforced: READ/WRITE granted on /, IOCTL_DEV denied
[1] ioctl(/dev/fuse, FUSE_DEV_IOC_CLONE) = -1 (Permission denied) <- Landlock DENIED
[2] URING_CMD(/dev/fuse, FUSE_IO_URING_CMD_REGISTER) res = -1 (EPERM)
<- reached fuse_uring_cmd; Landlock did NOT mediate
(-EPERM can only originate inside fuse_uring_cmd on a never-mounted
fd, proving the call passed security_uring_cmd into the driver.)
The ioctl admin passthrough is denied while the identical admin command
executes via io_uring. (Note: NVMe uring passthrough requires
SQE128+CQE32, per nvme_uring_cmd_checks.)
Mitigations / not affected
--------------------------
The bypass is neutralised anywhere io_uring cannot be reached:
- kernel.io_uring_disabled = 2 makes io_uring_setup() return -EPERM for all
callers (io_uring_allowed(), io_uring/io_uring.c); value 1 restricts it to
io_uring_group / CAP_SYS_ADMIN. Where an admin has set either, this path is
blocked. This is a hardening knob, not a universal default: RHEL 9.3 / Rocky
9.3 ship io_uring *enabled* on the host (Red Hat documents the syscalls as
succeeding or returning EPERM per configuration).
- Many container runtimes block the io_uring syscalls in their default seccomp
profile (e.g. Docker/Podman -> io_uring_setup fails with EPERM/ENOSYS inside
the container, verified with Docker 29 on Fedora 42), so a sandboxee confined
by such a runtime is protected. Likewise application sandboxers that
seccomp-filter io_uring (e.g. Chromium) are not affected via this path.
Additionally, on SELinux-enforcing hosts, containers without a relaxed label
are denied io_uring_setup by selinux_uring_allowed -- a second independent
gate (verified: the same container run fails with EACCES until
--security-opt label=disable is given).
So the exposed population is: io_uring-enabled kernels (the desktop default,
and RHEL/Rocky on a bare host) running a Landlock sandbox that handles
IOCTL_DEV without an io_uring seccomp block.
Fix direction
-------------
security_uring_cmd(ioucmd) gives the LSM the io_uring_cmd (and thus the
struct file). Landlock should implement a uring_cmd hook that, for a
device file, requires LANDLOCK_ACCESS_FS_IOCTL_DEV -- mirroring
hook_file_ioctl. The uring_cmd command is driver-specific rather than a
standard ioctl cmd number, so the is_masked_device_ioctl() allow-list
(FIONREAD etc.) does not apply; the safe behavior is to require IOCTL_DEV
for any uring_cmd on a device file. I am happy to prepare that patch
(LSM_HOOK_INIT(uring_cmd, ...) + a tools/testing/selftests/landlock test)
if you agree with the direction.
CCing Jens and io-uring, as the io_uring side is involved (a new LSM hook
consumer, no io_uring behavior change expected).
Thanks,
Vivek
^ permalink raw reply
* Landlock: mount_setattr(2) is unmediated by LSMs (ro-mount confinement bypass)
From: Vivek Parikh @ 2026-07-18 10:14 UTC (permalink / raw)
To: Mickaël Salaün, Christian Brauner, Alexander Viro
Cc: Günther Noack, Paul Moore, linux-security-module,
linux-fsdevel, Vivek Parikh
Hi Mickaël, Christian,
Note: this was found with AI assistance, so I am treating it as public per
Documentation/process/security-bugs.
While auditing Landlock's filesystem-topology restrictions I found that
mount_setattr(2) is not mediated by any LSM. It only matters for a
sandboxed task that holds CAP_SYS_ADMIN in its own user namespace -- the
rootful-container / userns-root profile that landlock_restrict_self()
explicitly supports (security/landlock/syscalls.c). Fully unprivileged
callers are not affected.
do_mount_setattr() (fs/namespace.c:4928) -> mount_setattr_prepare() has no
security_* hook anywhere on its path, whereas mount(2), move_mount(2),
umount(2), remount and pivot_root(2) all do (security_sb_mount,
security_move_mount, security_sb_umount, security_sb_remount,
security_sb_pivotroot). Landlock hooks exactly those five in
security/landlock/fs.c (hook_sb_mount, hook_move_mount, hook_sb_umount,
hook_sb_remount, hook_sb_pivotroot) but cannot see mount_setattr(2).
Documentation/userspace-api/landlock.rst ("Filesystem topology
modification") states that sandboxed threads cannot modify filesystem
topology, but such a task can still, via mount_setattr(2):
1. Clear MOUNT_ATTR_RDONLY on a read-only (bind) mount and write through
it -- subverting the common ro-bind-mount confinement pattern.
2. Change mount propagation (shared/private/slave/unbindable).
3. Request MOUNT_ATTR_IDMAP changes (narrower in practice: gated by
can_idmap_mount()).
This is a mediation/coverage gap, not a rule-evaluation bug: Landlock's
filesystem access-rights checks still apply on top. The issue is that the
ro-mount / propagation / idmap layer of a confinement -- which sandbox
setups rely on -- is changeable despite the documented topology
restriction. It affects every LSM, not just Landlock (SELinux, AppArmor
and Smack cannot mediate mount_setattr(2) either).
A self-contained unprivileged reproducer (userns+mountns, no external
privilege) is available on request; I am not inlining it here. Its output,
on 7.0.0-27-generic (host) and reproduced on 7.2.0-rc3 (QEMU guest,
CONFIG_SECURITY_LANDLOCK=y):
[1] write via ro mount before Landlock: Read-only file system (expected)
[2] Landlock enforced (all fs accesses allowed via rule on /)
[3] mount(2) under Landlock: Operation not permitted (expected EPERM)
[4] mount_setattr(rw-flip) under Landlock: SUCCESS <-- gap
[5] write via formerly-ro mount: SUCCEEDED <-- confinement subverted
mount(2) is correctly denied (step 3) while the equivalent attribute
change via mount_setattr(2) succeeds (step 4) and makes the previously
read-only tree writable (step 5).
I could not find an existing LSM hook for this in v7.2-rc3. If this is a
known and accepted limitation (the v30 Landlock series was synchronized
with mount_setattr(2)), then documenting it in landlock.rst -- the
"Filesystem topology modification" section currently names only mount(2)
and pivot_root(2) -- would already help. Otherwise, the consistent fix
would be to add a security_sb_mount_setattr() LSM hook in
do_mount_setattr() and wire Landlock's topology denial to it, mirroring
hook_sb_remount(). I am happy to prepare that patch (plus a
tools/testing/selftests/landlock/ test) if you agree with the direction.
Thanks,
Vivek
^ permalink raw reply
* Re: [PATCH] apparmor: leverage audit_log_n_untrustedstring() when possible
From: Ryan Lee @ 2026-07-17 23:52 UTC (permalink / raw)
To: Paul Moore; +Cc: Georgia Garcia, John Johansen, linux-security-module, apparmor
In-Reply-To: <CAHC9VhQNthghRo8S-w8BQhe46ur8vhUyzW+xkOngAJQUb6O-Ug@mail.gmail.com>
On Fri, Jul 17, 2026 at 2:58 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Fri, Jul 17, 2026 at 5:54 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Fri, Jul 17, 2026 at 5:52 PM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > Make use of the audit_log_n_untrustedstring() function to simplify the
> > > code in aa_label_xaudit().
> > >
> > > Signed-off-by: Paul Moore <paul@paul-moore.com>
> > > ---
> > > security/apparmor/label.c | 5 +----
> > > 1 file changed, 1 insertion(+), 4 deletions(-)
> >
> > Untested beyond a basic compile, but I noticed this while looking at
> > something else (unrelated) and wanted to send it to the list before I
> > forgot about it ...
>
> My apologies Georgia, I thought I had copied your email into the
> original posting but it appears I copied John's email twice into my
> posting script (which helpfully de-duped it).
>
> Sorry about that.
>
> > > diff --git a/security/apparmor/label.c b/security/apparmor/label.c
> > > index 3fd384d8c41a..a165cadf8249 100644
> > > --- a/security/apparmor/label.c
> > > +++ b/security/apparmor/label.c
> > > @@ -1743,10 +1743,7 @@ void aa_label_xaudit(struct audit_buffer *ab, struct aa_ns *ns,
> > > str = (char *) label->hname;
> > > len = strlen(str);
> > > }
> > > - if (audit_string_contains_control(str, len))
> > > - audit_log_n_hex(ab, str, len);
> > > - else
> > > - audit_log_n_string(ab, str, len);
> > > + audit_log_n_untrustedstring(ab, str, len);
> > >
> > > kfree(name);
> > > }
> > > --
> > > 2.55.0
>
> --
> paul-moore.com
>
Reviewed-By: Ryan Lee <ryan.lee@canonical.com>
^ permalink raw reply
* [PATCH v2 3/3] landlock: Document LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS
From: Justin Suess @ 2026-07-17 22:03 UTC (permalink / raw)
To: gnoack3000, mic; +Cc: linux-kernel, linux-security-module, Justin Suess
In-Reply-To: <20260717220320.1030123-1-utilityemal77@gmail.com>
Document atomically setting no_new_privs with ruleset enforcement,
following the same compatibility section style as previous ABI
additions.
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
Documentation/userspace-api/landlock.rst | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 5a63d4476c1c..ec87d35f4715 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -8,7 +8,7 @@ Landlock: unprivileged access control
=====================================
:Author: Mickaël Salaün
-:Date: June 2026
+:Date: July 2026
The goal of Landlock is to enable restriction of ambient rights (e.g. global
filesystem or network access) for a set of processes. Because Landlock
@@ -789,6 +789,18 @@ when at least one sys_landlock_add_rule() call is made for it with the
``LANDLOCK_ADD_RULE_QUIET`` flag, additional add-rule calls for the same
object without this flag do not clear it.
+Atomic no_new_privs (ABI < 11)
+------------------------------
+
+Starting with the Landlock ABI version 11, sys_landlock_restrict_self()
+accepts the ``LANDLOCK_RESTRICT_SELF_NO_NEW_PRIVS`` flag, which sets the
+no_new_privs attribute of the calling thread atomically with the enforcement
+of the ruleset: no_new_privs is set if and only if the call succeeds. This
+removes the need for a prior :manpage:`prctl(2)` ``PR_SET_NO_NEW_PRIVS``
+call, and with it the ``CAP_SYS_ADMIN`` requirement. When combined with
+``LANDLOCK_RESTRICT_SELF_TSYNC``, no_new_privs is set on all threads of the
+process.
+
.. _kernel_support:
Kernel support
--
2.54.0
^ permalink raw reply related
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