]> git.ipfire.org Git - thirdparty/kernel/stable-queue.git/commitdiff
4.19-stable patches
authorGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sun, 3 Apr 2022 13:40:29 +0000 (15:40 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sun, 3 Apr 2022 13:40:29 +0000 (15:40 +0200)
added patches:
ubi-fix-race-condition-between-ctrl_cdev_ioctl-and-ubi_cdev_ioctl.patch

queue-4.19/series
queue-4.19/ubi-fix-race-condition-between-ctrl_cdev_ioctl-and-ubi_cdev_ioctl.patch [new file with mode: 0644]

index 800b957d17b3acdf437c0d7bc3cd50d56c6442b7..fd5e2969623ebce6cece191675894940f688f265 100644 (file)
@@ -242,3 +242,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.19/ubi-fix-race-condition-between-ctrl_cdev_ioctl-and-ubi_cdev_ioctl.patch b/queue-4.19/ubi-fix-race-condition-between-ctrl_cdev_ioctl-and-ubi_cdev_ioctl.patch
new file mode 100644 (file)
index 0000000..9cd7634
--- /dev/null
@@ -0,0 +1,192 @@
+From 3cbf0e392f173ba0ce425968c8374a6aa3e90f2e Mon Sep 17 00:00:00 2001
+From: Baokun Li <libaokun1@huawei.com>
+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 <libaokun1@huawei.com>
+
+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 <hulkci@huawei.com>
+Signed-off-by: Baokun Li <libaokun1@huawei.com>
+Signed-off-by: Richard Weinberger <richard@nod.at>
+Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
+---
+ 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;
+ }
+@@ -969,9 +965,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;
+@@ -1016,6 +1009,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;
+@@ -1024,7 +1018,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;
+ }