Re: [PATCH] usb: usbip: fix error handling of kthread_get_run()

[Shuah Khan <skhan@linuxfoundation.org>]

On 2/5/21 6:08 PM, Tetsuo Handa wrote:
> On 2021/02/06 1:27, Shuah Khan wrote:
>> Good find. For this fix to be complete, you will have to add checks
>> for kthread_get_run() NULL return in attach_store() and
>> usbip_sockfd_store() routines in stub_dev.c and vudc_sysfs.c
> 
> Initially I thought that the cleaner fix is to get kthread_create() out of kthread_get_run()
> ( the drivers/usb/usbip/vhci_sysfs.c portion in
> https://syzkaller.appspot.com/x/patch.diff?x=16c3c090d00000 ) so that we can undo
> kthread_create() via kthread_stop(). But I found that such fix makes little sense because
> it is possible that SIGKILL is delivered between vhci_rx_loop() and vhci_tx_loop() have
> started and before leaving attach_store().
> 
> Since the code prior to "staging/usbip: convert to kthread" was already capable of surviving
> such race condition, this patch should be already good enough for sending to stable kernels.
> Of course, since kthread_create() may return -ENOMEM without being SIGKILLed, we could update
> attach_store() to report kthread_get_run() failure to the caller, but that will be a separate
> patch. This patch alone avoids the crash although there is a hung task problem similar to
> https://syzkaller.appspot.com/bug?id=5677eeeb83e5d47ef2b04e9bd68f5ff4c7e572ab remains
> ( https://syzkaller.appspot.com/text?tag=CrashReport&x=17aa3f78d00000 ). The cause of hung
> task is currently unknown; maybe too much printk() messages.
> 

I would like to see to see a complete fix. This patch changes
kthread_get_run() to return NULL. Without adding handling for
NULL in the callers of kthread_get_run(), we will start seeing
problems.

Does this patch fix the problem syzbot found?

thanks,
-- Shuah