netdev - Re: [PATCH] usb: core: Save the config when a device is deauthorized+authorized

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: <62b7467f-f142-459d-aa23-8bfd70bbe733@rowland.harvard.edu>
Date: Fri, 1 Dec 2023 10:59:53 -0500
From: Alan Stern <stern@...land.harvard.edu>
To: Douglas Anderson <dianders@...omium.org>
Cc: linux-usb@...r.kernel.org,
  Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
  Simon Horman <horms@...nel.org>, Grant Grundler <grundler@...omium.org>,
  Hayes Wang <hayeswang@...ltek.com>,
  Bjørn Mork <bjorn@...k.no>,
  Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
  "David S . Miller" <davem@...emloft.net>, Brian Geffon <bgeffon@...gle.com>,
  Bastien Nocera <hadess@...ess.net>,
  Benjamin Tissoires <benjamin.tissoires@...hat.com>,
  Flavio Suligoi <f.suligoi@...m.it>,
  Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
  Ricardo Cañuelo <ricardo.canuelo@...labora.com>,
  Rob Herring <robh@...nel.org>, Roy Luo <royluo@...gle.com>,
  Stanley Chang <stanley_chang@...ltek.com>,
  Vincent Mailhol <mailhol.vincent@...adoo.fr>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] usb: core: Save the config when a device is
 deauthorized+authorized

On Thu, Nov 30, 2023 at 03:43:47PM -0800, Douglas Anderson wrote:
> Right now, when a USB device is deauthorized (by writing 0 to the
> "authorized" field in sysfs) and then reauthorized (by writing a 1) it
> loses any configuration it might have had. This is because
> usb_deauthorize_device() calls:
>   usb_set_configuration(usb_dev, -1);
> ...and then usb_authorize_device() calls:
>   usb_choose_configuration(udev);
> ...to choose the "best" configuration.
> 
> This generally works OK and it looks like the above design was chosen
> on purpose. In commit 93993a0a3e52 ("usb: introduce
> usb_authorize/deauthorize()") we can see some discussion about keeping
> the old config but it was decided not to bother since we can't save it
> for wireless USB anyway. It can be noted that as of commit
> 1e4c574225cc ("USB: Remove remnants of Wireless USB and UWB") wireless
> USB is removed anyway, so there's really not a good reason not to keep
> the old config.
> 
> Unfortunately, throwing away the old config breaks when something has
> decided to choose a config other than the normal "best" config.
> Specifically, it can be noted that as of commit ec51fbd1b8a2 ("r8152:
> add USB device driver for config selection") that the r8152 driver
> subclasses the generic USB driver and selects a config other than the
> one that would have been selected by usb_choose_configuration(). This
> logic isn't re-run after a deauthorize + authorize and results in the
> r8152 driver not being re-bound.
> 
> Let's change things to save the old config when we deauthorize and
> then restore it when we re-authorize. We'll disable this logic for
> wireless USB where we re-fetch the descriptor after authorization.

Would it be better to make the r8152 driver override 
usb_choose_configuration()?  This is the sort of thing that subclassing 
is intended for.

Alan Stern