netdev - Re: [PATCH net 2/2] r8152: Switch to using choose

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]

Date: Mon, 25 Dec 2023 23:18:48 -0800
From: Grant Grundler <grundler@...omium.org>
To: Maxim Mikityanskiy <maxtram95@...il.com>
Cc: "David S. Miller" <davem@...emloft.net>, Bjørn Mork <bjorn@...k.no>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Hayes Wang <hayeswang@...ltek.com>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Douglas Anderson <dianders@...omium.org>, Grant Grundler <grundler@...omium.org>, linux-usb@...r.kernel.org, 
	netdev@...r.kernel.org
Subject: Re: [PATCH net 2/2] r8152: Switch to using choose_configuration

On Sat, Dec 23, 2023 at 3:36 PM Maxim Mikityanskiy <maxtram95@...il.com> wrote:
>
> With the introduction of r8152-cfgselector, the following regression
> appeared on machines that use usbguard: the netdev appears only when the
> USB device is inserted the first time (before the module is loaded), but
> on the second and next insertions no netdev is registered.
>
> It happens because the device is probed as unauthorized, and usbguard
> gives it an authorization a moment later. If the module is not loaded,
> it's normally loaded slower than the authorization is given, and
> everything works. If the module is already loaded, the cfgselector's
> probe function runs first, but then usb_authorize_device kicks in and
> changes the configuration to something chosen by the standard
> usb_choose_configuration. rtl8152_probe refuses to probe non-vendor
> configurations, and the user ends up without a netdev.
>
> The previous commit added possibility to override
> usb_choose_configuration. Use it to fix the bug and pick the right
> configuration on both probe and authorization.
>
> Fixes: ec51fbd1b8a2 ("r8152: add USB device driver for config selection")
> Signed-off-by: Maxim Mikityanskiy <maxtram95@...il.com>

Maxim,
Does this solve the same problem that Doug Anderson posted patches for
a few weeks ago?

https://lore.kernel.org/all/20231201183113.343256-1-dianders@chromium.org/

Those went through the USB tree, so that probably explains why you
didn't see them in the net.git tree.

cheers,
grant

> ---
>  drivers/net/usb/r8152.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 9bf2140fd0a1..f0ac31a94f3c 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -10070,6 +10070,11 @@ static struct usb_driver rtl8152_driver = {
>  };
>
>  static int rtl8152_cfgselector_probe(struct usb_device *udev)
> +{
> +       return 0;
> +}
> +
> +static int rtl8152_cfgselector_choose_configuration(struct usb_device *udev)
>  {
>         struct usb_host_config *c;
>         int i, num_configs;
> @@ -10078,7 +10083,7 @@ static int rtl8152_cfgselector_probe(struct usb_device *udev)
>          * driver supports it.
>          */
>         if (__rtl_get_hw_ver(udev) == RTL_VER_UNKNOWN)
> -               return 0;
> +               return -EOPNOTSUPP;
>
>         /* The vendor mode is not always config #1, so to find it out. */
>         c = udev->config;
> @@ -10094,20 +10099,15 @@ static int rtl8152_cfgselector_probe(struct usb_device *udev)
>         }
>
>         if (i == num_configs)
> -               return -ENODEV;
> -
> -       if (usb_set_configuration(udev, c->desc.bConfigurationValue)) {
> -               dev_err(&udev->dev, "Failed to set configuration %d\n",
> -                       c->desc.bConfigurationValue);
> -               return -ENODEV;
> -       }
> +               return -EOPNOTSUPP;
>
> -       return 0;
> +       return c->desc.bConfigurationValue;
>  }
>
>  static struct usb_device_driver rtl8152_cfgselector_driver = {
>         .name =         MODULENAME "-cfgselector",
>         .probe =        rtl8152_cfgselector_probe,
> +       .choose_configuration = rtl8152_cfgselector_choose_configuration,
>         .id_table =     rtl8152_table,
>         .generic_subclass = 1,
>         .supports_autosuspend = 1,
> --
> 2.43.0
>