On Sat, 2026-07-18 at 08:52 +0200, Fabrice Derepas wrote:
> evm_read_protected_xattrs() accepts a buffer_size parameter but never
> validates it on the 'n' (names) and 'l' (lengths) paths. The sole
> in-tree caller, ima_eventinodexattrs_init_common(), uses a lock-free
> measure-then-allocate-then-fill pattern across two separate calls. A
> concurrent write to /sys/kernel/security/integrity/evm/evm_xattrs
> between the measure and fill calls causes the fill to iterate more
> xattr entries than the buffer was sized for, producing a heap write
> past the end of the allocation.
Hi Fabrice
thanks for reporting the bug.
I need to say, also for everybody, please don't send AI-style bug
reports. I would like to receive a kernel-style commit message
explaining the problem and the solution in just a few sentences. Also,
for the next iteration, please make a real patch.
Thanks
Roberto
> Affected code
> -------------
> security/integrity/evm/evm_main.c, evm_read_protected_xattrs(),
> lines 374-425.
>
> The 'n' path writes at line 395 without checking total_size + size
> <= buffer_size. The 'l' path stores a u32 at line 404 with the same
> omission. The 'v' path at line 413 correctly passes
> (buffer_size - total_size) to __vfs_getxattr(), so the omission on
> the other two paths is inconsistent with clear design intent:
>
> switch (type) {
> case 'n':
> size = strlen(xattr->name) + 1;
> if (buffer) {
> if (total_size)
> *(buffer + total_size - 1) = '|';
>
> memcpy(buffer + total_size, xattr->name, size); /* line 395 -- no
> bounds check */
> }
> break;
> case 'l':
> size = sizeof(u32);
> if (buffer) {
> if (canonical_fmt)
> rc = (__force int)cpu_to_le32(rc);
>
> *(u32 *)(buffer + total_size) = rc; /* line 404 -- no
> bounds check */
> }
> break;
> case 'v':
> size = rc;
> if (buffer) {
> rc = __vfs_getxattr(dentry,
> d_backing_inode(dentry), xattr->name,
> buffer + total_size,
> buffer_size - total_size); /* line 413 -- has bounds check */
> if (rc < 0)
> return rc;
> }
> break;
>
>
> Root cause
> ----------
> Two defects combine:
>
> (1) Missing bounds check (function-level): the 'n' and 'l' paths write
> into buffer + total_size without verifying total_size + size <=
> buffer_size. The buffer_size parameter is accepted but unused on
> these paths.
>
> (2) TOCTOU race (caller-level): ima_eventinodexattrs_init_common()
> in ima_template_lib.c:703 calls evm_read_protected_xattrs() twice,
> separated by a kmalloc():
>
> line 713: rc = evm_read_protected_xattrs(..., NULL, 0, ...);
> line 718: buffer = kmalloc(rc, GFP_KERNEL);
> line 722: rc = evm_read_protected_xattrs(..., buffer, rc, ...);
>
> list_for_each_entry_lockless() provides RCU read-side traversal but
> does not stabilise the list across two separate calls. A concurrent
> list_add_tail_rcu() in evm_write_xattrs() (evm_secfs.c:263) between
> lines 713 and 722 causes the fill call to see more entries than the
> buffer accommodates.
>
> Note: xattr_list_mutex (evm_secfs.c:24) is held only by the writer;
> there is no read-side lock that would prevent the list from changing
> between the two calls.
>
> Neither defect alone is exploitable: without the race, buffer_size
> equals the measured size; without defect (1), an overflow would be
> caught. Together they produce a controlled heap write past the
> allocation.
>
>
> Reachability
> ------------
> Write path: evm_write_xattrs() (evm_secfs.c:179) handles writes to
> /sys/kernel/security/integrity/evm/evm_xattrs. It requires
> CAP_SYS_ADMIN and evm_xattrs_locked == 0 (checked at line 188).
> This path is only compiled when CONFIG_EVM_ADD_XATTRS=y.
>
> Read path: ima_eventinodexattrs_init_common() is called during IMA
> event logging for templates that include xattrnames or xattrlengths
> fields (e.g. a custom template with 'n' or 'l' fields). This runs
> on any file access (open, exec, mmap) matched by the active IMA
> policy.
>
> Concrete scenario:
>
> 1. IMA policy active with a template including xattrnames ('n').
> 2. Initial xattr list: security.selinux (17+1 bytes),
> security.ima (12+1 bytes) -- measure call returns 31.
> 3. kmalloc(31) allocates the buffer.
> 4. CAP_SYS_ADMIN process writes security.apparmor and
> security.capability to evm_xattrs before the fill call.
> 5. Fill call iterates 4 entries, needs ~62 bytes -- overflows
> the 31-byte allocation.
>
> The attacker controls write timing and can trigger IMA measurements
> on demand (e.g. by opening a file matching the IMA policy), making
> the race window practically reliable.
>
>
> Reproducer
> ----------
> The following reproduces the race by simulating the list growth
> between the two calls. Tested with gcc -fsanitize=address.
>
> /*
> * repro_evm_oob.c
> * Compile: gcc -fsanitize=address -g -o repro_evm_oob repro_evm_oob.c
> * Run: ./repro_evm_oob
> */
> #include <stdint.h>
> #include <stdlib.h>
> #include <string.h>
> #include <stdio.h>
>
> typedef uint32_t u32;
>
> struct xattr_entry {
> const char *name;
> int xattr_size;
> struct xattr_entry *next;
> };
>
> static struct xattr_entry *xattr_head = NULL;
>
> static void add_xattr(const char *name, int sz)
> {
> struct xattr_entry *e = malloc(sizeof(*e));
> e->name = name; e->xattr_size = sz; e->next = NULL;
> if (!xattr_head) { xattr_head = e; }
> else {
> struct xattr_entry *p = xattr_head;
> while (p->next) p = p->next;
> p->next = e;
> }
> }
>
> /* Mirrors evm_main.c:374-425 for type='n'. buffer_size is accepted
> * but never checked -- the bug. */
> static int evm_read_protected_xattrs_sim(uint8_t *buffer,
> int buffer_size, char type)
> {
> struct xattr_entry *xattr;
> int rc, size, total_size = 0;
>
> for (xattr = xattr_head; xattr; xattr = xattr->next) {
> rc = xattr->xattr_size;
> if (rc < 0) continue;
> switch (type) {
> case 'n':
> size = strlen(xattr->name) + 1;
> if (buffer) {
> if (total_size)
> *(buffer + total_size - 1) = '|';
> /* BUG: no check that total_size + size <= buffer_size */
> memcpy(buffer + total_size, xattr->name, size);
> }
> break;
> default:
> return -1;
> }
> total_size += size;
> }
> return total_size;
> }
>
> int main(void)
> {
> /* Initial list: 2 entries */
> add_xattr("security.selinux", 42);
> add_xattr("security.ima", 32);
>
> /* Measuring call (buffer=NULL) */
> int measured = evm_read_protected_xattrs_sim(NULL, 0, 'n');
> printf("[1] measured=%d\n", measured);
>
> uint8_t *buf = malloc(measured);
> if (!buf) return 1;
>
> /* TOCTOU: list grows before fill call */
> add_xattr("security.apparmor", 100);
> add_xattr("security.capability", 20);
>
> /* Filling call -- overflows */
> printf("[2] filling...\n");
> int rc = evm_read_protected_xattrs_sim(buf, measured, 'n');
> printf("[3] wrote %d into %d-byte buffer\n", rc, measured);
>
> free(buf);
> return 0;
> }
>
> Observed ASan output:
>
> ERROR: AddressSanitizer: heap-buffer-overflow on address 0x...
> WRITE of size 18 at 0x... thread T0
> #0 in memcpy ...
> #1 in evm_read_protected_xattrs_sim repro_evm_oob.c
> #2 in main repro_evm_oob.c
>
> 0x... is located 0 bytes after 30-byte region [0x..., 0x...)
> allocated by thread T0 here:
> #0 in malloc ...
> #1 in main repro_evm_oob.c
>
>
> Impact
> ------
> Heap write past an allocation. The attacker chooses the xattr names
> (write content) and the number of entries added (write size).
> Consequence ranges from a kernel crash (DoS) to, with heap layout
> control, a potential privilege escalation.
>
> Requirements: CAP_SYS_ADMIN (to write evm_xattrs), CONFIG_EVM=y,
> CONFIG_IMA=y, CONFIG_EVM_ADD_XATTRS=y, and an active IMA policy
> with a template containing xattrnames or xattrlengths fields.
> CAP_SYS_ADMIN inside a user namespace may suffice on configurations
> that permit it.
>
>
> Proposed fix
> ------------
> Add bounds checks to the 'n' and 'l' paths, mirroring the existing
> check on the 'v' path:
>
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -388,6 +388,8 @@
> case 'n':
> size = strlen(xattr->name) + 1;
> if (buffer) {
> + if (total_size + size > buffer_size)
> + return -ERANGE;
> if (total_size)
> *(buffer + total_size - 1) = '|';
>
> @@ -398,6 +400,8 @@
> case 'l':
> size = sizeof(u32);
> if (buffer) {
> + if (total_size + size > buffer_size)
> + return -ERANGE;
> if (canonical_fmt)
> rc = (__force int)cpu_to_le32(rc);
>
> A note on the return value: I have used -ERANGE rather than
> returning total_size (partial bytes written). The caller in
> ima_eventinodexattrs_init_common() treats negative returns as an
> error and falls through to kfree(buffer) without logging partial
> data, which is a safer failure mode than silently recording a
> truncated xattr list. The TOCTOU race that triggers the overflow
> is also an integrity anomaly (the xattr list changed under us), so
> treating it as an error rather than silently truncating seems
> correct. Maintainer preference on the return value is welcome.
>
> An alternative fix at the caller level would be to hold
> xattr_list_mutex around both calls in
> ima_eventinodexattrs_init_common(), but this requires exposing the
> mutex and the function-level bounds check is more robust against
> future callers.
>
>
> How this was found
> ------------------
> Found by applying the Squeeze Loop strategy ("The Squeeze Loop
> Strategy: Catching Coherent-and-Wrong Artifacts with an
> Author-Independent Executable Oracle," Zenodo DOI 10.5281/zenodo.20787816,
> 2026) on its C terrain - the deductive-verification analogue of
> the paper's Rust application.
>
> Signed-off-by: Fabrice Derepas <[email protected]>
