Skip to content

Commit

Permalink
brd: defer automatic disk creation until module initialization succeeds
Browse files Browse the repository at this point in the history
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 <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>
  • Loading branch information
Yang Erkun authored and axboe committed Oct 30, 2024
1 parent 8d3fd05 commit 826cc42
Showing 1 changed file with 44 additions and 22 deletions.
66 changes: 44 additions & 22 deletions drivers/block/brd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);

Expand Down Expand Up @@ -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;
}

Expand All @@ -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);
}
}

Expand All @@ -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.
Expand All @@ -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;

Expand Down

0 comments on commit 826cc42

Please sign in to comment.