Re: [PATCH 5.8 05/85] Revert "usbip: Implement a match function to fix usbip"

["M. Vefa Bicakci" <m.v.b@runbox.com>]

On 07/10/2020 12.13, Greg Kroah-Hartman wrote:
> On Tue, Oct 06, 2020 at 04:26:27PM +0300, M. Vefa Bicakci wrote:
>> On 05/10/2020 18.26, Greg Kroah-Hartman wrote:
>>> From: M. Vefa Bicakci <m.v.b@runbox.com>
>>>
>>> commit d6407613c1e2ef90213dee388aa16b6e1bd08cbc upstream.
>>>
>>> This commit reverts commit 7a2f2974f265 ("usbip: Implement a match
>>> function to fix usbip").
>>>
>>> In summary, commit d5643d2249b2 ("USB: Fix device driver race")
>>> inadvertently broke usbip functionality, which I resolved in an incorrect
>>> manner by introducing a match function to usbip, usbip_match(), that
>>> unconditionally returns true.
>>>
>>> However, the usbip_match function, as is, causes usbip to take over
>>> virtual devices used by syzkaller for USB fuzzing, which is a regression
>>> reported by Andrey Konovalov.
>>>
>>> Furthermore, in conjunction with the fix of another bug, handled by another
>>> patch titled "usbcore/driver: Fix specific driver selection" in this patch
>>> set, the usbip_match function causes unexpected USB subsystem behaviour
>>> when the usbip_host driver is loaded. The unexpected behaviour can be
>>> qualified as follows:
>>> - If commit 41160802ab8e ("USB: Simplify USB ID table match") is included
>>>     in the kernel, then all USB devices are bound to the usbip_host
>>>     driver, which appears to the user as if all USB devices were
>>>     disconnected.
>>> - If the same commit (41160802ab8e) is not in the kernel (as is the case
>>>     with v5.8.10) then all USB devices are re-probed and re-bound to their
>>>     original device drivers, which appears to the user as a disconnection
>>>     and re-connection of USB devices.
>>>
>>> Please note that this commit will make usbip non-operational again,
>>> until yet another patch in this patch set is merged, titled
>>> "usbcore/driver: Accommodate usbip".
>>>
>>> Cc: <stable@vger.kernel.org> # 5.8: 41160802ab8e: USB: Simplify USB ID table match
>>> Cc: <stable@vger.kernel.org> # 5.8
>>
>> Hello Greg,
>>
>> Sorry for the lateness of this e-mail.
>>
>> I had noted commit 41160802ab8e ("USB: Simplify USB ID table match") as a
>> prerequisite in the commit message, but I just realized that the commit
>> identifier refers to a commit in my local git tree, and not to the actual
>> commit in Linus Torvalds' git tree! I apologize for this mistake.
>>
>> Here is the correct commit identifier:
>>    0ed9498f9ecfde50c93f3f3dd64b4cd5414d9944 ("USB: Simplify USB ID table match")
>>
>> Perhaps this is why the prerequisite commit was not cherry-picked to the 5.8.y
>> branch.
>>
>> As a justification for the cherry-pick, commit 0ed9498f9ecf actually resolves
>> a bug. In summary, this commit works together with commit adb6e6ac20ee ("USB:
>> Also match device drivers using the ->match vfunc", which has been cherry-picked
>> as part of v5.8.6) and ensures that a USB driver's ->match function is also
>> called during the search for a more specialized/appropriate USB driver, in case
>> the driver in question does not have an id_table.
>>
>> If I am to be the devil's advocate, however, then given that there is only one
>> specialized USB device driver ("apple-mfi-fastcharge"), which conveniently has
>> an id_table, and also given that usbip no longer has a match function, I also
>> realize that it may not be crucial to cherry-pick 0ed9498f9ecf as a prerequisite
>> commit.
> 
> I'm sorry, but I don't really understand.  Does 5.8.y need an additional
> patch here, or is all ok because I missed the above patch?

No worries; sorry for not communicating clearly and for the delay.

I meant to state that it would be good to have commit 0ed9498f9ecf ("USB: Simplify
USB ID table match") cherry picked to 5.8.y, as it fixes a bug, albeit a minor one.

Thank you,

Vefa