zohar@linux.ibm.com, roberto.sassu@huawei.com

zohar@linux.ibm.com, roberto.sassu@huawei.com

[Fabrice Derepas]

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.


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 <fabrice.derepas@canonical.com>

Re: zohar@linux.ibm.com, roberto.sassu@huawei.com

[Roberto Sassu]

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 <fabrice.derepas@canonical.com>

[PATCH] evm: Bound writes in evm_read_protected_xattrs() to buffer_size

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

ima_eventinodexattrs_init_common() calls evm_read_protected_xattrs()
first with a NULL buffer to obtain the required size, allocates a buffer
of that size, and then calls it again to fill it. Nothing keeps the
result stable between the two calls: a protected xattr may be added to
evm_config_xattrnames through the evm_xattrs securityfs file (with
CONFIG_EVM_ADD_XATTRS=y), or a protected xattr may be set on, removed
from, or changed in the measured file in the meantime.

However, the 'n' (names) and 'l' (lengths) cases write into the buffer
without checking buffer_size, so if the data grew between the two calls,
they write past the end of the allocation.

The 'v' (values) case passes the remaining space to __vfs_getxattr(),
so the copy itself is bounded, but advances total_size by the length
returned by the earlier probing __vfs_getxattr() call, rather than by
the length actually copied. This causes two problems:

- If the probed size fits in the remaining space, but the value shrinks
  before the copy, total_size does not exceed buffer_size, but the
  returned size covers bytes of the buffer that were not written.

- If the probed size exceeds the remaining space, but the value shrinks
  enough to fit before the copy, total_size exceeds buffer_size after
  adding the probed size.

In addition, once the buffer is full, any further non-empty xattr (e.g.
one added to the list or set on the file after the buffer size was
determined) is read with a zero remaining size. Some filesystems, such
as XFS and btrfs, take a zero size as a request for the xattr length,
return it without copying, and total_size exceeds buffer_size as well.

When total_size exceeds buffer_size, either the next iteration passes a
negative remaining size to __vfs_getxattr(), converted to a huge size_t,
causing a write past the end of the buffer, or the caller reads past
the end of the buffer.

Return -ERANGE in the 'n' and 'l' cases when the data no longer fits in
the buffer. In the 'v' case, skip the copy if the probed length is zero,
return -ERANGE if no space is left, so that __vfs_getxattr() is never
called with a zero size, and advance total_size by the number of bytes
actually copied. Since __vfs_getxattr() then either copies at most the
remaining space or fails with -ERANGE, total_size never exceeds
buffer_size. Also document the -ERANGE return in the kernel-doc.

On -ERANGE, ima_eventinodexattrs_init_common() records an empty template
field, as it already does for the other errors.

Fixes: 8314b6732ae4 ("ima: Define new template fields xattrnames, xattrlengths and xattrvalues")
Reported-by: Fabrice Derepas <fabrice.derepas@canonical.com>
Closes: https://lore.kernel.org/linux-integrity/CAD2AHCbuQyKwR27hNcYRhU7njCeigmQoL9x9i25zb0N5-8Kx+A@mail.gmail.com/
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/evm/evm_main.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 5be9349afcd0..a29b7f80e55d 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -369,6 +369,10 @@ int evm_protected_xattr_if_enabled(const char *req_xattr_name)
  * given dentry and return the total size of copied data. If buffer is NULL,
  * just return the total size.
  *
+ * The set of protected xattrs and their values may change between a call with
+ * a NULL buffer and a subsequent call with a buffer of the returned size. If
+ * the data no longer fits in @buffer_size, -ERANGE is returned.
+ *
  * Returns the total size on success, a negative value on error.
  */
 int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
