linux-kernel - Re: [PATCH] usb: storage: Add ums-cros-aoa driver

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: <Pine.LNX.4.44L0.1908301337150.1459-100000@iolanthe.rowland.org>
Date:   Fri, 30 Aug 2019 13:43:36 -0400 (EDT)
From:   Alan Stern <stern@...land.harvard.edu>
To:     Julius Werner <jwerner@...omium.org>, Greg KH <greg@...ah.com>
cc:     Dan Williams <dcbw@...hat.com>,
        Kernel development list <linux-kernel@...r.kernel.org>,
        USB list <linux-usb@...r.kernel.org>,
        USB Storage list <usb-storage@...ts.one-eyed-alien.net>
Subject: Re: [PATCH] usb: storage: Add ums-cros-aoa driver

On Thu, 29 Aug 2019, Julius Werner wrote:

> > In fact, there already is a way to do this in the kernel: write to the
> > sysfs "bind" file.  The difficulty is that you can't force a driver to
> > bind to an interface if it doesn't believe it is compatible with the
> > interface.  And if the driver believes it is compatible, it will
> > automatically attempt to bind with all such interfaces regardless of
> > their path.
> >
> > Perhaps what you need is a usb_device_id flag to indicate that the
> > entry should never be used for automatic matches -- only for matches
> > made by the user via the "bind" file.  Greg KH would probably be
> > willing to accept a new USB_DEVICE_ID_MATCH_NO_AUTO flag, which
> > could be included in your unusual_devs.h entries.
> 
> This is an interesting idea, but I don't quite see how it can work as
> you described? When I write to 'bind', the driver core calls
> driver_match_device(), which ends up calling usb_device_match()
> (right?), which is the same path that it would call for automatic
> matching.

Oh, too bad.  I had a vague memory that it did not call
driver_match_device().

>  It still ends up in usb_match_one_id(), and if I check for
> the NO_AUTO flag there it would abort just as if it was an auto-match
> attempt. I see no way to pass the information that this is an
> explicit, user-requested "bind" rather than an automatic match across
> the bus->match() callback into the USB code. (I could change the
> signature of the match() callback, but that would require changing
> code for all device busses in Linux, which I assume is something we
> wouldn't want to do? I could also add a flag to struct device to
> communicate "this is currently trying to match for a user-initiated
> bind", but that seems hacky and not really the right place to put
> that.)
> 
> I could instead add a new sysfs node 'force_bind' to the driver core,
> that would work like 'bind' except for skipping the
> driver_match_device() call entirely and forcing a probe(). Do you
> think that would be acceptable? Or is that too big of a hammer to make
> available for all drivers in Linux? Maybe if I do the same thing but
> only for usb drivers, or even only for the usb-storage driver
> specifically, would that be acceptable?

This is a question for Greg.  The problem is that there may be drivers
which can't handle being probed for devices they don't match.

Still, we ought to have a mechanism for doing manual but not automatic 
matches.

Greg, any thoughts?

> If none of this works, I could also extend the new_id interface to
> allow subclass/protocol matches instead. I don't like that as much
> because it doesn't allow me to control the devpath of the device I'm
> matching, but I think it would be enough for my use case (I can't make
> the usb-storage driver bind all AOA devices at all times, but at the
> times where I do want to use it for my one device, I don't expect any
> other AOA devices to be connected). The problem with this is that the
> order of arguments for new ID is already set in stone (vendor,
> product, interface class, refVendor, refProduct), and I don't think I
> can use the refVendor/refProduct for my case so I can't just append
> more numbers behind that. I could maybe instead change it so that it
> also accepts a key-value style line (like "idVendor=abcd
> idProduct=efgh bInterfaceSubClass=ff"), while still being
> backwards-compatible to the old format if you only give it numbers?
> What do you think?

I prefer the manual/automatic approach.  It allows the user to control 
exactly which device will be probed, which could be important.

Alan Stern