netdev - Re: [PATCH net v5 2/3] net: usb: ax88179_178a: add USB device driver for config selection

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: <6640b191-d25b-4c4e-ac67-144357eb5cc3@rowland.harvard.edu>
Date: Sat, 18 Oct 2025 11:36:11 -0400
From: Alan Stern <stern@...land.harvard.edu>
To: Michal Pecio <michal.pecio@...il.com>
Cc: yicongsrfy@....com, andrew+netdev@...n.ch, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, linux-usb@...r.kernel.org,
	netdev@...r.kernel.org, oliver@...kum.org, pabeni@...hat.com
Subject: Re: [PATCH net v5 2/3] net: usb: ax88179_178a: add USB device driver
 for config selection

On Sat, Oct 18, 2025 at 05:21:56PM +0200, Michal Pecio wrote:
> Existing r8152-cfgselector and the planned ax88179-cfgselector
> implement the following logic:
> 
> IF a device has particular IDs
>    (same id_table as in the vendor interface driver)
> 
> IF the vendor interface driver is loaded
>    (ensured by loading it together with cfgselector)
> 
> IF the vendor driver supports this device
>    (calls internal vendor driver code)
> 
> THEN select the vendor configuration
> 
> 
> It was a PITA, but I have a working proof of concept for r8152.
> 
> Still missing is automatic reevaluation of configuration choice when
> the vendor driver is loaded after device connection (e.g. by udev).
> Those cfgselectors can do it because it seems that registering a new
> device (but not interface) driver forces reevaluation.

It looks like something else is missing too...

> diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
> index d29edc7c616a..eaf21c30eac1 100644
> --- a/drivers/usb/core/driver.c
> +++ b/drivers/usb/core/driver.c
> @@ -1119,6 +1119,29 @@ void usb_deregister(struct usb_driver *driver)
>  }
>  EXPORT_SYMBOL_GPL(usb_deregister);
>  
> +/**
> + * usb_driver_preferred - check if this is a preferred interface driver
> + * @drv: interface driver to check (device drivers are ignored)
> + * @udev: the device we are looking up a driver for
> + * Context: must be able to sleep
> + *
> + * TODO locking?
> + */
> +bool usb_driver_preferred(struct device_driver *drv, struct usb_device *udev)
> +{
> +	struct usb_driver *usb_drv;
> +
> +	if (is_usb_device_driver(drv))
> +		return false;
> +
> +	usb_drv = to_usb_driver(drv);
> +
> +	return usb_drv->preferred &&
> +		usb_device_match_id(udev, usb_drv->id_table) &&
> +		usb_drv->preferred(udev);
> +}
> +EXPORT_SYMBOL_GPL(usb_driver_preferred);

> diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
> index a48994e11ef3..1923e6f4923b 100644
> --- a/drivers/usb/core/generic.c
> +++ b/drivers/usb/core/generic.c
> @@ -49,11 +49,17 @@ static bool is_uac3_config(struct usb_interface_descriptor *desc)
>  	return desc->bInterfaceProtocol == UAC_VERSION_3;
>  }
>  
> +static int prefer_vendor(struct device_driver *drv, void *data)
> +{
> +	return usb_driver_preferred(drv, data);
> +}
> +
>  int usb_choose_configuration(struct usb_device *udev)
>  {
>  	int i;
>  	int num_configs;
>  	int insufficient_power = 0;
> +	bool class_found = false;
>  	struct usb_host_config *c, *best;
>  	struct usb_device_driver *udriver;
>  
> @@ -169,6 +175,12 @@ int usb_choose_configuration(struct usb_device *udev)
>  #endif
>  		}
>  
> +		/* Check if we have a preferred vendor driver for this config */
> +		else if (bus_for_each_drv(&usb_bus_type, NULL, (void *) udev, prefer_vendor)) {
> +			best = c;
> +			break;
> +		}

How are prefer_vendor() and usb_driver_preferred() supposed to know 
which configuration is being considered?

(Also, is prefer_vendor() really needed?  Can't you just pass 
usb_driver_preferred as the argument to bus_for_each_drv()?  Maybe after 
changing the type of its second argument to void * instead of struct 
usb_device *?)

Alan Stern