Re: [PATCH] brd: fix null pointer when modprobe brd

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





在 2024/10/26 16:06, Tetsuo Handa 写道:
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.

Tetsuo and Kuai,

This commit msg looks good to me. I couldn't reply to some emails the
other day. Thanks a lot for your discussion and review to make this
problem clear!

Thanks,
Erkun.





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux