[PATCH -next] loop: Add parm check in loop_control_ioctl

From: Zhong Jinghua
Date: Mon Jun 05 2023 - 08:32:47 EST


From: Zhong Jinghua <zhongjinghua@xxxxxxxxxx>

We found that in loop_control_ioctl, the kernel panic can be easily caused:

1. syscall(__NR_ioctl, r[1], 0x4c80, 0x80000200000ul);
Create a loop device 0x80000200000ul.
In fact, in the code, it is used as the first_minor number, and the
first_minor number is 0.
So the created loop device number is 7:0.

2. syscall(__NR_ioctl, r[2], 0x4c80, 0ul);
Create a loop device 0x0ul.
Since the 7:0 device has been created in 1, add_disk will fail because
the major and first_minor numbers are consistent.

3. syscall(__NR_ioctl, r[5], 0x4c81, 0ul);
Delete the device that failed to create, the kernel panics.

Panic like below:
BUG: KASAN: null-ptr-deref in device_del+0xb3/0x840 drivers/base/core.c:3107
Call Trace:
kill_device drivers/base/core.c:3079 [inline]
device_del+0xb3/0x840 drivers/base/core.c:3107
del_gendisk+0x463/0x5f0 block/genhd.c:971
loop_remove drivers/block/loop.c:2190 [inline]
loop_control_ioctl drivers/block/loop.c:2289 [inline]

The stack like below:
Create loop device:
loop_control_ioctl
loop_add
add_disk
device_add_disk
bdi_register
bdi_register_va
device_create
device_create_groups_vargs
device_add
kfree(dev->p);
dev->p = NULL;

Remove loop device:
loop_control_ioctl
loop_remove
del_gendisk
device_del
kill_device
if (dev->p->dead) // p is null

Fix it by adding a check for parm.

Fixes: 770fe30a46a1 ("loop: add management interface for on-demand device allocation")
Signed-off-by: Zhong Jinghua <zhongjinghua@xxxxxxxxxx>
---
drivers/block/loop.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 76b96c42f417..60f2a31c4a24 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -2084,6 +2084,17 @@ static int loop_add(struct loop_device **l, int i)
struct gendisk *disk;
int err;

+ /*
+ * i << part_shift is actually used as the first_minor.
+ * So here should avoid i << part_shift overflow.
+ * And, MKDEV() expect that the max bits of
+ * first_minor is 20.
+ */
+ if (i > 0 && i > MINORMASK >> part_shift) {
+ err = -EINVAL;
+ goto out;
+ }
+
err = -ENOMEM;
lo = kzalloc(sizeof(*lo), GFP_KERNEL);
if (!lo)
@@ -2097,7 +2108,8 @@ static int loop_add(struct loop_device **l, int i)
if (err == -ENOSPC)
err = -EEXIST;
} else {
- err = idr_alloc(&loop_index_idr, lo, 0, 0, GFP_KERNEL);
+ err = idr_alloc(&loop_index_idr, lo, 0,
+ (MINORMASK >> part_shift) + 1, GFP_KERNEL);
}
if (err < 0)
goto out_free_dev;
--
2.31.1