On 2024/10/26 15:28, Yu Kuai wrote: > Hi, > > 在 2024/10/26 13:55, Tetsuo Handa 写道: >> On 2024/10/26 10:21, Yu Kuai wrote: >>> Hi, >>> >>> 在 2024/10/25 18:40, Tetsuo Handa 写道: >>>> On 2024/10/25 16:05, Yang Erkun wrote: >>>>> From: Yang Erkun <yangerkun@xxxxxxxxxx> >>>>> >>>>> My colleague Wupeng found the following problems during fault injection: >>>>> >>>>> BUG: unable to handle page fault for address: fffffbfff809d073 >>>> >>>> Excuse me, but subject says "null pointer" whereas dmesg says >>>> "not a null pointer dereference". Is this a use-after-free bug? >>>> Also, what verb comes after "when modprobe brd" ? >>>> >>>> Is this problem happening with parallel execution? If yes, parallelly >>>> running what and what? >>> >>> The problem is straightforward, to be short, >>> >>> T1: morprobe brd >>> brd_init >>> brd_alloc >>> add_disk >>> T2: open brd >>> bdev_open >>> try_module_get >>> // err path >>> brd_cleanup >>> // dereference brd_fops() while module is freed. >> >> Then, fault injection is irrelevant, isn't it? > > Fault injection must involved in the test, brd_init() is unlikely to > fail. >> >> If bdev_open() can grab a reference before module's initialization phase >> completes is a problem, I think that we can fix the problem with just > > Yes, and root cause is that stuff inside module can be freed if module > initialization failed, it's not safe to deference disk->fops in this > case. Too bad. Then, we have to defer disk_alloc() until module initialization phase is guaranteed to return success like loop.c does. Please update patch title and description to something like below. Subject: brd: defer automatic disk creation until module initialization succeeds 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 when brd_init() failure raced with bdev_open(), for successfully created disks are already visible to bdev_open(). Fix this problem by following what loop_init() does.
