public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to prevent out-of-bounds access
@ 2026-02-24 23:14 Deepanshu Kartikey
  2026-03-10  1:47 ` Deepanshu Kartikey
  2026-03-26  5:47 ` Theodore Tso
  0 siblings, 2 replies; 6+ messages in thread
From: Deepanshu Kartikey @ 2026-02-24 23:14 UTC (permalink / raw)
  To: tytso, adilger.kernel
  Cc: linux-ext4, linux-kernel, Deepanshu Kartikey,
	syzbot+fb32afec111a7d61b939

When mounting a corrupted ext4 filesystem, the inode's i_extra_isize
can be set to a value that leaves insufficient space in the inode for
the inline xattr header and entries. While ext4_iget() validates that
i_extra_isize fits within the inode size, it does not account for the
additional sizeof(ext4_xattr_ibody_header) needed by IHDR/IFIRST.

This results in IFIRST(header) pointing at or beyond ITAIL(raw_inode),
leaving no room for even the 4-byte terminator entry. When
xattr_find_entry() is subsequently called, IS_LAST_ENTRY() reads 4
bytes from this out-of-bounds pointer, triggering a use-after-free.

For example, with EXT4_INODE_SIZE=256 and i_extra_isize=124:
  - ext4_iget() check: 128 + 124 = 252 <= 256, passes
  - IFIRST = offset 252 + 4 (xattr header) = 256
  - ITAIL  = 256
  - IS_LAST_ENTRY() reads 4 bytes at offset 256, past the inode buffer

Fix this by validating in ext4_xattr_ibody_get() that there is enough
space between IFIRST(header) and ITAIL for at least a 4-byte read
before calling xattr_find_entry(). Return -EFSCORRUPTED if the inline
xattr region is too small.

Reported-by: syzbot+fb32afec111a7d61b939@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=fb32afec111a7d61b939
Tested-by: syzbot+fb32afec111a7d61b939@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 fs/ext4/xattr.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 7bf9ba19a89d..5080ec44228a 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -652,6 +652,13 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
 	header = IHDR(inode, raw_inode);
 	end = ITAIL(inode, raw_inode);
 	entry = IFIRST(header);
