• Baokun Li's avatar
    ubi: Fix race condition between ctrl_cdev_ioctl and ubi_cdev_ioctl · 3cbf0e39
    Baokun Li authored
    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 714fb87e 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: 714fb87e ("ubi: Fix race condition between ubi device creation and udev")
    Reported-by: default avatarHulk Robot <hulkci@huawei.com>
    Signed-off-by: default avatarBaokun Li <libaokun1@huawei.com>
    Signed-off-by: default avatarRichard Weinberger <richard@nod.at>
    3cbf0e39
vmt.c 21.2 KB