linux-kernel - Re: [PATCH next] usb: usbfs: Add reset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <987f97f4-293a-42f0-a9f5-9d67b6db2ce5@kylinos.cn>
Date: Thu, 18 Jul 2024 11:23:02 +0800
From: Hongyu Xie <xy521521@...il.com>
To: stern@...land.harvard.edu,
	oneukum@...e.com,
	xy521521@...il.com
Cc: gregkh@...uxfoundation.org,
	brauner@...nel.org,
	jlayton@...nel.org,
	jack@...e.cz,
	linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	xiehongyu1@...inos.cn
Subject: Re: [PATCH next] usb: usbfs: Add reset_resume for usbfs

From: Hongyu Xie <xiehongyu1@...inos.cn>



On 2024/7/17 15:42, Oliver Neukum wrote:
> On 17.07.24 03:43, Hongyu Xie wrote:
>> From: Hongyu Xie <xiehongyu1@...inos.cn>
> 
> Hi,
> 
> sorry for being incomprehensible. I'll try to do better.
> 
>>  From what I know, that CONFIG_USB_DEFAULT_PERSIST is enabled by 
>> default. Then udev->persist_enabled is set to 1 and this causing 
>> udev->reset_resume set to 1 during init2 in hub_activate.
>> During resume,
>> usb_resume_both
>>    usb_resume_device
>>      generic_resume
>>        usb_port_resume
>>          finish_port_resume
>>            usb_reset_and_verify_device (if udev->reset_resume is 1)
>>              hub_port_init
>>                hub_port_reset
>>    usb_resume_interface (udev->reset_resume is 1 but usbfs doesn't 
>> have reset_resume implementation so set intf->needs_binding to 1, and 
>> it will be rebind in usb_resume_complete)
> 
> That is correct. But even if CONFIG_USB_DEFAULT_PERSIST were not set, 
> losing power
> would just lead to reenumeration by another code path. Devices reset 
> themselves
> when they are power cycled. There is no way around that.
> 
>> Even before usbfs->reset_resume is called (if there is one), the USB 
>> device has already been reset
> 
> Yes, it has been reset.
> 
>> and in a good state.
> 
> No, it is not. Or rather, it is in the wrong state. This is not a 
> question of good and bad.
> It is a question of being in the same state.
After resume, it's in USB_STATE_CONFIGURED state. But here I'm guessing 
you mean not in a good state from user-space's point of view, right?
> 
>> After all that thaw_processes is called and user-space application 
>> runs again.
> 
> Yes. And user space does not know what has happened.
>>
>> So I still don't understand why "the race necessarily exists". Can you 
>> show me an example that usbfs->reset_resume causes race?
> 
> Sure. Let's look at the example of a scanner attached to the host.
> 
> OS            Scanning process (in user space)
>              1. Set a resolution
> 2. Going to S4
> 3. Returning to S0
>              4. Initiate a scan
> 
> As you can see the system would now scan at the wrong resolution. Step#4
> has to fail. As there is no synchronization between S4 and user space, 
> initiating
> the scan can always be the last step.
> For this to work, reset_resume() would have to restore the correct 
> resolution. The kernel
> cannot do so.
Now I can see your point. And I think with or without 
usbfs->reset_resume right now, Step#4 has to fail.
> 
>      Regards
>          Oliver
Regards
Hongyu Xie