• Vegard Nossum's avatar
    nbd: fix race in ioctl · 72ac9f0c
    Vegard Nossum authored
    BugLink: https://bugs.launchpad.net/bugs/1793464
    
    Quentin ran into this bug:
    
    WARNING: CPU: 64 PID: 10085 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x65/0x80
    sysfs: cannot create duplicate filename '/devices/virtual/block/nbd3/pid'
    Modules linked in: nbd
    CPU: 64 PID: 10085 Comm: qemu-nbd Tainted: G      D         4.6.0+ #7
     0000000000000000 ffff8820330bba68 ffffffff814b8791 ffff8820330bbac8
     0000000000000000 ffff8820330bbab8 ffffffff810d04ab ffff8820330bbaa8
     0000001f00000296 0000000000017681 ffff8810380bf000 ffffffffa0001790
    Call Trace:
     [<ffffffff814b8791>] dump_stack+0x4d/0x6c
     [<ffffffff810d04ab>] __warn+0xdb/0x100
     [<ffffffff810d0574>] warn_slowpath_fmt+0x44/0x50
     [<ffffffff81218c65>] sysfs_warn_dup+0x65/0x80
     [<ffffffff81218a02>] sysfs_add_file_mode_ns+0x172/0x180
     [<ffffffff81218a35>] sysfs_create_file_ns+0x25/0x30
     [<ffffffff81594a76>] device_create_file+0x36/0x90
     [<ffffffffa0000e8d>] __nbd_ioctl+0x32d/0x9b0 [nbd]
     [<ffffffff814cc8e8>] ? find_next_bit+0x18/0x20
     [<ffffffff810f7c29>] ? select_idle_sibling+0xe9/0x120
     [<ffffffff810f6cd7>] ? __enqueue_entity+0x67/0x70
     [<ffffffff810f9bf0>] ? enqueue_task_fair+0x630/0xe20
     [<ffffffff810efa76>] ? resched_curr+0x36/0x70
     [<ffffffff810f0078>] ? check_preempt_curr+0x78/0x90
     [<ffffffff810f00a2>] ? ttwu_do_wakeup+0x12/0x80
     [<ffffffff810f01b1>] ? ttwu_do_activate.constprop.86+0x61/0x70
     [<ffffffff810f0c15>] ? try_to_wake_up+0x185/0x2d0
     [<ffffffff810f0d6d>] ? default_wake_function+0xd/0x10
     [<ffffffff81105471>] ? autoremove_wake_function+0x11/0x40
     [<ffffffffa0001577>] nbd_ioctl+0x67/0x94 [nbd]
     [<ffffffff814ac0fd>] blkdev_ioctl+0x14d/0x940
     [<ffffffff811b0da2>] ? put_pipe_info+0x22/0x60
     [<ffffffff811d96cc>] block_ioctl+0x3c/0x40
     [<ffffffff811ba08d>] do_vfs_ioctl+0x8d/0x5e0
     [<ffffffff811aa329>] ? ____fput+0x9/0x10
     [<ffffffff810e9092>] ? task_work_run+0x72/0x90
     [<ffffffff811ba627>] SyS_ioctl+0x47/0x80
     [<ffffffff8185f5df>] entry_SYSCALL_64_fastpath+0x17/0x93
    ---[ end trace 7899b295e4f850c8 ]---
    
    It seems fairly obvious that device_create_file() is not being protected
    from being run concurrently on the same nbd.
    
    Quentin found the following relevant commits:
    
    1a2ad211 nbd: add locking to nbd_ioctl
    90b8f282 [PATCH] end of methods switch: remove the old ones
    d4430d62 [PATCH] beginning of methods conversion
    08f85851 [PATCH] move block_device_operations to blkdev.h
    
    It would seem that the race was introduced in the process of moving nbd
    from BKL to unlocked ioctls.
    
    By setting nbd->task_recv while the mutex is held, we can prevent other
    processes from running concurrently (since nbd->task_recv is also checked
    while the mutex is held).
    Reported-and-tested-by: default avatarQuentin Casasnovas <quentin.casasnovas@oracle.com>
    Cc: Markus Pargmann <mpa@pengutronix.de>
    Cc: Paul Clements <paul.clements@steeleye.com>
    Cc: Pavel Machek <pavel@suse.cz>
    Cc: Jens Axboe <axboe@fb.com>
    Cc: Al Viro <viro@zeniv.linux.org.uk>
    Signed-off-by: default avatarVegard Nossum <vegard.nossum@oracle.com>
    Signed-off-by: default avatarJens Axboe <axboe@fb.com>
    (backported from commit 97240963)
    Signed-off-by: default avatarColin Ian King <colin.king@canonical.com>
    Acked-by: default avatarKleber Souza <kleber.souza@canonical.com>
    Acked-by: default avatarStefan Bader <stefan.bader@canonical.com>
    Signed-off-by: default avatarKhalid Elmously <khalid.elmously@canonical.com>
    72ac9f0c
nbd.c 26.4 KB