• Anton Eidelman's avatar
    nvme-multipath: fix crash in nvme_mpath_clear_ctrl_paths · 763303a8
    Anton Eidelman authored
    nvme_mpath_clear_ctrl_paths() iterates through
    the ctrl->namespaces list while holding ctrl->scan_lock.
    This does not seem to be the correct way of protecting
    from concurrent list modification.
    
    Specifically, nvme_scan_work() sorts ctrl->namespaces
    AFTER unlocking scan_lock.
    
    This may result in the following (rare) crash in ctrl disconnect
    during scan_work:
    
        BUG: kernel NULL pointer dereference, address: 0000000000000050
        Oops: 0000 [#1] SMP PTI
        CPU: 0 PID: 3995 Comm: nvme 5.3.5-050305-generic
        RIP: 0010:nvme_mpath_clear_current_path+0xe/0x90 [nvme_core]
        ...
        Call Trace:
         nvme_mpath_clear_ctrl_paths+0x3c/0x70 [nvme_core]
         nvme_remove_namespaces+0x35/0xe0 [nvme_core]
         nvme_do_delete_ctrl+0x47/0x90 [nvme_core]
         nvme_sysfs_delete+0x49/0x60 [nvme_core]
         dev_attr_store+0x17/0x30
         sysfs_kf_write+0x3e/0x50
         kernfs_fop_write+0x11e/0x1a0
         __vfs_write+0x1b/0x40
         vfs_write+0xb9/0x1a0
         ksys_write+0x67/0xe0
         __x64_sys_write+0x1a/0x20
         do_syscall_64+0x5a/0x130
         entry_SYSCALL_64_after_hwframe+0x44/0xa9
        RIP: 0033:0x7f8d02bfb154
    
    Fix:
    After taking scan_lock in nvme_mpath_clear_ctrl_paths()
    down_read(&ctrl->namespaces_rwsem) as well to make list traversal safe.
    This will not cause deadlocks because taking scan_lock never happens
    while holding the namespaces_rwsem.
    Moreover, scan work downs namespaces_rwsem in the same order.
    
    Alternative: sort ctrl->namespaces in nvme_scan_work()
    while still holding the scan_lock.
    This would leave nvme_mpath_clear_ctrl_paths() without correct protection
    against ctrl->namespaces modification by anyone other than scan_work.
    Reviewed-by: default avatarSagi Grimberg <sagi@grimberg.me>
    Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
    Signed-off-by: default avatarAnton Eidelman <anton@lightbitslabs.com>
    Signed-off-by: default avatarKeith Busch <kbusch@kernel.org>
    763303a8
multipath.c 18.6 KB