From: Greg Kroah-Hartman Date: Sun, 3 Apr 2022 13:40:19 +0000 (+0200) Subject: 4.14-stable patches X-Git-Tag: v5.17.2~114 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=fe215233a29d3995901c5114dceba18d6efe3fd5;p=thirdparty%2Fkernel%2Fstable-queue.git 4.14-stable patches added patches: ubi-fix-race-condition-between-ctrl_cdev_ioctl-and-ubi_cdev_ioctl.patch --- diff --git a/queue-4.14/series b/queue-4.14/series index 76282fc1ed5..d4e8762cb3c 100644 --- a/queue-4.14/series +++ b/queue-4.14/series @@ -195,3 +195,4 @@ can-mcba_usb-mcba_usb_start_xmit-fix-double-dev_kfree_skb-in-error-path.patch can-mcba_usb-properly-check-endpoint-type.patch gfs2-make-sure-fitrim-minlen-is-rounded-up-to-fs-block-size.patch pinctrl-pinconf-generic-print-arguments-for-bias-pull.patch +ubi-fix-race-condition-between-ctrl_cdev_ioctl-and-ubi_cdev_ioctl.patch diff --git a/queue-4.14/ubi-fix-race-condition-between-ctrl_cdev_ioctl-and-ubi_cdev_ioctl.patch b/queue-4.14/ubi-fix-race-condition-between-ctrl_cdev_ioctl-and-ubi_cdev_ioctl.patch new file mode 100644 index 00000000000..27ef2db2d72 --- /dev/null +++ b/queue-4.14/ubi-fix-race-condition-between-ctrl_cdev_ioctl-and-ubi_cdev_ioctl.patch @@ -0,0 +1,192 @@ +From 3cbf0e392f173ba0ce425968c8374a6aa3e90f2e Mon Sep 17 00:00:00 2001 +From: Baokun Li +Date: Fri, 5 Nov 2021 17:30:22 +0800 +Subject: ubi: Fix race condition between ctrl_cdev_ioctl and ubi_cdev_ioctl + +From: Baokun Li + +commit 3cbf0e392f173ba0ce425968c8374a6aa3e90f2e upstream. + +Hulk Robot reported a KASAN report about use-after-free: + ================================================================== + BUG: KASAN: use-after-free in __list_del_entry_valid+0x13d/0x160 + Read of size 8 at addr ffff888035e37d98 by task ubiattach/1385 + [...] + Call Trace: + klist_dec_and_del+0xa7/0x4a0 + klist_put+0xc7/0x1a0 + device_del+0x4d4/0xed0 + cdev_device_del+0x1a/0x80 + ubi_attach_mtd_dev+0x2951/0x34b0 [ubi] + ctrl_cdev_ioctl+0x286/0x2f0 [ubi] + + Allocated by task 1414: + device_add+0x60a/0x18b0 + cdev_device_add+0x103/0x170 + ubi_create_volume+0x1118/0x1a10 [ubi] + ubi_cdev_ioctl+0xb7f/0x1ba0 [ubi] + + Freed by task 1385: + cdev_device_del+0x1a/0x80 + ubi_remove_volume+0x438/0x6c0 [ubi] + ubi_cdev_ioctl+0xbf4/0x1ba0 [ubi] + [...] + ================================================================== + +The lock held by ctrl_cdev_ioctl is ubi_devices_mutex, but the lock held +by ubi_cdev_ioctl is ubi->device_mutex. Therefore, the two locks can be +concurrent. + +ctrl_cdev_ioctl contains two operations: ubi_attach and ubi_detach. +ubi_detach is bug-free because it uses reference counting to prevent +concurrency. However, uif_init and uif_close in ubi_attach may race with +ubi_cdev_ioctl. + +uif_init will race with ubi_cdev_ioctl as in the following stack. + cpu1 cpu2 cpu3 +_______________________|________________________|______________________ +ctrl_cdev_ioctl + ubi_attach_mtd_dev + uif_init + ubi_cdev_ioctl + ubi_create_volume + cdev_device_add + ubi_add_volume + // sysfs exist + kill_volumes + ubi_cdev_ioctl + ubi_remove_volume + cdev_device_del + // first free + ubi_free_volume + cdev_del + // double free + cdev_device_del + +And uif_close will race with ubi_cdev_ioctl as in the following stack. + 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 + +The cause of this problem is that commit 714fb87e8bc0 make device +"available" before it becomes accessible via sysfs. Therefore, we +roll back the modification. We will fix the race condition between +ubi device creation and udev by removing ubi_get_device in +vol_attribute_show and dev_attribute_show.This avoids accessing +uninitialized ubi_devices[ubi_num]. + +ubi_get_device is used to prevent devices from being deleted during +sysfs execution. However, now kernfs ensures that devices will not +be deleted before all reference counting are released. +The key process is shown in the following stack. + +device_del + device_remove_attrs + device_remove_groups + sysfs_remove_groups + sysfs_remove_group + remove_files + kernfs_remove_by_name + kernfs_remove_by_name_ns + __kernfs_remove + kernfs_drain + +Fixes: 714fb87e8bc0 ("ubi: Fix race condition between ubi device creation and udev") +Reported-by: Hulk Robot +Signed-off-by: Baokun Li +Signed-off-by: Richard Weinberger +Signed-off-by: Greg Kroah-Hartman +--- + drivers/mtd/ubi/build.c | 9 +-------- + drivers/mtd/ubi/vmt.c | 8 +------- + 2 files changed, 2 insertions(+), 15 deletions(-) + +--- a/drivers/mtd/ubi/build.c ++++ b/drivers/mtd/ubi/build.c +@@ -363,9 +363,6 @@ static ssize_t dev_attribute_show(struct + * we still can use 'ubi->ubi_num'. + */ + ubi = container_of(dev, struct ubi_device, dev); +- ubi = ubi_get_device(ubi->ubi_num); +- if (!ubi) +- return -ENODEV; + + if (attr == &dev_eraseblock_size) + ret = sprintf(buf, "%d\n", ubi->leb_size); +@@ -394,7 +391,6 @@ static ssize_t dev_attribute_show(struct + else + ret = -EINVAL; + +- ubi_put_device(ubi); + return ret; + } + +@@ -960,9 +956,6 @@ int ubi_attach_mtd_dev(struct mtd_info * + goto out_detach; + } + +- /* Make device "available" before it becomes accessible via sysfs */ +- ubi_devices[ubi_num] = ubi; +- + err = uif_init(ubi); + if (err) + goto out_detach; +@@ -1007,6 +1000,7 @@ int ubi_attach_mtd_dev(struct mtd_info * + wake_up_process(ubi->bgt_thread); + spin_unlock(&ubi->wl_lock); + ++ ubi_devices[ubi_num] = ubi; + ubi_notify_all(ubi, UBI_VOLUME_ADDED, NULL); + return ubi_num; + +@@ -1015,7 +1009,6 @@ out_debugfs: + out_uif: + uif_close(ubi); + out_detach: +- ubi_devices[ubi_num] = NULL; + ubi_wl_close(ubi); + ubi_free_internal_volumes(ubi); + vfree(ubi->vtbl); +--- a/drivers/mtd/ubi/vmt.c ++++ b/drivers/mtd/ubi/vmt.c +@@ -69,16 +69,11 @@ static ssize_t vol_attribute_show(struct + { + int ret; + struct ubi_volume *vol = container_of(dev, struct ubi_volume, dev); +- struct ubi_device *ubi; +- +- ubi = ubi_get_device(vol->ubi->ubi_num); +- if (!ubi) +- return -ENODEV; ++ struct ubi_device *ubi = vol->ubi; + + spin_lock(&ubi->volumes_lock); + if (!ubi->volumes[vol->vol_id]) { + spin_unlock(&ubi->volumes_lock); +- ubi_put_device(ubi); + return -ENODEV; + } + /* Take a reference to prevent volume removal */ +@@ -116,7 +111,6 @@ static ssize_t vol_attribute_show(struct + vol->ref_count -= 1; + ubi_assert(vol->ref_count >= 0); + spin_unlock(&ubi->volumes_lock); +- ubi_put_device(ubi); + return ret; + } +