• Nikanth Karthikesan's avatar
    loop: fix circular locking in loop_clr_fd() · f028f3b2
    Nikanth Karthikesan authored
    With CONFIG_PROVE_LOCKING enabled
    
    $ losetup /dev/loop0 file
    $ losetup -o 32256 /dev/loop1 /dev/loop0
    
    $ losetup -d /dev/loop1
    $ losetup -d /dev/loop0
    
    triggers a [ INFO: possible circular locking dependency detected ]
    
    I think this warning is a false positive.
    
    Open/close on a loop device acquires bd_mutex of the device before
    acquiring lo_ctl_mutex of the same device. For ioctl(LOOP_CLR_FD) after
    acquiring lo_ctl_mutex, fput on the backing_file might acquire the bd_mutex of
    a device, if backing file is a device and this is the last reference to the
    file being dropped . But it is guaranteed that it is impossible to have a
    circular list of backing devices.(say loop2->loop1->loop0->loop2 is not
    possible), which guarantees that this can never deadlock.
    
    So this warning should be suppressed. It is very difficult to annotate lockdep
    not to warn here in the correct way. A simple way to silence lockdep could be
    to mark the lo_ctl_mutex in ioctl to be a sub class, but this might mask some
    other real bugs.
    
    @@ -1164,7 +1164,7 @@ static int lo_ioctl(struct block_device *bdev, fmode_t mode,
     	struct loop_device *lo = bdev->bd_disk->private_data;
     	int err;
    
    -	mutex_lock(&lo->lo_ctl_mutex);
    +	mutex_lock_nested(&lo->lo_ctl_mutex, 1);
     	switch (cmd) {
     	case LOOP_SET_FD:
     		err = loop_set_fd(lo, mode, bdev, arg);
    
    Or actually marking the bd_mutex after lo_ctl_mutex as a sub class could be
    a better solution.
    
    Luckily it is easy to avoid calling fput on backing file with lo_ctl_mutex
    held, so no lockdep annotation is required.
    
    If you do not like the special handling of the lo_ctl_mutex just for the
    LOOP_CLR_FD ioctl in lo_ioctl(), the mutex handling could be moved inside
    each of the individual ioctl handlers and I could send you another patch.
    Signed-off-by: default avatarJens Axboe <jens.axboe@oracle.com>
    f028f3b2
loop.c 40 KB