On 2020/06/19 22:56, Andrey Konovalov wrote: > Oliver, any chance you could help us with fixing the hang in this > driver? You seem to be its original author. This hang is one of the > top crashers on syzbot, with over 32000 crashed kernels. > Yes, I think that wdm_flush() has another bug and wdm_write() has yet another bug. I need the authors' comments. wdm_flush() says /* cannot dereference desc->intf if WDM_DISCONNECTING */ if (test_bit(WDM_DISCONNECTING, &desc->flags)) return -ENODEV; if (desc->werr < 0) dev_err(&desc->intf->dev, "Error in flush path: %d\n", desc->werr); but it seems to me that nothing guarantees that test_bit(WDM_DISCONNECTING) == false indicates dereferencing desc->intf->dev is safe, for wdm_flush() tests WDM_DISCONNECTING without any lock whereas wdm_disconnect() sets WDM_DISCONNECTING under wdm_mutex and desc->iuspin held. It might be safe to dereference from wdm_release() which holds wdm_mutex. Also, if wait_event() in wdm_flush() might fail to wake up (due to close() dependency problem this crash report is focusing on), wait_event_interruptible() in wdm_write() might also fail to wake up (unless interrupted) due to the same dependency. Then, why can't we wait for completion of wdm_out_callback() (with reasonable timeout) inside wdm_write() ? I feel that wdm_flush() is so bogus (which could/should be removed).
