Re: [PATCH] loop: don't change loop device under exclusive opener in loop_set_status

Jan Kara
2025-12-17T17:48:50Z

On Tue 02-12-25 11:07:06, Jan Kara wrote:
> On Fri 14-11-25 09:42:05, Raphael Pinsonneault-Thibeault wrote:
> > loop_set_status() is allowed to change the loop device while there
> > are other openers of the device, even exclusive ones.
> >
> > In this case, it causes a KASAN: slab-out-of-bounds Read in
> > ext4_search_dir(), since when looking for an entry in an inlined
> > directory, e_value_offs is changed underneath the filesystem by
> > loop_set_status().
> >
> > Fix the problem by forbidding loop_set_status() from modifying the loop
> > device while there are exclusive openers of the device. This is similar
> > to the fix in loop_configure() by commit 33ec3e53e7b1 ("loop: Don't
> > change loop device under exclusive opener") alongside commit ecbe6bc0003b
> > ("block: use bd_prepare_to_claim directly in the loop driver").
> >
> > Reported-by: syzbot+3ee481e21fd75e14c397@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=3ee481e21fd75e14c397
> > Tested-by: syzbot+3ee481e21fd75e14c397@syzkaller.appspotmail.com
> > Signed-off-by: Raphael Pinsonneault-Thibeault <rpthibeault@gmail.com>
>
> This patch looks mostly good to me. Just one comment:
>
> > -loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
> > +loop_set_status(struct loop_device *lo, blk_mode_t mode,
> > + struct block_device *bdev, const struct loop_info64 *info)
> > {
> > int err;
> > bool partscan = false;
> > bool size_changed = false;
> > unsigned int memflags;
> >
> > + /*
> > + * If we don't hold exclusive handle for the device, upgrade to it
> > + * here to avoid changing device under exclusive owner.
> > + */
> > + if (!(mode & BLK_OPEN_EXCL)) {
> > + err = bd_prepare_to_claim(bdev, loop_set_status, NULL);
> > + if (err)
> > + goto out_reread_partitions;
> > + }
> > +
>
> So now any LOOP_SET_STATUS call will fail for device that is already
> exclusively open. There are some operations (like modifying the AUTOCLEAR
> flag or loop device name) that are safe even for a device with a mounted
> filesystem. I wouldn't probably bother with that now but I wanted to note
> that there may be valid uses of LOOP_SET_STATUS even for an exclusively
> open loop device and if there are users of that out there we might need to
> refine this a bit. Anyway for now feel free to add:
>
> Reviewed-by: Jan Kara <jack@suse.cz>

Raphael, this patch seems to have fallen through the cracks. Can you please
resend it to Jens with by Reviewed-by tag so that he picks it up? Thanks!

Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR