linux-kernel - Re: [PATCH v4 3/5] usb: add apis for sideband uasge tracking

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: <2024101104-feminist-gulf-97e3@gregkh>
Date: Fri, 11 Oct 2024 06:30:58 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Guan-Yu Lin <guanyulin@...gle.com>
Cc: Thinh.Nguyen@...opsys.com, mathias.nyman@...el.com,
	stern@...land.harvard.edu, elder@...nel.org, oneukum@...e.com,
	yajun.deng@...ux.dev, dianders@...omium.org, kekrby@...il.com,
	perex@...ex.cz, tiwai@...e.com, tj@...nel.org,
	stanley_chang@...ltek.com, andreyknvl@...il.com,
	christophe.jaillet@...adoo.fr, quic_jjohnson@...cinc.com,
	ricardo@...liere.net, grundler@...omium.org, niko.mauno@...sala.com,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-sound@...r.kernel.org, badhri@...gle.com,
	albertccwang@...gle.com, quic_wcheng@...cinc.com,
	pumahsu@...gle.com
Subject: Re: [PATCH v4 3/5] usb: add apis for sideband uasge tracking

On Fri, Oct 11, 2024 at 12:14:00AM +0800, Guan-Yu Lin wrote:
> On Thu, Oct 10, 2024 at 2:33 PM Greg KH <gregkh@...uxfoundation.org> wrote:
> >
> > On Thu, Oct 10, 2024 at 01:30:00PM +0800, Guan-Yu Lin wrote:
> > > On Wed, Oct 9, 2024 at 8:44 PM Greg KH <gregkh@...uxfoundation.org> wrote:
> > > >
> > > > On Wed, Oct 09, 2024 at 05:42:57AM +0000, Guan-Yu Lin wrote:
> > > > > +             parent = parent->parent;
> > > > > +     } while (parent);
> > > >
> > > > Woah, walking up the device chain?  That should not be needed, or if so,
> > > > then each device's "usage count" is pointless.
> > > >
> > >
> > > Say a hub X with usb devices A,B,C attached on it, where usb device A
> > > is actively used by sideband now. We'd like to introduce a mechanism
> > > so that hub X won't have to iterate through all its children to
> > > determine sideband activities under this usb device tree.
> >
> > Why would a hub care?
> >
> 
> Without the information of sideband activities on the usb devices
> connected to the hub, the hub couldn't determine if it could suspend
> or not.

You are talking about an "internal" hub, right?  And isn't this already
covered by the original sideband patchset?  If not, how is power
management being handled there?

> > > This problem
> > > is similar to runtime suspending a device, where rpm uses
> > > power.usage_count for tracking activity of the device itself and
> > > power.child_count to check the children's activity. In our scenario,
> > > we don't see the need to separate activities on the device itself or
> > > on its children.
> >
> > But that's exactly what is needed here, if a hub wants to know what is
> > happening on a child device, it should just walk the list of children
> > and look :)
> >
> > > So we combine two counters in rpm as sb_usage_count,
> >
> > Combining counters is almost always never a good idea and will come back
> > to bite you in the end.  Memory isn't an issue here, speed isn't an
> > issue here, so why not just do it properly?
> >
> 
> By combining the two comments above, my understanding is that we should either:
> 1. separating the counter to one recording the sideband activity of
> itself, one for its children.
> 2. walk the list of children to check sideband activities on demand.
> Please correct me if I mistake your messages.

I think 2 is better, as this is infrequent and should be pretty fast to
do when needed, right?  But I really don't care, just don't combine
things together that shouldn't be combined.

> > > denoting the sideband activities under a specific usb device. We have
> > > to keep a counter in each device so that we won't influence the usb
> > > devices that aren't controlled by a sideband.
> >
> > I can understand that for the device being "controlled" by a sideband,
> > but that's it.
> >
> > > When sideband activity changes on a usb device, its usb device parents
> > > should all get notified to maintain the correctness of sb_usage_count.
> >
> > Why "should" they?  They shouldn't really care.
> >
> 
> Hubs need the sideband activity information on downstream usb devices,
> so that the hub won't suspend the upstream usb port when there is a
> sideband accessing the usb device connected to it.

Then why doesn't the sideband code just properly mark the "downstream"
device a being used like any other normal device?  Why is this acting
"special"?

thanks,

greg k-h