From: Jan Kara <jack@xxxxxxx> [ Upstream commit 7e49538288e523427beedd26993d446afef1a6fb ] 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@xxxxxxxxxxxxxxxxxxxxxxxxx Signed-off-by: Jan Kara <jack@xxxxxxx> Tested-by: syzbot+01ef7a8da81a975e1ccd@xxxxxxxxxxxxxxxxxxxxxxxxx Link: https://lore.kernel.org/r/20250711163202.19623-2-jack@xxxxxxx Signed-off-by: Jens Axboe <axboe@xxxxxxxxx> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> --- LLM Generated explanations, may be completely bogus: **Backport Status: YES** This commit is a strong candidate for stable backporting based on the following analysis: 1. **Fixes a Real User-Impacting Bug**: The commit addresses a syzbot- reported issue where changing a loop device's block size while it's mounted can cause filesystem corruption and warnings. The specific issue is a warning in `__getblk_slow()` in fs/buffer.c due to mismatched buffer and block device sizes. 2. **Security and Data Integrity Issue**: This bug can lead to filesystem corruption when the block size is changed underneath a mounted filesystem, which is a serious data integrity concern that affects users in production environments. 3. **Small and Contained Fix**: The changes are limited to the loop driver (drivers/block/loop.c) and specifically to the `loop_set_block_size()` function. The fix adds proper exclusive ownership checks before allowing block size changes. 4. **Clear Fix Strategy**: The solution is straightforward - it uses existing kernel mechanisms (`bd_prepare_to_claim()` and `bd_abort_claiming()`) to ensure exclusive access before modifying the block size. This prevents concurrent modifications when the device is already exclusively owned (e.g., by a mounted filesystem). 5. **Minimal Risk of Regression**: The change only affects the LOOP_SET_BLOCK_SIZE ioctl path and adds additional safety checks. It doesn't modify the core functionality but rather adds protection against an unsafe operation. 6. **Follows Stable Rules**: The commit: - Fixes a real bug (filesystem corruption/warnings) - Is small and self-contained - Has been tested (confirmed by syzbot) - Doesn't introduce new features - Has minimal performance impact The key code changes show the fix properly handles the exclusive ownership by: - Checking if the caller already has exclusive access (`!(mode & BLK_OPEN_EXCL)`) - If not, attempting to claim exclusive access via `bd_prepare_to_claim()` - Properly releasing the claim with `bd_abort_claiming()` if the operation completes without exclusive mode - Moving the operation out of `lo_simple_ioctl()` to handle the block device parameter This is exactly the type of bug fix that stable kernels should receive - it prevents data corruption with minimal code changes and low regression risk. 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 455e2a2b149f..ed004e1610dd 100644 --- a/drivers/block/loop.c +++ b/drivers/block/loop.c @@ -1472,19 +1472,36 @@ static int loop_set_dio(struct loop_device *lo, unsigned long arg) return error; } -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) { 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; + } err = blk_validate_block_size(arg); if (err) return err; if (lo->lo_queue->limits.logical_block_size == arg) - return 0; + goto unlock; sync_blockdev(lo->lo_device); invalidate_bdev(lo->lo_device); @@ -1496,6 +1513,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); +unlock: + mutex_unlock(&lo->lo_mutex); +abort_claim: + if (!(mode & BLK_OPEN_EXCL)) + bd_abort_claiming(bdev, loop_set_block_size); return err; } @@ -1514,9 +1536,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; } @@ -1571,9 +1590,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.39.5
