* [PATCH linux-5.4.y 1/8] Revert "loop: Check for overflow while configuring loop"
2024-03-07 4:14 [PATCH linux-5.4.y 0/8] Fix the UAF issue caused by the loop driver Genjian
@ 2024-03-07 4:14 ` Genjian
2024-03-07 4:14 ` [PATCH linux-5.4.y 2/8] loop: Call loop_config_discard() only after new config is applied Genjian
` (6 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Genjian @ 2024-03-07 4:14 UTC (permalink / raw)
To: stable
Cc: axboe, stable, linux-block, linux-kernel, zhanggenjian123,
Genjian Zhang, k2ci
From: Genjian Zhang <zhanggenjian@kylinos.cn>
This reverts commit 13b2856037a651ba3ab4a8b25ecab3e791926da3.
This patch lost a unlock loop_ctl_mutex in loop_get_status(...),
which caused syzbot to report a UAF issue.The upstream patch does not
have this issue. Therefore, we revert this patch and directly apply
the upstream patch later on.
Risk use-after-free as reported by syzbot：
[ 84.669496] ==================================================================
[ 84.670021] BUG: KASAN: use-after-free in __mutex_lock.isra.9+0xc13/0xcb0
[ 84.670433] Read of size 4 at addr ffff88808dba43b8 by task syz-executor.22/14230
[ 84.670885]
[ 84.670987] CPU: 1 PID: 14230 Comm: syz-executor.22 Not tainted 5.4.270 #4
[ 84.671397] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1kylin1 04/01/2014
[ 84.671927] Call Trace:
[ 84.672085] dump_stack+0x94/0xc7
[ 84.672293] ? __mutex_lock.isra.9+0xc13/0xcb0
[ 84.672569] print_address_description.constprop.6+0x16/0x220
[ 84.672915] ? __mutex_lock.isra.9+0xc13/0xcb0
[ 84.673187] ? __mutex_lock.isra.9+0xc13/0xcb0
[ 84.673462] __kasan_report.cold.9+0x1a/0x32
[ 84.673723] ? __mutex_lock.isra.9+0xc13/0xcb0
[ 84.673993] kasan_report+0x10/0x20
[ 84.674208] __mutex_lock.isra.9+0xc13/0xcb0
[ 84.674468] ? __mutex_lock.isra.9+0x617/0xcb0
[ 84.674739] ? ww_mutex_lock_interruptible+0x100/0x100
[ 84.675055] ? ww_mutex_lock_interruptible+0x100/0x100
[ 84.675369] ? kobject_get_unless_zero+0x144/0x190
[ 84.675668] ? kobject_del+0x60/0x60
[ 84.675893] ? __module_get+0x120/0x120
[ 84.676128] ? __mutex_lock_slowpath+0x10/0x10
[ 84.676399] mutex_lock_killable+0xde/0xf0
[ 84.676652] ? __mutex_lock_killable_slowpath+0x10/0x10
[ 84.676967] ? __mutex_lock_slowpath+0x10/0x10
[ 84.677243] ? disk_block_events+0x1d/0x120
[ 84.677509] lo_open+0x16/0xc0
[ 84.677701] ? lo_compat_ioctl+0x160/0x160
[ 84.677954] __blkdev_get+0xb0f/0x1160
[ 84.678185] ? bd_may_claim+0xd0/0xd0
[ 84.678410] ? bdev_disk_changed+0x190/0x190
[ 84.678674] ? _raw_spin_lock+0x7c/0xd0
[ 84.678915] ? _raw_write_lock_bh+0xd0/0xd0
[ 84.679172] blkdev_get+0x9b/0x290
[ 84.679381] ? ihold+0x1a/0x40
[ 84.679574] blkdev_open+0x1bd/0x240
[ 84.679794] do_dentry_open+0x439/0x1000
[ 84.680035] ? blkdev_get_by_dev+0x60/0x60
[ 84.680286] ? __x64_sys_fchdir+0x1a0/0x1a0
[ 84.680557] ? inode_permission+0x86/0x320
[ 84.680814] path_openat+0x998/0x4120
[ 84.681044] ? stack_trace_consume_entry+0x160/0x160
[ 84.681348] ? do_futex+0x136/0x1880
[ 84.681568] ? path_mountpoint+0xb50/0xb50
[ 84.681823] ? save_stack+0x4d/0x80
[ 84.682038] ? save_stack+0x19/0x80
[ 84.682253] ? __kasan_kmalloc.constprop.6+0xc1/0xd0
[ 84.682553] ? kmem_cache_alloc+0xc7/0x210
[ 84.682804] ? getname_flags+0xc4/0x560
[ 84.683045] ? do_sys_open+0x1ce/0x450
[ 84.683272] ? do_syscall_64+0x9a/0x330
[ 84.683509] ? entry_SYSCALL_64_after_hwframe+0x5c/0xc1
[ 84.683826] ? _raw_spin_lock+0x7c/0xd0
[ 84.684063] ? _raw_write_lock_bh+0xd0/0xd0
[ 84.684319] ? futex_exit_release+0x60/0x60
[ 84.684574] ? kasan_unpoison_shadow+0x30/0x40
[ 84.684844] ? __kasan_kmalloc.constprop.6+0xc1/0xd0
[ 84.685149] ? get_partial_node.isra.83.part.84+0x1e5/0x340
[ 84.685485] ? __fget_light+0x1d1/0x550
[ 84.685721] do_filp_open+0x197/0x270
[ 84.685946] ? may_open_dev+0xd0/0xd0
[ 84.686172] ? kasan_unpoison_shadow+0x30/0x40
[ 84.686443] ? __kasan_kmalloc.constprop.6+0xc1/0xd0
[ 84.686743] ? __alloc_fd+0x1a3/0x580
[ 84.686973] do_sys_open+0x2c7/0x450
[ 84.687195] ? filp_open+0x60/0x60
[ 84.687406] ? __x64_sys_timer_settime32+0x280/0x280
[ 84.687707] do_syscall_64+0x9a/0x330
[ 84.687931] ? syscall_return_slowpath+0x17a/0x230
[ 84.688221] entry_SYSCALL_64_after_hwframe+0x5c/0xc1
[ 84.688524]
[ 84.688622] Allocated by task 14056:
[ 84.688842] save_stack+0x19/0x80
[ 84.689044] __kasan_kmalloc.constprop.6+0xc1/0xd0
[ 84.689333] kmem_cache_alloc_node+0xe2/0x230
[ 84.689600] copy_process+0x165c/0x72d0
[ 84.689833] _do_fork+0xf9/0x9a0
[ 84.690032] __x64_sys_clone+0x17a/0x200
[ 84.690271] do_syscall_64+0x9a/0x330
[ 84.690496] entry_SYSCALL_64_after_hwframe+0x5c/0xc1
[ 84.690800]
[ 84.690903] Freed by task 0:
[ 84.691081] save_stack+0x19/0x80
[ 84.691287] __kasan_slab_free+0x125/0x170
[ 84.691535] kmem_cache_free+0x7a/0x2a0
[ 84.691774] __put_task_struct+0x1ec/0x4a0
[ 84.692023] delayed_put_task_struct+0x178/0x1d0
[ 84.692303] rcu_core+0x538/0x16c0
[ 84.692512] __do_softirq+0x175/0x63d
[ 84.692741]
[ 84.692840] The buggy address belongs to the object at ffff88808dba4380
[ 84.692840] which belongs to the cache task_struct of size 3328
[ 84.693584] The buggy address is located 56 bytes inside of
[ 84.693584] 3328-byte region [ffff88808dba4380, ffff88808dba5080)
[ 84.694272] The buggy address belongs to the page:
[ 84.694563] page:ffffea000236e800 refcount:1 mapcount:0 mapping:ffff8881838acdc0 index:0x0 compound_mapcount: 0
[ 84.695166] flags: 0x100000000010200(slab|head)
[ 84.695457] raw: 0100000000010200 dead000000000100 dead000000000122 ffff8881838acdc0
[ 84.695919] raw: 0000000000000000 0000000000090009 00000001ffffffff 0000000000000000
[ 84.696375] page dumped because: kasan: bad access detected
[ 84.696705]
[ 84.696801] Memory state around the buggy address:
[ 84.697089] ffff88808dba4280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 84.697519] ffff88808dba4300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 84.697945] >ffff88808dba4380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 84.698371] ^
[ 84.698674] ffff88808dba4400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 84.699111] ffff88808dba4480: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 84.699537] ==================================================================
[ 84.699965] Disabling lock debugging due to kernel taint
Reported-by: k2ci <kernel-bot@kylinos.cn>
Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn>
---
drivers/block/loop.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index d8821c9cb170..fced67ab1068 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1397,11 +1397,6 @@ loop_get_status(struct loop_device *lo, struct loop_info64 *info)
info->lo_number = lo->lo_number;
info->lo_offset = lo->lo_offset;
info->lo_sizelimit = lo->lo_sizelimit;
-
- /* loff_t vars have been assigned __u64 */
- if (lo->lo_offset < 0 || lo->lo_sizelimit < 0)
- return -EOVERFLOW;
-
info->lo_flags = lo->lo_flags;
memcpy(info->lo_file_name, lo->lo_file_name, LO_NAME_SIZE);
memcpy(info->lo_crypt_name, lo->lo_crypt_name, LO_NAME_SIZE);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH linux-5.4.y 2/8] loop: Call loop_config_discard() only after new config is applied
2024-03-07 4:14 [PATCH linux-5.4.y 0/8] Fix the UAF issue caused by the loop driver Genjian
2024-03-07 4:14 ` [PATCH linux-5.4.y 1/8] Revert "loop: Check for overflow while configuring loop" Genjian
@ 2024-03-07 4:14 ` Genjian
2024-03-07 4:14 ` [PATCH linux-5.4.y 3/8] loop: Remove sector_t truncation checks Genjian
` (5 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Genjian @ 2024-03-07 4:14 UTC (permalink / raw)
To: stable
Cc: axboe, stable, linux-block, linux-kernel, zhanggenjian123,
Martijn Coenen, Christoph Hellwig, Bob Liu, Bart Van Assche,
Genjian Zhang
From: Martijn Coenen <maco@android.com>
[ Upstream commit 7c5014b0987a30e4989c90633c198aced454c0ec ]
loop_set_status() calls loop_config_discard() to configure discard for
the loop device; however, the discard configuration depends on whether
the loop device uses encryption, and when we call it the encryption
configuration has not been updated yet. Move the call down so we apply
the correct discard configuration based on the new configuration.
Signed-off-by: Martijn Coenen <maco@android.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Bob Liu <bob.liu@oracle.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn>
---
drivers/block/loop.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index fced67ab1068..eb7b9629f6dd 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1332,8 +1332,6 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
}
}
- loop_config_discard(lo);
-
memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
lo->lo_file_name[LO_NAME_SIZE-1] = 0;
@@ -1357,6 +1355,8 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
lo->lo_key_owner = uid;
}
+ loop_config_discard(lo);
+
/* update dio if lo_offset or transfer is changed */
__loop_update_dio(lo, lo->use_dio);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH linux-5.4.y 3/8] loop: Remove sector_t truncation checks
2024-03-07 4:14 [PATCH linux-5.4.y 0/8] Fix the UAF issue caused by the loop driver Genjian
2024-03-07 4:14 ` [PATCH linux-5.4.y 1/8] Revert "loop: Check for overflow while configuring loop" Genjian
2024-03-07 4:14 ` [PATCH linux-5.4.y 2/8] loop: Call loop_config_discard() only after new config is applied Genjian
@ 2024-03-07 4:14 ` Genjian
2024-03-07 4:14 ` [PATCH linux-5.4.y 4/8] loop: Factor out setting loop device size Genjian
` (4 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Genjian @ 2024-03-07 4:14 UTC (permalink / raw)
To: stable
Cc: axboe, stable, linux-block, linux-kernel, zhanggenjian123,
Martijn Coenen, Christoph Hellwig, Genjian Zhang
From: Martijn Coenen <maco@android.com>
[ Upstream commit 083a6a50783ef54256eec3499e6575237e0e3d53 ]
sector_t is now always u64, so we don't need to check for truncation.
Signed-off-by: Martijn Coenen <maco@android.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn>
---
drivers/block/loop.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index eb7b9629f6dd..45237d44a867 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -226,24 +226,20 @@ static void __loop_update_dio(struct loop_device *lo, bool dio)
blk_mq_unfreeze_queue(lo->lo_queue);
}
-static int
+static void
figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit)
{
loff_t size = get_size(offset, sizelimit, lo->lo_backing_file);
- sector_t x = (sector_t)size;
struct block_device *bdev = lo->lo_device;
- if (unlikely((loff_t)x != size))
- return -EFBIG;
if (lo->lo_offset != offset)
lo->lo_offset = offset;
if (lo->lo_sizelimit != sizelimit)
lo->lo_sizelimit = sizelimit;
- set_capacity(lo->lo_disk, x);
+ set_capacity(lo->lo_disk, size);
bd_set_size(bdev, (loff_t)get_capacity(bdev->bd_disk) << 9);
/* let user-space know about the new size */
kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE);
- return 0;
}
static inline int
@@ -1002,10 +998,8 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
!file->f_op->write_iter)
lo_flags |= LO_FLAGS_READ_ONLY;
- error = -EFBIG;
size = get_loop_size(lo, file);
- if ((loff_t)(sector_t)size != size)
- goto out_unlock;
+
error = loop_prepare_queue(lo);
if (error)
goto out_unlock;
@@ -1326,10 +1320,7 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
lo->lo_device->bd_inode->i_mapping->nrpages);
goto out_unfreeze;
}
- if (figure_loop_size(lo, info->lo_offset, info->lo_sizelimit)) {
- err = -EFBIG;
- goto out_unfreeze;
- }
+ figure_loop_size(lo, info->lo_offset, info->lo_sizelimit);
}
memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
@@ -1532,7 +1523,9 @@ static int loop_set_capacity(struct loop_device *lo)
if (unlikely(lo->lo_state != Lo_bound))
return -ENXIO;
- return figure_loop_size(lo, lo->lo_offset, lo->lo_sizelimit);
+ figure_loop_size(lo, lo->lo_offset, lo->lo_sizelimit);
+
+ return 0;
}
static int loop_set_dio(struct loop_device *lo, unsigned long arg)
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH linux-5.4.y 4/8] loop: Factor out setting loop device size
2024-03-07 4:14 [PATCH linux-5.4.y 0/8] Fix the UAF issue caused by the loop driver Genjian
` (2 preceding siblings ...)
2024-03-07 4:14 ` [PATCH linux-5.4.y 3/8] loop: Remove sector_t truncation checks Genjian
@ 2024-03-07 4:14 ` Genjian
2024-03-07 4:14 ` [PATCH linux-5.4.y 5/8] loop: Refactor loop_set_status() size calculation Genjian
` (3 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Genjian @ 2024-03-07 4:14 UTC (permalink / raw)
To: stable
Cc: axboe, stable, linux-block, linux-kernel, zhanggenjian123,
Martijn Coenen, Christoph Hellwig, Genjian Zhang
From: Martijn Coenen <maco@android.com>
[ Upstream commit 5795b6f5607f7e4db62ddea144727780cb351a9b ]
This code is used repeatedly.
Signed-off-by: Martijn Coenen <maco@android.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn>
---
drivers/block/loop.c | 30 +++++++++++++++++++++---------
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 45237d44a867..78bfba4bce8a 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -226,20 +226,35 @@ static void __loop_update_dio(struct loop_device *lo, bool dio)
blk_mq_unfreeze_queue(lo->lo_queue);
}
+/**
+ * loop_set_size() - sets device size and notifies userspace
+ * @lo: struct loop_device to set the size for
+ * @size: new size of the loop device
+ *
+ * Callers must validate that the size passed into this function fits into
+ * a sector_t, eg using loop_validate_size()
+ */
+static void loop_set_size(struct loop_device *lo, loff_t size)
+{
+ struct block_device *bdev = lo->lo_device;
+
+ set_capacity(lo->lo_disk, size);
+ bd_set_size(bdev, size << SECTOR_SHIFT);
+ /* let user-space know about the new size */
+ kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE);
+}
+
static void
figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit)
{
loff_t size = get_size(offset, sizelimit, lo->lo_backing_file);
- struct block_device *bdev = lo->lo_device;
if (lo->lo_offset != offset)
lo->lo_offset = offset;
if (lo->lo_sizelimit != sizelimit)
lo->lo_sizelimit = sizelimit;
- set_capacity(lo->lo_disk, size);
- bd_set_size(bdev, (loff_t)get_capacity(bdev->bd_disk) << 9);
- /* let user-space know about the new size */
- kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE);
+
+ loop_set_size(lo, size);
}
static inline int
@@ -1033,11 +1048,8 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
loop_update_rotational(lo);
loop_update_dio(lo);
- set_capacity(lo->lo_disk, size);
- bd_set_size(bdev, size << 9);
loop_sysfs_init(lo);
- /* let user-space know about the new size */
- kobject_uevent(&disk_to_dev(bdev->bd_disk)->kobj, KOBJ_CHANGE);
+ loop_set_size(lo, size);
set_blocksize(bdev, S_ISBLK(inode->i_mode) ?
block_size(inode->i_bdev) : PAGE_SIZE);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH linux-5.4.y 5/8] loop: Refactor loop_set_status() size calculation
2024-03-07 4:14 [PATCH linux-5.4.y 0/8] Fix the UAF issue caused by the loop driver Genjian
` (3 preceding siblings ...)
2024-03-07 4:14 ` [PATCH linux-5.4.y 4/8] loop: Factor out setting loop device size Genjian
@ 2024-03-07 4:14 ` Genjian
2024-03-07 4:14 ` [PATCH linux-5.4.y 6/8] loop: Factor out configuring loop from status Genjian
` (2 subsequent siblings)
7 siblings, 0 replies; 9+ messages in thread
From: Genjian @ 2024-03-07 4:14 UTC (permalink / raw)
To: stable
Cc: axboe, stable, linux-block, linux-kernel, zhanggenjian123,
Martijn Coenen, Christoph Hellwig, Genjian Zhang
From: Martijn Coenen <maco@android.com>
[ Upstream commit b0bd158dd630bd47640e0e418c062cda1e0da5ad ]
figure_loop_size() calculates the loop size based on the passed in
parameters, but at the same time it updates the offset and sizelimit
parameters in the loop device configuration. That is a somewhat
unexpected side effect of a function with this name, and it is only only
needed by one of the two callers of this function - loop_set_status().
Move the lo_offset and lo_sizelimit assignment back into loop_set_status(),
and use the newly factored out functions to validate and apply the newly
calculated size. This allows us to get rid of figure_loop_size() in a
follow-up commit.
Signed-off-by: Martijn Coenen <maco@android.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn>
---
drivers/block/loop.c | 37 +++++++++++++++++++------------------
1 file changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 78bfba4bce8a..54f0f592423d 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -249,11 +249,6 @@ figure_loop_size(struct loop_device *lo, loff_t offset, loff_t sizelimit)
{
loff_t size = get_size(offset, sizelimit, lo->lo_backing_file);
- if (lo->lo_offset != offset)
- lo->lo_offset = offset;
- if (lo->lo_sizelimit != sizelimit)
- lo->lo_sizelimit = sizelimit;
-
loop_set_size(lo, size);
}
@@ -1271,6 +1266,7 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
kuid_t uid = current_uid();
struct block_device *bdev;
bool partscan = false;
+ bool size_changed = false;
err = mutex_lock_killable(&loop_ctl_mutex);
if (err)
@@ -1292,6 +1288,7 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
if (lo->lo_offset != info->lo_offset ||
lo->lo_sizelimit != info->lo_sizelimit) {
+ size_changed = true;
sync_blockdev(lo->lo_device);
invalidate_bdev(lo->lo_device);
}
@@ -1299,6 +1296,15 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
/* I/O need to be drained during transfer transition */
blk_mq_freeze_queue(lo->lo_queue);
+ if (size_changed && lo->lo_device->bd_inode->i_mapping->nrpages) {
+ /* If any pages were dirtied after invalidate_bdev(), try again */
+ err = -EAGAIN;
+ pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
+ __func__, lo->lo_number, lo->lo_file_name,
+ lo->lo_device->bd_inode->i_mapping->nrpages);
+ goto out_unfreeze;
+ }
+
err = loop_release_xfer(lo);
if (err)
goto out_unfreeze;
@@ -1322,19 +1328,8 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
if (err)
goto out_unfreeze;
- if (lo->lo_offset != info->lo_offset ||
- lo->lo_sizelimit != info->lo_sizelimit) {
- /* kill_bdev should have truncated all the pages */
- if (lo->lo_device->bd_inode->i_mapping->nrpages) {
- err = -EAGAIN;
- pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
- __func__, lo->lo_number, lo->lo_file_name,
- lo->lo_device->bd_inode->i_mapping->nrpages);
- goto out_unfreeze;
- }
- figure_loop_size(lo, info->lo_offset, info->lo_sizelimit);
- }
-
+ lo->lo_offset = info->lo_offset;
+ lo->lo_sizelimit = info->lo_sizelimit;
memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
lo->lo_file_name[LO_NAME_SIZE-1] = 0;
@@ -1358,6 +1353,12 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
lo->lo_key_owner = uid;
}
+ if (size_changed) {
+ loff_t new_size = get_size(lo->lo_offset, lo->lo_sizelimit,
+ lo->lo_backing_file);
+ loop_set_size(lo, new_size);
+ }
+
loop_config_discard(lo);
/* update dio if lo_offset or transfer is changed */
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH linux-5.4.y 6/8] loop: Factor out configuring loop from status
2024-03-07 4:14 [PATCH linux-5.4.y 0/8] Fix the UAF issue caused by the loop driver Genjian
` (4 preceding siblings ...)
2024-03-07 4:14 ` [PATCH linux-5.4.y 5/8] loop: Refactor loop_set_status() size calculation Genjian
@ 2024-03-07 4:14 ` Genjian
2024-03-07 4:14 ` [PATCH linux-5.4.y 7/8] loop: Check for overflow while configuring loop Genjian
2024-03-07 4:14 ` [PATCH linux-5.4.y 8/8] loop: loop_set_status_from_info() check before assignment Genjian
7 siblings, 0 replies; 9+ messages in thread
From: Genjian @ 2024-03-07 4:14 UTC (permalink / raw)
To: stable
Cc: axboe, stable, linux-block, linux-kernel, zhanggenjian123,
Martijn Coenen, Christoph Hellwig, Genjian Zhang
From: Martijn Coenen <maco@android.com>
[ Upstream commit 0c3796c244598122a5d59d56f30d19390096817f ]
Factor out this code into a separate function, so it can be reused by
other code more easily.
Signed-off-by: Martijn Coenen <maco@android.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn>
---
drivers/block/loop.c | 117 +++++++++++++++++++++++++------------------
1 file changed, 67 insertions(+), 50 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 54f0f592423d..eadb189be0cc 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1258,75 +1258,43 @@ static int loop_clr_fd(struct loop_device *lo)
return __loop_clr_fd(lo, false);
}
+/**
+ * loop_set_status_from_info - configure device from loop_info
+ * @lo: struct loop_device to configure
+ * @info: struct loop_info64 to configure the device with
+ *
+ * Configures the loop device parameters according to the passed
+ * in loop_info64 configuration.
+ */
static int
-loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
+loop_set_status_from_info(struct loop_device *lo,
+ const struct loop_info64 *info)
{
int err;
struct loop_func_table *xfer;
kuid_t uid = current_uid();
- struct block_device *bdev;
- bool partscan = false;
- bool size_changed = false;
-
- err = mutex_lock_killable(&loop_ctl_mutex);
- if (err)
- return err;
- if (lo->lo_encrypt_key_size &&
- !uid_eq(lo->lo_key_owner, uid) &&
- !capable(CAP_SYS_ADMIN)) {
- err = -EPERM;
- goto out_unlock;
- }
- if (lo->lo_state != Lo_bound) {
- err = -ENXIO;
- goto out_unlock;
- }
- if ((unsigned int) info->lo_encrypt_key_size > LO_KEY_SIZE) {
- err = -EINVAL;
- goto out_unlock;
- }
-
- if (lo->lo_offset != info->lo_offset ||
- lo->lo_sizelimit != info->lo_sizelimit) {
- size_changed = true;
- sync_blockdev(lo->lo_device);
- invalidate_bdev(lo->lo_device);
- }
- /* I/O need to be drained during transfer transition */
- blk_mq_freeze_queue(lo->lo_queue);
-
- if (size_changed && lo->lo_device->bd_inode->i_mapping->nrpages) {
- /* If any pages were dirtied after invalidate_bdev(), try again */
- err = -EAGAIN;
- pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
- __func__, lo->lo_number, lo->lo_file_name,
- lo->lo_device->bd_inode->i_mapping->nrpages);
- goto out_unfreeze;
- }
+ if ((unsigned int) info->lo_encrypt_key_size > LO_KEY_SIZE)
+ return -EINVAL;
err = loop_release_xfer(lo);
if (err)
- goto out_unfreeze;
+ return err;
if (info->lo_encrypt_type) {
unsigned int type = info->lo_encrypt_type;
- if (type >= MAX_LO_CRYPT) {
- err = -EINVAL;
- goto out_unfreeze;
- }
+ if (type >= MAX_LO_CRYPT)
+ return -EINVAL;
xfer = xfer_funcs[type];
- if (xfer == NULL) {
- err = -EINVAL;
- goto out_unfreeze;
- }
+ if (xfer == NULL)
+ return -EINVAL;
} else
xfer = NULL;
err = loop_init_xfer(lo, xfer, info);
if (err)
- goto out_unfreeze;
+ return err;
lo->lo_offset = info->lo_offset;
lo->lo_sizelimit = info->lo_sizelimit;
@@ -1353,6 +1321,55 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
lo->lo_key_owner = uid;
}
+ return 0;
+}
+
+static int
+loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
+{
+ int err;
+ struct block_device *bdev;
+ kuid_t uid = current_uid();
+ bool partscan = false;
+ bool size_changed = false;
+
+ err = mutex_lock_killable(&loop_ctl_mutex);
+ if (err)
+ return err;
+ if (lo->lo_encrypt_key_size &&
+ !uid_eq(lo->lo_key_owner, uid) &&
+ !capable(CAP_SYS_ADMIN)) {
+ err = -EPERM;
+ goto out_unlock;
+ }
+ if (lo->lo_state != Lo_bound) {
+ err = -ENXIO;
+ goto out_unlock;
+ }
+
+ if (lo->lo_offset != info->lo_offset ||
+ lo->lo_sizelimit != info->lo_sizelimit) {
+ size_changed = true;
+ sync_blockdev(lo->lo_device);
+ invalidate_bdev(lo->lo_device);
+ }
+
+ /* I/O need to be drained during transfer transition */
+ blk_mq_freeze_queue(lo->lo_queue);
+
+ if (size_changed && lo->lo_device->bd_inode->i_mapping->nrpages) {
+ /* If any pages were dirtied after invalidate_bdev(), try again */
+ err = -EAGAIN;
+ pr_warn("%s: loop%d (%s) has still dirty pages (nrpages=%lu)\n",
+ __func__, lo->lo_number, lo->lo_file_name,
+ lo->lo_device->bd_inode->i_mapping->nrpages);
+ goto out_unfreeze;
+ }
+
+ err = loop_set_status_from_info(lo, info);
+ if (err)
+ goto out_unfreeze;
+
if (size_changed) {
loff_t new_size = get_size(lo->lo_offset, lo->lo_sizelimit,
lo->lo_backing_file);
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH linux-5.4.y 7/8] loop: Check for overflow while configuring loop
2024-03-07 4:14 [PATCH linux-5.4.y 0/8] Fix the UAF issue caused by the loop driver Genjian
` (5 preceding siblings ...)
2024-03-07 4:14 ` [PATCH linux-5.4.y 6/8] loop: Factor out configuring loop from status Genjian
@ 2024-03-07 4:14 ` Genjian
2024-03-07 4:14 ` [PATCH linux-5.4.y 8/8] loop: loop_set_status_from_info() check before assignment Genjian
7 siblings, 0 replies; 9+ messages in thread
From: Genjian @ 2024-03-07 4:14 UTC (permalink / raw)
To: stable
Cc: axboe, stable, linux-block, linux-kernel, zhanggenjian123,
Siddh Raman Pant, syzbot+a8e049cd3abd342936b6, Matthew Wilcox,
Christoph Hellwig, Genjian Zhang
From: Siddh Raman Pant <code@siddh.me>
[ Upstream commit c490a0b5a4f36da3918181a8acdc6991d967c5f3 ]
The userspace can configure a loop using an ioctl call, wherein
a configuration of type loop_config is passed (see lo_ioctl()'s
case on line 1550 of drivers/block/loop.c). This proceeds to call
loop_configure() which in turn calls loop_set_status_from_info()
(see line 1050 of loop.c), passing &config->info which is of type
loop_info64*. This function then sets the appropriate values, like
the offset.
loop_device has lo_offset of type loff_t (see line 52 of loop.c),
which is typdef-chained to long long, whereas loop_info64 has
lo_offset of type __u64 (see line 56 of include/uapi/linux/loop.h).
The function directly copies offset from info to the device as
follows (See line 980 of loop.c):
lo->lo_offset = info->lo_offset;
This results in an overflow, which triggers a warning in iomap_iter()
due to a call to iomap_iter_done() which has:
WARN_ON_ONCE(iter->iomap.offset > iter->pos);
Thus, check for negative value during loop_set_status_from_info().
Bug report: https://syzkaller.appspot.com/bug?id=c620fe14aac810396d3c3edc9ad73848bf69a29e
Reported-and-tested-by: syzbot+a8e049cd3abd342936b6@syzkaller.appspotmail.com
Cc: stable@vger.kernel.org
Reviewed-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: Siddh Raman Pant <code@siddh.me>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20220823160810.181275-1-code@siddh.me
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn>
---
drivers/block/loop.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index eadb189be0cc..c999eef4e345 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1298,6 +1298,11 @@ loop_set_status_from_info(struct loop_device *lo,
lo->lo_offset = info->lo_offset;
lo->lo_sizelimit = info->lo_sizelimit;
+
+ /* loff_t vars have been assigned __u64 */
+ if (lo->lo_offset < 0 || lo->lo_sizelimit < 0)
+ return -EOVERFLOW;
+
memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
lo->lo_file_name[LO_NAME_SIZE-1] = 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH linux-5.4.y 8/8] loop: loop_set_status_from_info() check before assignment
2024-03-07 4:14 [PATCH linux-5.4.y 0/8] Fix the UAF issue caused by the loop driver Genjian
` (6 preceding siblings ...)
2024-03-07 4:14 ` [PATCH linux-5.4.y 7/8] loop: Check for overflow while configuring loop Genjian
@ 2024-03-07 4:14 ` Genjian
7 siblings, 0 replies; 9+ messages in thread
From: Genjian @ 2024-03-07 4:14 UTC (permalink / raw)
To: stable
Cc: axboe, stable, linux-block, linux-kernel, zhanggenjian123,
Zhong Jinghua, Chaitanya Kulkarni, Genjian Zhang
From: Zhong Jinghua <zhongjinghua@huawei.com>
[ Upstream commit 9f6ad5d533d1c71e51bdd06a5712c4fbc8768dfa ]
In loop_set_status_from_info(), lo->lo_offset and lo->lo_sizelimit should
be checked before reassignment, because if an overflow error occurs, the
original correct value will be changed to the wrong value, and it will not
be changed back.
More, the original patch did not solve the problem, the value was set and
ioctl returned an error, but the subsequent io used the value in the loop
driver, which still caused an alarm:
loop_handle_cmd
do_req_filebacked
loff_t pos = ((loff_t) blk_rq_pos(rq) << 9) + lo->lo_offset;
lo_rw_aio
cmd->iocb.ki_pos = pos
Fixes: c490a0b5a4f3 ("loop: Check for overflow while configuring loop")
Signed-off-by: Zhong Jinghua <zhongjinghua@huawei.com>
Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com>
Link: https://lore.kernel.org/r/20230221095027.3656193-1-zhongjinghua@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn>
---
drivers/block/loop.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index c999eef4e345..ff452c02b61f 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1296,13 +1296,13 @@ loop_set_status_from_info(struct loop_device *lo,
if (err)
return err;
+ /* Avoid assigning overflow values */
+ if (info->lo_offset > LLONG_MAX || info->lo_sizelimit > LLONG_MAX)
+ return -EOVERFLOW;
+
lo->lo_offset = info->lo_offset;
lo->lo_sizelimit = info->lo_sizelimit;
- /* loff_t vars have been assigned __u64 */
- if (lo->lo_offset < 0 || lo->lo_sizelimit < 0)
- return -EOVERFLOW;
-
memcpy(lo->lo_file_name, info->lo_file_name, LO_NAME_SIZE);
memcpy(lo->lo_crypt_name, info->lo_crypt_name, LO_NAME_SIZE);
lo->lo_file_name[LO_NAME_SIZE-1] = 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 9+ messages in thread