+
+	if ((void *)entry + sizeof(__u32) > end) {
+		EXT4_ERROR_INODE(inode, "inline xattr region overflow");
+		error = -EFSCORRUPTED;
+		goto cleanup;
+	}
+
 	error = xattr_find_entry(inode, &entry, end, name_index, name, 0);
 	if (error)
 		goto cleanup;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to prevent out-of-bounds access
  2026-02-24 23:14 [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to prevent out-of-bounds access Deepanshu Kartikey
@ 2026-03-10  1:47 ` Deepanshu Kartikey
  2026-03-26  5:47 ` Theodore Tso
  1 sibling, 0 replies; 6+ messages in thread
From: Deepanshu Kartikey @ 2026-03-10  1:47 UTC (permalink / raw)
  To: tytso, adilger.kernel
  Cc: linux-ext4, linux-kernel, syzbot+fb32afec111a7d61b939

On Wed, Feb 25, 2026 at 4:44 AM Deepanshu Kartikey
<kartikey406@gmail.com> wrote:
>
> When mounting a corrupted ext4 filesystem, the inode's i_extra_isize
> can be set to a value that leaves insufficient space in the inode for
> the inline xattr header and entries. While ext4_iget() validates that
> i_extra_isize fits within the inode size, it does not account for the
> additional sizeof(ext4_xattr_ibody_header) needed by IHDR/IFIRST.
>
> This results in IFIRST(header) pointing at or beyond ITAIL(raw_inode),
> leaving no room for even the 4-byte terminator entry. When
> xattr_find_entry() is subsequently called, IS_LAST_ENTRY() reads 4
> bytes from this out-of-bounds pointer, triggering a use-after-free.
>
> For example, with EXT4_INODE_SIZE=256 and i_extra_isize=124:
>   - ext4_iget() check: 128 + 124 = 252 <= 256, passes
>   - IFIRST = offset 252 + 4 (xattr header) = 256
>   - ITAIL  = 256
>   - IS_LAST_ENTRY() reads 4 bytes at offset 256, past the inode buffer
>
> Fix this by validating in ext4_xattr_ibody_get() that there is enough
> space between IFIRST(header) and ITAIL for at least a 4-byte read
> before calling xattr_find_entry(). Return -EFSCORRUPTED if the inline
> xattr region is too small.
>
> Reported-by: syzbot+fb32afec111a7d61b939@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=fb32afec111a7d61b939
> Tested-by: syzbot+fb32afec111a7d61b939@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> ---
>  fs/ext4/xattr.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
> index 7bf9ba19a89d..5080ec44228a 100644
> --- a/fs/ext4/xattr.c
> +++ b/fs/ext4/xattr.c
> @@ -652,6 +652,13 @@ ext4_xattr_ibody_get(struct inode *inode, int name_index, const char *name,
>         header = IHDR(inode, raw_inode);
>         end = ITAIL(inode, raw_inode);
>         entry = IFIRST(header);
> +
> +       if ((void *)entry + sizeof(__u32) > end) {
> +               EXT4_ERROR_INODE(inode, "inline xattr region overflow");
> +               error = -EFSCORRUPTED;
> +               goto cleanup;
> +       }
> +
>         error = xattr_find_entry(inode, &entry, end, name_index, name, 0);
>         if (error)
>                 goto cleanup;
> --
> 2.34.1
>

Hi,

Gentle ping on this patch. It has been about two weeks since I
submitted it.

This fixes a KASAN use-after-free in xattr_find_entry() reported by
syzbot. The issue occurs when a corrupted ext4 filesystem has an
i_extra_isize value that leaves insufficient space for inline xattr
entries. The syzbot test has confirmed the fix.

syzbot report: https://syzkaller.appspot.com/bug?extid=fb32afec111a7d61b939

Please let me know if there are any concerns or if any changes are
needed.

Thanks,
Deepanshu Kartikey

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to prevent out-of-bounds access
  2026-02-24 23:14 [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to prevent out-of-bounds access Deepanshu Kartikey
  2026-03-10  1:47 ` Deepanshu Kartikey
@ 2026-03-26  5:47 ` Theodore Tso
  2026-03-27 14:32   ` Deepanshu Kartikey
  1 sibling, 1 reply; 6+ messages in thread
From: Theodore Tso @ 2026-03-26  5:47 UTC (permalink / raw)
  To: Deepanshu Kartikey
  Cc: adilger.kernel, linux-ext4, linux-kernel,
	syzbot+fb32afec111a7d61b939

On Wed, Feb 25, 2026 at 04:44:29AM +0530, Deepanshu Kartikey wrote:
> When mounting a corrupted ext4 filesystem, the inode's i_extra_isize
> can be set to a value that leaves insufficient space in the inode for
> the inline xattr header and entries. While ext4_iget() validates that
> i_extra_isize fits within the inode size, it does not account for the
> additional sizeof(ext4_xattr_ibody_header) needed by IHDR/IFIRST.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to prevent out-of-bounds access
  2026-03-26  5:47 ` Theodore Tso
@ 2026-03-27 14:32   ` Deepanshu Kartikey
  2026-03-27 16:31     ` SYZKALLER BUG: messing with a mounted file system via loop ioctls (was: Re: [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to) " Theodore Tso
  0 siblings, 1 reply; 6+ messages in thread
From: Deepanshu Kartikey @ 2026-03-27 14:32 UTC (permalink / raw)
  To: Theodore Tso
  Cc: adilger.kernel, linux-ext4, linux-kernel,
	syzbot+fb32afec111a7d61b939

On Thu, Mar 26, 2026 at 11:17 AM Theodore Tso <tytso@mit.edu> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* SYZKALLER BUG: messing with a mounted file system via loop ioctls (was: Re: [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to) prevent out-of-bounds access
  2026-03-27 14:32   ` Deepanshu Kartikey
@ 2026-03-27 16:31     ` Theodore Tso
  2026-03-28 15:02       ` Deepanshu Kartikey
  0 siblings, 1 reply; 6+ messages in thread
From: Theodore Tso @ 2026-03-27 16:31 UTC (permalink / raw)
  To: Deepanshu Kartikey
  Cc: adilger.kernel, linux-ext4, linux-kernel,
	syzbot+fb32afec111a7d61b939, syzkaller

On Fri, Mar 27, 2026 at 08:02:30PM +0530, Deepanshu Kartikey wrote:
> 
> 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.

I consider this more a defect in Syzkaller than in ext4.  Syzkaller
already unsets CONFIG_BLK_DEV_WRITE_MOUNTED because modifying a
mounted file system is not considered a valid security concern.
Unfortunately, the syzkaller fuzzer which corrupts a mounted file
system by messing with a loop device using a privileged ioctl bypasses
!CONFIG_DEV_BLK_WRITE_MOUNTED.

I'll accept a change to add a to check_xattrs(), which will protect
against static corruptions, but adding an extra check to a hotpath
(this would slow down SELinux, which aggressively needs to read the
file's sid) to protect against a corruption issue which requires root
privs is not something I'm interested in.

My priority is improving ext4 --- not reducing the syzkaller reports
against ext4 to zero, since there are false positives like this.  For
people who care about reducing the syzkaller report gamification, my
suggestion is to either extend CONFIG_BLK_DEV_WRITE_MOUNTED to prevent
these loop device reconfiguration while a file system is mounted on
that loop device, or to fix syzkaller to not create fuzzers like this.

Cheers,

						- Ted








> 
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: SYZKALLER BUG: messing with a mounted file system via loop ioctls (was: Re: [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to) prevent out-of-bounds access
  2026-03-27 16:31     ` SYZKALLER BUG: messing with a mounted file system via loop ioctls (was: Re: [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to) " Theodore Tso
@ 2026-03-28 15:02       ` Deepanshu Kartikey
  0 siblings, 0 replies; 6+ messages in thread
From: Deepanshu Kartikey @ 2026-03-28 15:02 UTC (permalink / raw)
  To: Theodore Tso
  Cc: adilger.kernel, linux-ext4, linux-kernel,
	syzbot+fb32afec111a7d61b939, syzkaller

On Fri, Mar 27, 2026 at 10:01 PM Theodore Tso <tytso@mit.edu> wrote:
>
> On Fri, Mar 27, 2026 at 08:02:30PM +0530, Deepanshu Kartikey wrote:
> >
> > 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.
>
> I consider this more a defect in Syzkaller than in ext4.  Syzkaller
> already unsets CONFIG_BLK_DEV_WRITE_MOUNTED because modifying a
> mounted file system is not considered a valid security concern.
> Unfortunately, the syzkaller fuzzer which corrupts a mounted file
> system by messing with a loop device using a privileged ioctl bypasses
> !CONFIG_DEV_BLK_WRITE_MOUNTED.
>
> I'll accept a change to add a to check_xattrs(), which will protect
> against static corruptions, but adding an extra check to a hotpath
> (this would slow down SELinux, which aggressively needs to read the
> file's sid) to protect against a corruption issue which requires root
> privs is not something I'm interested in.
>
> My priority is improving ext4 --- not reducing the syzkaller reports
> against ext4 to zero, since there are false positives like this.  For
> people who care about reducing the syzkaller report gamification, my
> suggestion is to either extend CONFIG_BLK_DEV_WRITE_MOUNTED to prevent
> these loop device reconfiguration while a file system is mounted on
> that loop device, or to fix syzkaller to not create fuzzers like this.
>
> Cheers,
>
>                                                 - Ted
>

Thanks for the clarification. I have sent the patch v2 with check in
check_xattrs.

Deepanshu

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-03-28 15:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-24 23:14 [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to prevent out-of-bounds access Deepanshu Kartikey
2026-03-10  1:47 ` Deepanshu Kartikey
2026-03-26  5:47 ` Theodore Tso
2026-03-27 14:32   ` Deepanshu Kartikey
2026-03-27 16:31     ` SYZKALLER BUG: messing with a mounted file system via loop ioctls (was: Re: [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to) " Theodore Tso
2026-03-28 15:02       ` Deepanshu Kartikey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox