• Tetsuo Handa's avatar
    media: imon: reorganize serialization · db264d4c
    Tetsuo Handa authored
    Since usb_register_dev() from imon_init_display() from imon_probe() holds
    minor_rwsem while display_open() which holds driver_lock and ictx->lock is
    called with minor_rwsem held from usb_open(), holding driver_lock or
    ictx->lock when calling usb_register_dev() causes circular locking
    dependency problem.
    
    Since usb_deregister_dev() from imon_disconnect() holds minor_rwsem while
    display_open() which holds driver_lock is called with minor_rwsem held,
    holding driver_lock when calling usb_deregister_dev() also causes circular
    locking dependency problem.
    
    Sean Young explained that the problem is there are imon devices which have
    two usb interfaces, even though it is one device. The probe and disconnect
    function of both usb interfaces can run concurrently.
    
    Alan Stern responded that the driver and USB cores guarantee that when an
    interface is probed, both the interface and its USB device are locked.
    Ditto for when the disconnect callback gets run. So concurrent probing/
    disconnection of multiple interfaces on the same device is not possible.
    
    Therefore, we don't need locks for handling race between imon_probe() and
    imon_disconnect(). But we still need to handle race between display_open()
    /vfd_write()/lcd_write()/display_close() and imon_disconnect(), for
    disconnect event can happen while file descriptors are in use.
    
    Since "struct file"->private_data is set by display_open(), vfd_write()/
    lcd_write()/display_close() can assume that "struct file"->private_data
    is not NULL even after usb_set_intfdata(interface, NULL) was called.
    
    Replace insufficiently held driver_lock with refcount_t based management.
    Add a boolean flag for recording whether imon_disconnect() was already
    called. Use RCU for accessing this boolean flag and refcount_t.
    
    Since the boolean flag for imon_disconnect() is shared, disconnect event
    on either intf0 or intf1 affects both interfaces. But I assume that this
    change does not matter, for usually disconnect event would not happen
    while interfaces are in use.
    
    Link: https://syzkaller.appspot.com/bug?extid=c558267ad910fc494497Reported-by: default avatarsyzbot <syzbot+c558267ad910fc494497@syzkaller.appspotmail.com>
    Signed-off-by: default avatarTetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
    Tested-by: default avatarsyzbot <syzbot+c558267ad910fc494497@syzkaller.appspotmail.com>
    Cc: Alan Stern <stern@rowland.harvard.edu>
    Signed-off-by: default avatarSean Young <sean@mess.org>
    Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@kernel.org>
    db264d4c
imon.c 70.2 KB