| Message ID | 20231103101334.1750094-1-lilingfeng@huaweicloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nbd: fix uaf in nbd_open | expand |
Hi, 在 2023/11/03 18:13, Li Lingfeng 写道: > From: Li Lingfeng <lilingfeng3@huawei.com> > > Commit 4af5f2e03013 ("nbd: use blk_mq_alloc_disk and > blk_cleanup_disk") cleans up disk by blk_cleanup_disk() and it won't set > disk->private_data as NULL as before. UAF may be triggered in nbd_open() > if someone tries to open nbd device right after nbd_put() since refcount > of nbd device is zero and private_data is not NULL. > Do you mean that nbd_open concurrent with nbd_dev_remove? nbd_open nbd_dev_remove del_gendisk kfree(nbd) mutex_lock nbd = disk->private_data -> UAF refcount_inc_not_zero Looks like it's possible, but you should use READ/WRITE_ONCE() here, because disk->pravate_data can be accessed concurrently. Thanks, Kuai > Fixes: 4af5f2e03013 ("nbd: use blk_mq_alloc_disk and blk_cleanup_disk") > Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com> > --- > drivers/block/nbd.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c > index 800f131222fc..aab93b836e84 100644 > --- a/drivers/block/nbd.c > +++ b/drivers/block/nbd.c > @@ -250,6 +250,7 @@ static void nbd_dev_remove(struct nbd_device *nbd) > struct gendisk *disk = nbd->disk; > > del_gendisk(disk); > + disk->private_data = NULL; > put_disk(disk); > blk_mq_free_tag_set(&nbd->tag_set); > >
On Fri, Nov 03, 2023 at 06:13:34PM +0800, Li Lingfeng wrote: > From: Li Lingfeng <lilingfeng3@huawei.com> > > Commit 4af5f2e03013 ("nbd: use blk_mq_alloc_disk and > blk_cleanup_disk") cleans up disk by blk_cleanup_disk() and it won't set > disk->private_data as NULL as before. UAF may be triggered in nbd_open() > if someone tries to open nbd device right after nbd_put() since refcount > of nbd device is zero and private_data is not NULL. I don't think this is the right fix. nbd needs to move to ->free_disk to free it's private data.
在 2023/11/3 16:12, Christoph Hellwig 写道: > On Fri, Nov 03, 2023 at 06:13:34PM +0800, Li Lingfeng wrote: >> From: Li Lingfeng <lilingfeng3@huawei.com> >> >> Commit 4af5f2e03013 ("nbd: use blk_mq_alloc_disk and >> blk_cleanup_disk") cleans up disk by blk_cleanup_disk() and it won't set >> disk->private_data as NULL as before. UAF may be triggered in nbd_open() >> if someone tries to open nbd device right after nbd_put() since refcount >> of nbd device is zero and private_data is not NULL. > I don't think this is the right fix. nbd needs to move to ->free_disk > to free it's private data. Thanks for your advice, I will send v2 soon.
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c index 800f131222fc..aab93b836e84 100644 --- a/drivers/block/nbd.c +++ b/drivers/block/nbd.c @@ -250,6 +250,7 @@ static void nbd_dev_remove(struct nbd_device *nbd) struct gendisk *disk = nbd->disk; del_gendisk(disk); + disk->private_data = NULL; put_disk(disk); blk_mq_free_tag_set(&nbd->tag_set);