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?
Future attach_store() will succeed if cleanup operation (which does
vdev->ud.status = VDEV_ST_NULL;) is done, doesn't it?
And vhci_device_reset() and/or vhci_device_init() involves cleanup
operation (which does vdev->ud.status = VDEV_ST_NULL;), doesn't it?
>
>>>
>>> Does this patch fix the problem syzbot found?
>>
>> Yes, this patch as-is avoids the crash syzbot found.
>>
>
> Good to know. Please add handling for kthread_get_run() return
> in the places I suggested in you next version of this patch.
Since vhci_{rx,tx}_loop() do not involve cleanup operation (they simply
terminate upon kthread_should_stop() == true), I don't understand why
failing to start vhci_{rx,tx}_loop() makes difference. Cleanup will be
done by functions other than vhci_{rx,tx}_loop(), won't it?
