All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
To: Shuah Khan <skhan@linuxfoundation.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Valentina Manea <valentina.manea.m@gmail.com>,
	Shuah Khan <shuah@kernel.org>
Cc: Hillf Danton <hdanton@sina.com>,
	linux-usb@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>
Subject: Re: [PATCH] usb: usbip: fix error handling of kthread_get_run()
Date: Thu, 11 Feb 2021 03:43:25 +0900	[thread overview]
Message-ID: <bb8f438f-8a77-2aac-cb2b-b2551f6a64b0@i-love.sakura.ne.jp> (raw)
In-Reply-To: <2f922e76-623e-1d87-17a5-c4a87dc8f2fc@linuxfoundation.org>

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?


  reply	other threads:[~2021-02-10 18:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-20 18:44 KASAN: null-ptr-deref Write in vhci_shutdown_connection syzbot
2021-02-05 13:57 ` [PATCH] usb: usbip: fix error handling of kthread_get_run() Tetsuo Handa
2021-02-05 16:27   ` Shuah Khan
2021-02-06  1:08     ` Tetsuo Handa
2021-02-10 18:11       ` Shuah Khan
2021-02-10 18:16         ` Tetsuo Handa
2021-02-10 18:20           ` Shuah Khan
2021-02-10 18:43             ` Tetsuo Handa [this message]
2021-02-10 20:15               ` Shuah Khan
2021-02-11  1:04                 ` Tetsuo Handa
2021-02-11  3:01                   ` Tetsuo Handa
2021-02-11 13:40                     ` Tetsuo Handa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bb8f438f-8a77-2aac-cb2b-b2551f6a64b0@i-love.sakura.ne.jp \
    --to=penguin-kernel@i-love.sakura.ne.jp \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=hdanton@sina.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=shuah@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=valentina.manea.m@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.