Re: [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to prevent out-of-bounds access

[Deepanshu Kartikey]

On Thu, Mar 26, 2026 at 11:17 AM Theodore Tso <tytso@xxxxxxx> wrote:
>
>
> Actually, it does more than that.  It also calls xattr_check_inode()
> which should validate the xattr block in the inode.
>
> So instead of adding the check in ext4_xattr_ibody_get(), we should
> fix the check in __xattr_check_inode().  This is preferable since it's
> more efficient than checking every time we try to fetch an extended
> attribute, instead of validating it when the inode is read from the
> inode table block.
>
>                                         - Ted


Subject: Re: [PATCH] ext4: add bounds check in ext4_xattr_ibody_get()
to prevent out-of-bounds access

Hi Ted,

Thank you for the review. I tried moving the fix to check_xattrs()
as you suggested, but the syzbot reproducer still crashes. I added
printk statements to trace the code path and found the root cause.

The issue is that __xattr_check_inode() runs once during ext4_iget(),
but ext4_xattr_ibody_get() re-reads the inode from disk via
ext4_get_inode_loc() on every call. The reproducer exploits this by
shrinking the loop device after mount, causing the re-read to return
corrupted data.

Here is the relevant debug output showing the sequence:

1) Inode 12 is loaded, __xattr_check_inode passes with gap=92:

  DEBUG: inode 12: calling ext4_iget_extra_inode
  DEBUG: inode 12: __xattr_check_inode called,
         IFIRST=ffff88805b9f9fa4 end=ffff88805b9fa000 gap=92

2) The loop device is then shrunk:

  loop0: detected capacity change from 1024 to 64

3) First access after shrink, inode re-read still looks okay:

  DEBUG: inode 12: ibody_get
         IFIRST=ffff88806edbcfa4 end=ffff88806edbd000 gap=92

4) Second access, re-read returns corrupted data with gap=-4:

  DEBUG: inode 12: ibody_get
         IFIRST=ffff88806edbd004 end=ffff88806edbd000 gap=-4

5) Crash follows immediately in xattr_find_entry().

Note that IFIRST and end are at different addresses between steps
1 and 4 - ext4_get_inode_loc() returned a different buffer_head
with corrupted i_extra_isize, pushing IFIRST 4 bytes past end.
The initial __xattr_check_inode() validation cannot protect
against this because it validated the original buffer, not the
corrupted re-read.

I think we need both fixes:

  1) The bounds fix in check_xattrs() as you suggested, changing
     (void *)next >= end to (void *)next + sizeof(u32) > end

  2) A bounds check in ext4_xattr_ibody_get() before calling
     xattr_find_entry(), to catch corrupted re-reads

Should I submit a v2 with both fixes as a patch series?

Thanks,
Deepanshu Kartikey