Message-ID: <okx6a3ngonajh7jrzc65ybd4i6bcnkc7gm4mggyo3jlm6s2ojx@yy5jcipsnd3l>
Date: Fri, 11 Jul 2025 18:42:08 +0200
From: Jan Kara <jack@...e.cz>
To: syzbot <syzbot+01ef7a8da81a975e1ccd@...kaller.appspotmail.com>
Cc: adilger.kernel@...ger.ca, anna.luese@...ien.de, brauner@...nel.org, 
	jack@...e.cz, jfs-discussion@...ts.sourceforge.net, libaokun1@...wei.com, 
	linkinjeon@...nel.org, linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	linux-kernel@...r.kernel.org, p.raghav@...sung.com, shaggy@...nel.org, sj1557.seo@...sung.com, 
	syzkaller-bugs@...glegroups.com, tytso@....edu
Subject: Re: [syzbot] [ext4?] WARNING in bdev_getblk

On Fri 11-07-25 17:51:40, Jan Kara wrote:
> On Fri 11-07-25 05:27:01, syzbot wrote:
> > syzbot has bisected this issue to:
> > 
> > commit 77eb64439ad52d8afb57bb4dae24a2743c68f50d
> > Author: Pankaj Raghav <p.raghav@...sung.com>
> > Date:   Thu Jun 26 11:32:23 2025 +0000
> > 
> >     fs/buffer: remove the min and max limit checks in __getblk_slow()
> > 
> > bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=127d8d82580000
> > start commit:   835244aba90d Add linux-next specific files for 20250709
> > git tree:       linux-next
> > final oops:     https://syzkaller.appspot.com/x/report.txt?x=117d8d82580000
> > console output: https://syzkaller.appspot.com/x/log.txt?x=167d8d82580000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=8396fd456733c122
> > dashboard link: https://syzkaller.appspot.com/bug?extid=01ef7a8da81a975e1ccd
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=115c40f0580000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=11856a8c580000
> > 
> > Reported-by: syzbot+01ef7a8da81a975e1ccd@...kaller.appspotmail.com
> > Fixes: 77eb64439ad5 ("fs/buffer: remove the min and max limit checks in __getblk_slow()")
> > 
> > For information about bisection process see: https://goo.gl/tpsmEJ#bisection
> 
> Ah, I see what's going on here. The reproducer mounts ext4 filesystem and
> sets block size on loop0 loop device to 32k using LOOP_SET_BLOCK_SIZE. Now
> because there are multiple reproducer running using various loop devices it
> can happen that we're setting blocksize during mount which obviously
> confuses the filesystem (and makes sb mismatch the bdev block size). It is
> really not a good idea to allow setting block size (or capacity for that
> matter) underneath an exclusive opener. The ioctl should have required
> exclusive open from the start but now it's too late to change that so we
> need to perform a similar dance with bd_prepare_to_claim() as in
> loop_configure() to grab temporary exclusive access... Sigh.
> 
> Anyway, the commit 77eb64439ad5 is just a victim that switched KERN_ERR
> messages in the log to WARN_ON so syzbot started to notice this breakage.

Sent fix here:
https://lore.kernel.org/all/20250711163202.19623-2-jack@suse.cz

								Honza

#syz test

>From 4aa776eb9b3967bff31087b7595ddcc902200056 Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@...e.cz>
Date: Fri, 11 Jul 2025 18:16:44 +0200
Subject: [PATCH] loop: Avoid updating block size under exclusive owner
X-Developer-Signature: v=1; a=openpgp-sha256; l=3277; i=jack@...e.cz;
 h=from:subject; bh=cdZuEh4Dk+/Xi2fTOZRQxovM+RzBRb1FW6MBEN4Icws=;
 b=owGbwMvMwME4Z+4qdvsUh5uMp9WSGDIKbZq0xH0Ml+s1mUn/FXh44vuGfB7/6WemZT/fv/SJRXZ0
 +Oc9nYzGLAyMHAyyYoosqyMval+bZ9S1NVRDBmYQKxPIFAYuTgGYyJE37P+TzLkNtVn5K1Q9Umaq3T
 Vmcb4tyOvXYsYcLfHv9g9F9zVruARylmxL8Gx73a307cwFM9/kww0b8vqYHvX33H2TnVeYsO+NW0RF
 KsOmd0cUjl7aWW4kFO6cOtNdIZ/HrHijk2bAKdYZ1wwucvbN27p4kRvXZfMu06WeZyz/7+FrF/9270
 Ostl1ml45ORWBlp5amyvXZvl33u5tr76znST50+oRf0to8n2afbSZCK3Ris+7MjpqnMo+lK+Sf5s/+
 L65hUjq2EwOyugu3PAk1khb9cHXhXN6anJo32VfsI6N+MsSzMxh+4O86wRg/98EH3obLLcIX2wOjKu
 es+vb1lH1Gx6ctQVE7E7Ok87oA