@@ -389,6 +393,9 @@ int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
 		case 'n':
 			size = strlen(xattr->name) + 1;
 			if (buffer) {
+				if (size > buffer_size - total_size)
+					return -ERANGE;
+
 				if (total_size)
 					*(buffer + total_size - 1) = '|';
 
@@ -398,6 +405,9 @@ int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
 		case 'l':
 			size = sizeof(u32);
 			if (buffer) {
+				if (size > buffer_size - total_size)
+					return -ERANGE;
+
 				if (canonical_fmt)
 					rc = (__force int)cpu_to_le32(rc);
 
@@ -407,12 +417,22 @@ int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
 		case 'v':
 			size = rc;
 			if (buffer) {
+				/* Nothing to copy. */
+				if (!size)
+					break;
+
+				/* A zero size would query the length. */
+				if (buffer_size - total_size == 0)
+					return -ERANGE;
+
 				rc = __vfs_getxattr(dentry,
 					d_backing_inode(dentry), xattr->name,
 					buffer + total_size,
 					buffer_size - total_size);
 				if (rc < 0)
 					return rc;
+
+				size = rc;
 			}
 			break;
 		default:
-- 
2.43.0

Re: [PATCH] evm: Bound writes in evm_read_protected_xattrs() to buffer_size

[sashiko-bot]

> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> ima_eventinodexattrs_init_common() calls evm_read_protected_xattrs()
> first with a NULL buffer to obtain the required size, allocates a buffer
> of that size, and then calls it again to fill it. Nothing keeps the
> result stable between the two calls: a protected xattr may be added to
> evm_config_xattrnames through the evm_xattrs securityfs file (with
> CONFIG_EVM_ADD_XATTRS=y), or a protected xattr may be set on, removed
> from, or changed in the measured file in the meantime.
> 
> However, the 'n' (names) and 'l' (lengths) cases write into the buffer
> without checking buffer_size, so if the data grew between the two calls,
> they write past the end of the allocation.
> 
> The 'v' (values) case passes the remaining space to __vfs_getxattr(),
> [ ... ]
> 
> Fixes: 8314b6732ae4 ("ima: Define new template fields xattrnames, xattrlengths and xattrvalues")
> Reported-by: Fabrice Derepas <fabrice.derepas@canonical.com>
> Closes: https://lore.kernel.org/linux-integrity/CAD2AHCbuQyKwR27hNcYRhU7njCeigmQoL9x9i25zb0N5-8Kx+A@mail.gmail.com/
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029142707.2602008-1-roberto.sassu@huaweicloud.com?part=1

Re: [PATCH] evm: Bound writes in evm_read_protected_xattrs() to buffer_size

[Roberto Sassu]

On Thu, 2026-10-29 at 15:27 +0100, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> ima_eventinodexattrs_init_common() calls evm_read_protected_xattrs()
> first with a NULL buffer to obtain the required size, allocates a buffer
> of that size, and then calls it again to fill it. Nothing keeps the
> result stable between the two calls: a protected xattr may be added to
> evm_config_xattrnames through the evm_xattrs securityfs file (with
> CONFIG_EVM_ADD_XATTRS=y), or a protected xattr may be set on, removed
> from, or changed in the measured file in the meantime.
> 
> However, the 'n' (names) and 'l' (lengths) cases write into the buffer
> without checking buffer_size, so if the data grew between the two calls,
> they write past the end of the allocation.
> 
> The 'v' (values) case passes the remaining space to __vfs_getxattr(),
> so the copy itself is bounded, but advances total_size by the length
> returned by the earlier probing __vfs_getxattr() call, rather than by
> the length actually copied. This causes two problems:
> 
> - If the probed size fits in the remaining space, but the value shrinks
>   before the copy, total_size does not exceed buffer_size, but the
>   returned size covers bytes of the buffer that were not written.
> 
> - If the probed size exceeds the remaining space, but the value shrinks
>   enough to fit before the copy, total_size exceeds buffer_size after
>   adding the probed size.
> 
> In addition, once the buffer is full, any further non-empty xattr (e.g.
> one added to the list or set on the file after the buffer size was
> determined) is read with a zero remaining size. Some filesystems, such
> as XFS and btrfs, take a zero size as a request for the xattr length,
> return it without copying, and total_size exceeds buffer_size as well.
> 
> When total_size exceeds buffer_size, either the next iteration passes a
> negative remaining size to __vfs_getxattr(), converted to a huge size_t,
> causing a write past the end of the buffer, or the caller reads past
> the end of the buffer.
> 
> Return -ERANGE in the 'n' and 'l' cases when the data no longer fits in
> the buffer. In the 'v' case, skip the copy if the probed length is zero,
> return -ERANGE if no space is left, so that __vfs_getxattr() is never
> called with a zero size, and advance total_size by the number of bytes
> actually copied. Since __vfs_getxattr() then either copies at most the
> remaining space or fails with -ERANGE, total_size never exceeds
> buffer_size. Also document the -ERANGE return in the kernel-doc.
> 
> On -ERANGE, ima_eventinodexattrs_init_common() records an empty template
> field, as it already does for the other errors.
> 
> Fixes: 8314b6732ae4 ("ima: Define new template fields xattrnames, xattrlengths and xattrvalues")
> Reported-by: Fabrice Derepas <fabrice.derepas@canonical.com>

Hi Fabrice

can change to Co-developed-by: Fabrice Derepas <fabrice.derepas@canonical.com>

if you provide your Signed-off-by.

Thanks

Roberto

> Closes: https://lore.kernel.org/linux-integrity/CAD2AHCbuQyKwR27hNcYRhU7njCeigmQoL9x9i25zb0N5-8Kx+A@mail.gmail.com/
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  security/integrity/evm/evm_main.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 5be9349afcd0..a29b7f80e55d 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -369,6 +369,10 @@ int evm_protected_xattr_if_enabled(const char *req_xattr_name)
>   * given dentry and return the total size of copied data. If buffer is NULL,
>   * just return the total size.
>   *
> + * The set of protected xattrs and their values may change between a call with
> + * a NULL buffer and a subsequent call with a buffer of the returned size. If
> + * the data no longer fits in @buffer_size, -ERANGE is returned.
> + *
>   * Returns the total size on success, a negative value on error.
>   */
>  int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
> @@ -389,6 +393,9 @@ int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
>  		case 'n':
>  			size = strlen(xattr->name) + 1;
>  			if (buffer) {
> +				if (size > buffer_size - total_size)
> +					return -ERANGE;
> +
>  				if (total_size)
>  					*(buffer + total_size - 1) = '|';
>  
> @@ -398,6 +405,9 @@ int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
>  		case 'l':
>  			size = sizeof(u32);
>  			if (buffer) {
> +				if (size > buffer_size - total_size)
> +					return -ERANGE;
> +
>  				if (canonical_fmt)
>  					rc = (__force int)cpu_to_le32(rc);
>  
> @@ -407,12 +417,22 @@ int evm_read_protected_xattrs(struct dentry *dentry, u8 *buffer,
>  		case 'v':
>  			size = rc;
>  			if (buffer) {
> +				/* Nothing to copy. */
> +				if (!size)
> +					break;
> +
> +				/* A zero size would query the length. */
> +				if (buffer_size - total_size == 0)
> +					return -ERANGE;
> +
>  				rc = __vfs_getxattr(dentry,
>  					d_backing_inode(dentry), xattr->name,
>  					buffer + total_size,
>  					buffer_size - total_size);
>  				if (rc < 0)
>  					return rc;
> +
> +				size = rc;
>  			}
>  			break;
>  		default: