linux-kernel - Re: [PATCH] loop: don't change loop device under exclusive opener in loop_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2crvwmytxw5splvtauxdq6o3dt4rnnzuy22vcub45rjk354alr@6m66k3ucoics>
Date: Wed, 17 Dec 2025 18:48:47 +0100
From: Jan Kara <jack@...e.cz>
To: Raphael Pinsonneault-Thibeault <rpthibeault@...il.com>
Cc: axboe@...nel.dk, linux-block@...r.kernel.org, 
	linux-kernel@...r.kernel.org, jack@...e.cz, cascardo@...lia.com, 
	linux-kernel-mentees@...ts.linux.dev, skhan@...uxfoundation.org, 
	syzbot+3ee481e21fd75e14c397@...kaller.appspotmail.com
Subject: Re: [PATCH] loop: don't change loop device under exclusive opener in
 loop_set_status

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@...kaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=3ee481e21fd75e14c397
> > Tested-by: syzbot+3ee481e21fd75e14c397@...kaller.appspotmail.com
> > Signed-off-by: Raphael Pinsonneault-Thibeault <rpthibeault@...il.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@...e.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@...e.com>
SUSE Labs, CR