Skip to content

Commit dde1d38

Browse files
Yang Erkunsmb49
Yang Erkun
authored andcommitted
brd: defer automatic disk creation until module initialization succeeds
BugLink: https://bugs.launchpad.net/bugs/2101915 [ Upstream commit 826cc42adf44930a633d11a5993676d85ddb0842 ] 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: 7f9b348 ("brd: convert to blk_alloc_disk/blk_cleanup_disk") Reported-by: Wupeng Ma <[email protected]> Signed-off-by: Yang Erkun <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Jens Axboe <[email protected]> Signed-off-by: Sasha Levin <[email protected]> CVE-2024-56693 Signed-off-by: Koichiro Den <[email protected]> Signed-off-by: Stefan Bader <[email protected]>
1 parent 2ce079e commit dde1d38

File tree

1 file changed

+44
-22
lines changed

1 file changed

+44
-22
lines changed

drivers/block/brd.c

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -310,23 +310,50 @@ __setup("ramdisk_size=", ramdisk_size);
310310
* (should share code eventually).
311311
*/
312312
static LIST_HEAD(brd_devices);
313+
static DEFINE_MUTEX(brd_devices_mutex);
313314
static struct dentry *brd_debugfs_dir;
314315

316+
static struct brd_device *brd_find_or_alloc_device(int i)
317+
{
318+
struct brd_device *brd;
319+
320+
mutex_lock(&brd_devices_mutex);
321+
list_for_each_entry(brd, &brd_devices, brd_list) {
322+
if (brd->brd_number == i) {
323+
mutex_unlock(&brd_devices_mutex);
324+
return ERR_PTR(-EEXIST);
325+
}
326+
}
327+
328+
brd = kzalloc(sizeof(*brd), GFP_KERNEL);
329+
if (!brd) {
330+
mutex_unlock(&brd_devices_mutex);
331+
return ERR_PTR(-ENOMEM);
332+
}
333+
brd->brd_number = i;
334+
list_add_tail(&brd->brd_list, &brd_devices);
335+
mutex_unlock(&brd_devices_mutex);
336+
return brd;
337+
}
338+
339+
static void brd_free_device(struct brd_device *brd)
340+
{
341+
mutex_lock(&brd_devices_mutex);
342+
list_del(&brd->brd_list);
343+
mutex_unlock(&brd_devices_mutex);
344+
kfree(brd);
345+
}
346+
315347
static int brd_alloc(int i)
316348
{
317349
struct brd_device *brd;
318350
struct gendisk *disk;
319351
char buf[DISK_NAME_LEN];
320352
int err = -ENOMEM;
321353

322-
list_for_each_entry(brd, &brd_devices, brd_list)
323-
if (brd->brd_number == i)
324-
return -EEXIST;
325-
brd = kzalloc(sizeof(*brd), GFP_KERNEL);
326-
if (!brd)
327-
return -ENOMEM;
328-
brd->brd_number = i;
329-
list_add_tail(&brd->brd_list, &brd_devices);
354+
brd = brd_find_or_alloc_device(i);
355+
if (IS_ERR(brd))
356+
return PTR_ERR(brd);
330357

331358
xa_init(&brd->brd_pages);
332359

@@ -369,8 +396,7 @@ static int brd_alloc(int i)
369396
out_cleanup_disk:
370397
put_disk(disk);
371398
out_free_dev:
372-
list_del(&brd->brd_list);
373-
kfree(brd);
399+
brd_free_device(brd);
374400
return err;
375401
}
376402

@@ -389,8 +415,7 @@ static void brd_cleanup(void)
389415
del_gendisk(brd->brd_disk);
390416
put_disk(brd->brd_disk);
391417
brd_free_pages(brd);
392-
list_del(&brd->brd_list);
393-
kfree(brd);
418+
brd_free_device(brd);
394419
}
395420
}
396421

@@ -417,16 +442,6 @@ static int __init brd_init(void)
417442
{
418443
int err, i;
419444

420-
brd_check_and_reset_par();
421-
422-
brd_debugfs_dir = debugfs_create_dir("ramdisk_pages", NULL);
423-
424-
for (i = 0; i < rd_nr; i++) {
425-
err = brd_alloc(i);
426-
if (err)
427-
goto out_free;
428-
}
429-
430445
/*
431446
* brd module now has a feature to instantiate underlying device
432447
* structure on-demand, provided that there is an access dev node.
@@ -442,11 +457,18 @@ static int __init brd_init(void)
442457
* dynamically.
443458
*/
444459

460+
brd_check_and_reset_par();
461+
462+
brd_debugfs_dir = debugfs_create_dir("ramdisk_pages", NULL);
463+
445464
if (__register_blkdev(RAMDISK_MAJOR, "ramdisk", brd_probe)) {
446465
err = -EIO;
447466
goto out_free;
448467
}
449468

469+
for (i = 0; i < rd_nr; i++)
470+
brd_alloc(i);
471+
450472
pr_info("brd: module loaded\n");
451473
return 0;
452474

0 commit comments

Comments
 (0)