[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CANEJEGuJFq3Wf8=DxTnrbENi586ccsi7Y+pgQWOSaqzvCp2aZg@mail.gmail.com>
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
>
Powered by blists - more mailing lists