[PATCH -next V2 2/2] ubi: fix race between volume operations and uif_close

From: Baokun Li
Date: Tue Nov 02 2021 - 21:20:32 EST


KASAN reported a UAF about ubi:
==================================================================
BUG: KASAN: use-after-free in kobject_get+0x44/0xd0
Write of size 4 at addr ffff8881216e5038 by task ubirmvol/18988
[...]
Call Trace:
kobject_get+0x44/0xd0
get_device+0x25/0x40
ubi_open_volume+0x22c/0x490 [ubi]
ubi_cdev_ioctl+0x300/0x11a0 [ubi]

Allocated by task 18850:
ubi_read_volume_table+0x676/0x1330 [ubi]
ubi_attach+0xd13/0x2460 [ubi]
ubi_attach_mtd_dev+0xafa/0x17b0 [ubi]
ctrl_cdev_ioctl+0x248/0x2b0 [ubi]

Freed by task 18850:
kfree+0xa2/0x490
device_release+0x65/0x130
kobject_put+0x17b/0x330
device_unregister+0x39/0x90
uif_close+0x61/0xc0 [ubi]
ubi_attach_mtd_dev+0xdd2/0x17b0 [ubi]
ctrl_cdev_ioctl+0x248/0x2b0 [ubi]
[...]
==================================================================

The following race could cause the use-after-free problem:
cpu1 cpu2 cpu3
_______________________|________________________|______________________
ctrl_cdev_ioctl
ubi_attach_mtd_dev
uif_init
ubi_cdev_ioctl
ubi_create_volume
cdev_device_add
ubi_debugfs_init_dev
//error goto out_uif;
uif_close
kill_volumes
ubi_cdev_ioctl
ubi_remove_volume
cdev_device_del
// first free
ubi_free_volume
// double free

To solve this problem, add spin_lock(&ubi->volumes_lock) in uif_close.

Fixes: 801c135ce73d ("UBI: Unsorted Block Images")
Reported-by: Zhihao Cheng <chengzhihao1@xxxxxxxxxx>
Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx>
---
drivers/mtd/ubi/build.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 708b1b96de01..5a11cdc6e076 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -501,7 +501,9 @@ static int uif_init(struct ubi_device *ubi)
*/
static void uif_close(struct ubi_device *ubi)
{
+ spin_lock(&ubi->volumes_lock);
kill_volumes(ubi);
+ spin_unlock(&ubi->volumes_lock);
cdev_device_del(&ubi->cdev, &ubi->dev);
unregister_chrdev_region(ubi->cdev.dev, ubi->vtbl_slots + 1);
}
--
2.31.1