public inbox for linux-ext4@vger.kernel.org
 help / color / mirror / Atom feed
From: "Theodore Tso" <tytso@mit.edu>
To: Deepanshu Kartikey <kartikey406@gmail.com>
Cc: adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	syzbot+fb32afec111a7d61b939@syzkaller.appspotmail.com,
	syzkaller@googlegroups.com
Subject: 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
Date: Fri, 27 Mar 2026 11:31:35 -0500	[thread overview]
Message-ID: <20260327163135.GE4383@macsyma.local> (raw)
In-Reply-To: <CADhLXY5E_4Hi=Uzu3LPwg_YNgcFR+UcAbfY-XzwVnmFBaJqjkA@mail.gmail.com>

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

  reply	other threads:[~2026-03-27 16:31 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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     ` Theodore Tso [this message]
2026-03-28 15:02       ` SYZKALLER BUG: messing with a mounted file system via loop ioctls (was: Re: [PATCH] ext4: add bounds check in ext4_xattr_ibody_get() to) " Deepanshu Kartikey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260327163135.GE4383@macsyma.local \
    --to=tytso@mit.edu \
    --cc=adilger.kernel@dilger.ca \
    --cc=kartikey406@gmail.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=syzbot+fb32afec111a7d61b939@syzkaller.appspotmail.com \
    --cc=syzkaller@googlegroups.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox