linux-kernel - Re: [PATCH v6 5/5] usb: host: enable sideband transfer during system sleep

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOuDEK1r+2f-nXBRO-i0NZ7oriM93kyQzgKMpw+ocqgmWBBpoA@mail.gmail.com>
Date: Wed, 6 Nov 2024 16:38:43 +0800
From: Guan-Yu Lin <guanyulin@...gle.com>
To: gregkh@...uxfoundation.org, Thinh.Nguyen@...opsys.com, 
	mathias.nyman@...el.com, stern@...land.harvard.edu, sumit.garg@...aro.org, 
	dianders@...omium.org, kekrby@...il.com, oneukum@...e.com, 
	yajun.deng@...ux.dev, niko.mauno@...sala.com, christophe.jaillet@...adoo.fr, 
	tj@...nel.org, stanley_chang@...ltek.com, andreyknvl@...il.com, 
	quic_jjohnson@...cinc.com, ricardo@...liere.net
Cc: linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v6 5/5] usb: host: enable sideband transfer during system sleep

On Wed, Nov 6, 2024 at 4:35 PM Guan-Yu Lin <guanyulin@...gle.com> wrote:
>
> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index e53cb4c267b3..e5bb26e6c71a 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -1583,6 +1583,11 @@ int usb_suspend(struct device *dev, pm_message_t msg)
>         struct usb_device       *udev = to_usb_device(dev);
>         int r;
>
> +       if (msg.event == PM_EVENT_SUSPEND && usb_sideband_check(udev)) {
> +               dev_dbg(dev, "device accessed via sideband\n");
> +               return 0;
> +       }
> +
>         unbind_no_pm_drivers_interfaces(udev);
>
>         /* From now on we are sure all drivers support suspend/resume
> @@ -1619,6 +1624,11 @@ int usb_resume(struct device *dev, pm_message_t msg)
>         struct usb_device       *udev = to_usb_device(dev);
>         int                     status;
>
> +       if (msg.event == PM_EVENT_RESUME && usb_sideband_check(udev)) {
> +               dev_dbg(dev, "device accessed via sideband\n");
> +               return 0;
> +       }
> +
>         /* For all calls, take the device back to full power and
>          * tell the PM core in case it was autosuspended previously.
>          * Unbind the interfaces that will need rebinding later,

In v5, Greg points out the race window between checking sideband
activity and handling power management of usb devices. We should
consider a lock mechanism to address the race window. Given that the
design hasn't locked down and the race window might change from time
to time. I'll address this after the discussion of suspending USB
devices/interfaces has converged.

In addition, Alan suggests to only keep USB devices active but suspend
the USB interfaces. However, hub events and key events require active
USB interfaces to function. In the sideband model, the sideband driver
only handles USB transfers on specific endpoints, leaving other
functionalities like connection changes and key events to the Linux
USB kernel drivers. Therefore, a potential design modification is to
shift the sideband model's focus from voting for USB devices to voting
for USB interfaces. This way, the driver could selectively hold
necessary interfaces active during system suspend. This adjustment
accommodates use cases where specific interfaces must remain active to
support overall USB functionality when partial interfaces are
offloaded to a sideband.