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

[Shuah Khan <skhan@linuxfoundation.org>]

On 2/10/21 11:43 AM, Tetsuo Handa wrote:
> On 2021/02/11 3:20, Shuah Khan wrote:
>> On 2/10/21 11:16 AM, Tetsuo Handa wrote:
>>> On 2021/02/11 3:11, Shuah Khan wrote:
>>>> 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.
>>>
>>> What problems are you aware of?
>>>
>>
>> The fact that driver doesn't cleanup after failing to create
>> the thread is a problem.
> 
> What are the cleanup functions?
> 

When user-space requests attaching to a device, attach_store()
tries to attach the requested device. When kthread_get_run()
failure is ignored silently, and continue with call to
rh_port_connect(), user-space assumes attach is successful.
User thinks attach is successful.

When and how will this attach failure gets reported to the
in this scenario?

Error handling for this case is no different from other error
paths in attach_store().

Please see error handling for other errors in attach_store().
In this case the right error handling is to rewind the vdev
init and bail out returning error. This would include setting
vdev->ud.status to VDEV_ST_NULL.

I found the following reproducer that tells me how attach
is triggered.
https://syzkaller.appspot.com/text?tag=ReproC&x=128506e4d00000

syzbot is helping us harden these paths, which is awesome.
Fixing these have to consider user api.

I you would like to fix this, please send me a complete fix.

thanks,
-- Shuah