mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2024-12-28 16:53:49 +00:00
brd: defer automatic disk creation until module initialization succeeds
My colleague Wupeng found the following problems during fault injection:
BUG: unable to handle page fault for address: fffffbfff809d073
PGD 6e648067 P4D 123ec8067 PUD 123ec4067 PMD 100e38067 PTE 0
Oops: Oops: 0000 [#1] PREEMPT SMP KASAN NOPTI
CPU: 5 UID: 0 PID: 755 Comm: modprobe Not tainted 6.12.0-rc3+ #17
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
1.16.1-2.fc37 04/01/2014
RIP: 0010:__asan_load8+0x4c/0xa0
...
Call Trace:
<TASK>
blkdev_put_whole+0x41/0x70
bdev_release+0x1a3/0x250
blkdev_release+0x11/0x20
__fput+0x1d7/0x4a0
task_work_run+0xfc/0x180
syscall_exit_to_user_mode+0x1de/0x1f0
do_syscall_64+0x6b/0x170
entry_SYSCALL_64_after_hwframe+0x76/0x7e
loop_init() is calling loop_add() after __register_blkdev() succeeds and
is ignoring disk_add() failure from loop_add(), for loop_add() failure
is not fatal and successfully created disks are already visible to
bdev_open().
brd_init() is currently calling brd_alloc() before __register_blkdev()
succeeds and is releasing successfully created disks when brd_init()
returns an error. This can cause UAF for the latter two case:
case 1:
T1:
modprobe brd
brd_init
brd_alloc(0) // success
add_disk
disk_scan_partitions
bdev_file_open_by_dev // alloc file
fput // won't free until back to userspace
brd_alloc(1) // failed since mem alloc error inject
// error path for modprobe will release code segment
// back to userspace
__fput
blkdev_release
bdev_release
blkdev_put_whole
bdev->bd_disk->fops->release // fops is freed now, UAF!
case 2:
T1: T2:
modprobe brd
brd_init
brd_alloc(0) // success
open(/dev/ram0)
brd_alloc(1) // fail
// error path for modprobe
close(/dev/ram0)
...
/* UAF! */
bdev->bd_disk->fops->release
Fix this problem by following what loop_init() does. Besides,
reintroduce brd_devices_mutex to help serialize modifications to
brd_list.
Fixes: 7f9b348cb5
("brd: convert to blk_alloc_disk/blk_cleanup_disk")
Reported-by: Wupeng Ma <mawupeng1@huawei.com>
Signed-off-by: Yang Erkun <yangerkun@huawei.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Link: https://lore.kernel.org/r/20241030034914.907829-1-yangerkun@huaweicloud.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
This commit is contained in:
parent
8d3fd059dd
commit
826cc42adf
@ -316,8 +316,40 @@ __setup("ramdisk_size=", ramdisk_size);
|
||||
* (should share code eventually).
|
||||
*/
|
||||
static LIST_HEAD(brd_devices);
|
||||
static DEFINE_MUTEX(brd_devices_mutex);
|
||||
static struct dentry *brd_debugfs_dir;
|
||||
|
||||
static struct brd_device *brd_find_or_alloc_device(int i)
|
||||
{
|
||||
struct brd_device *brd;
|
||||
|
||||
mutex_lock(&brd_devices_mutex);
|
||||
list_for_each_entry(brd, &brd_devices, brd_list) {
|
||||
if (brd->brd_number == i) {
|
||||
mutex_unlock(&brd_devices_mutex);
|
||||
return ERR_PTR(-EEXIST);
|
||||
}
|
||||
}
|
||||
|
||||
brd = kzalloc(sizeof(*brd), GFP_KERNEL);
|
||||
if (!brd) {
|
||||
mutex_unlock(&brd_devices_mutex);
|
||||
return ERR_PTR(-ENOMEM);
|
||||
}
|
||||
brd->brd_number = i;
|
||||
list_add_tail(&brd->brd_list, &brd_devices);
|
||||
mutex_unlock(&brd_devices_mutex);
|
||||
return brd;
|
||||
}
|
||||
|
||||
static void brd_free_device(struct brd_device *brd)
|
||||
{
|
||||
mutex_lock(&brd_devices_mutex);
|
||||
list_del(&brd->brd_list);
|
||||
mutex_unlock(&brd_devices_mutex);
|
||||
kfree(brd);
|
||||
}
|
||||
|
||||
static int brd_alloc(int i)
|
||||
{
|
||||
struct brd_device *brd;
|
||||
@ -340,14 +372,9 @@ static int brd_alloc(int i)
|
||||
BLK_FEAT_NOWAIT,
|
||||
};
|
||||
|
||||
list_for_each_entry(brd, &brd_devices, brd_list)
|
||||
if (brd->brd_number == i)
|
||||
return -EEXIST;
|
||||
brd = kzalloc(sizeof(*brd), GFP_KERNEL);
|
||||
if (!brd)
|
||||
return -ENOMEM;
|
||||
brd->brd_number = i;
|
||||
list_add_tail(&brd->brd_list, &brd_devices);
|
||||
brd = brd_find_or_alloc_device(i);
|
||||
if (IS_ERR(brd))
|
||||
return PTR_ERR(brd);
|
||||
|
||||
xa_init(&brd->brd_pages);
|
||||
|
||||
@ -378,8 +405,7 @@ static int brd_alloc(int i)
|
||||
out_cleanup_disk:
|
||||
put_disk(disk);
|
||||
out_free_dev:
|
||||
list_del(&brd->brd_list);
|
||||
kfree(brd);
|
||||
brd_free_device(brd);
|
||||
return err;
|
||||
}
|
||||
|
||||
@ -398,8 +424,7 @@ static void brd_cleanup(void)
|
||||
del_gendisk(brd->brd_disk);
|
||||
put_disk(brd->brd_disk);
|
||||
brd_free_pages(brd);
|
||||
list_del(&brd->brd_list);
|
||||
kfree(brd);
|
||||
brd_free_device(brd);
|
||||
}
|
||||
}
|
||||
|
||||
@ -426,16 +451,6 @@ static int __init brd_init(void)
|
||||
{
|
||||
int err, i;
|
||||
|
||||
brd_check_and_reset_par();
|
||||
|
||||
brd_debugfs_dir = debugfs_create_dir("ramdisk_pages", NULL);
|
||||
|
||||
for (i = 0; i < rd_nr; i++) {
|
||||
err = brd_alloc(i);
|
||||
if (err)
|
||||
goto out_free;
|
||||
}
|
||||
|
||||
/*
|
||||
* brd module now has a feature to instantiate underlying device
|
||||
* structure on-demand, provided that there is an access dev node.
|
||||
@ -451,11 +466,18 @@ static int __init brd_init(void)
|
||||
* dynamically.
|
||||
*/
|
||||
|
||||
brd_check_and_reset_par();
|
||||
|
||||
brd_debugfs_dir = debugfs_create_dir("ramdisk_pages", NULL);
|
||||
|
||||
if (__register_blkdev(RAMDISK_MAJOR, "ramdisk", brd_probe)) {
|
||||
err = -EIO;
|
||||
goto out_free;
|
||||
}
|
||||
|
||||
for (i = 0; i < rd_nr; i++)
|
||||
brd_alloc(i);
|
||||
|
||||
pr_info("brd: module loaded\n");
|
||||
return 0;
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user