X-Developer-Key: i=jack@...e.cz; a=openpgp;
 fpr=93C6099A142276A28BBE35D815BC833443038D8C

Syzbot came up with a reproducer where a loop device block size is
changed underneath a mounted filesystem. This causes a mismatch between
the block device block size and the block size stored in the superblock
causing confusion in various places such as fs/buffer.c. The particular
issue triggered by syzbot was a warning in __getblk_slow() due to
requested buffer size not matching block device block size.

Fix the problem by getting exclusive hold of the loop device to change
its block size. This fails if somebody (such as filesystem) has already
an exclusive ownership of the block device and thus prevents modifying
the loop device under some exclusive owner which doesn't expect it.

Reported-by: syzbot+01ef7a8da81a975e1ccd@...kaller.appspotmail.com
Signed-off-by: Jan Kara <jack@...e.cz>
---
 drivers/block/loop.c | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 500840e4a74e..5cc72770e253 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1432,17 +1432,34 @@ static int loop_set_dio(struct loop_device *lo, unsigned long arg)
 	return 0;
 }
 
-static int loop_set_block_size(struct loop_device *lo, unsigned long arg)
+static int loop_set_block_size(struct loop_device *lo, blk_mode_t mode,
+			       struct block_device *bdev, unsigned long arg)
 {
 	struct queue_limits lim;
 	unsigned int memflags;
 	int err = 0;
 
-	if (lo->lo_state != Lo_bound)
-		return -ENXIO;
+	/*
+	 * 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_block_size, NULL);
+		if (err)
+			return err;
+	}
+
+	err = mutex_lock_killable(&lo->lo_mutex);
+	if (err)
+		goto abort_claim;
+
+	if (lo->lo_state != Lo_bound) {
+		err = -ENXIO;
+		goto unlock;
+	}
 
 	if (lo->lo_queue->limits.logical_block_size == arg)
-		return 0;
+		goto unlock;
 
 	sync_blockdev(lo->lo_device);
 	invalidate_bdev(lo->lo_device);
@@ -1455,6 +1472,11 @@ static int loop_set_block_size(struct loop_device *lo, unsigned long arg)
 	loop_update_dio(lo);
 	blk_mq_unfreeze_queue(lo->lo_queue, memflags);
 
+unlock:
+	mutex_unlock(&lo->lo_mutex);
+abort_claim:
+	if (!(mode & BLK_OPEN_EXCL))
+		bd_abort_claiming(bdev, loop_set_block_size);
 	return err;
 }
 
@@ -1473,9 +1495,6 @@ static int lo_simple_ioctl(struct loop_device *lo, unsigned int cmd,
 	case LOOP_SET_DIRECT_IO:
 		err = loop_set_dio(lo, arg);
 		break;
-	case LOOP_SET_BLOCK_SIZE:
-		err = loop_set_block_size(lo, arg);
-		break;
 	default:
 		err = -EINVAL;
 	}
@@ -1530,9 +1549,12 @@ static int lo_ioctl(struct block_device *bdev, blk_mode_t mode,
 		break;
 	case LOOP_GET_STATUS64:
 		return loop_get_status64(lo, argp);
+	case LOOP_SET_BLOCK_SIZE:
+		if (!(mode & BLK_OPEN_WRITE) && !capable(CAP_SYS_ADMIN))
+			return -EPERM;
+		return loop_set_block_size(lo, mode, bdev, arg);
 	case LOOP_SET_CAPACITY:
 	case LOOP_SET_DIRECT_IO:
-	case LOOP_SET_BLOCK_SIZE:
 		if (!(mode & BLK_OPEN_WRITE) && !capable(CAP_SYS_ADMIN))
 			return -EPERM;
 		fallthrough;
-- 
2.43.0

Powered by blists - more mailing lists