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: <20251013110753.0f640774.michal.pecio@gmail.com>
Date: Mon, 13 Oct 2025 11:07:53 +0200
From: Michal Pecio <michal.pecio@...il.com>
To: yicongsrfy@....com
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, oliver@...kum.org,
 linux-usb@...r.kernel.org, netdev@...r.kernel.org, Yi Cong
 <yicong@...inos.cn>
Subject: Re: [PATCH net v5 2/3] net: usb: ax88179_178a: add USB device
 driver for config selection

On Sat, 11 Oct 2025 15:53:13 +0800, yicongsrfy@....com wrote:
> From: Yi Cong <yicong@...inos.cn>
> 
> A similar reason was raised in commit ec51fbd1b8a2 ("r8152: add USB
> device driver for config selection"):
> Linux prioritizes probing non-vendor-specific configurations.
> 
> Referring to the implementation of this patch, cfgselect is also
> used for ax88179 to override the default configuration selection.
> 
> Signed-off-by: Yi Cong <yicong@...inos.cn>
> 
> ---
> v2: fix warning from checkpatch.
> v5: 1. use KBUILD_MODNAME to obtain the module name.
>     2. add error handling when usb_register fail.
>     3. use .choose_configuration instead of .probe.
>     4. reorder deregister logic.
> ---
>  drivers/net/usb/ax88179_178a.c | 68 ++++++++++++++++++++++++++++++++--
>  1 file changed, 65 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/usb/ax88179_178a.c b/drivers/net/usb/ax88179_178a.c
> index b034ef8a73ea..b6432d414a38 100644
> --- a/drivers/net/usb/ax88179_178a.c
> +++ b/drivers/net/usb/ax88179_178a.c
> @@ -1713,6 +1713,14 @@ static int ax88179_stop(struct usbnet *dev)
>  	return 0;
>  }
>  
> +static int ax88179_probe(struct usb_interface *intf, const struct usb_device_id *i)
> +{
> +	if (intf->cur_altsetting->desc.bInterfaceClass != USB_CLASS_VENDOR_SPEC)
> +		return -ENODEV;
> +
> +	return usbnet_probe(intf, i);
> +}

This isn't part of the cfgselector driver being added by this commit
nor is it documented in the changelog, so why is it here?

It doesn't seem to be necessary, because USB_DEVICE_AND_INTERFACE_INFO
matches used by this driver ensure that probe() will only be called on
interfaces of the correct class 0xff.

> +
>  static const struct driver_info ax88179_info = {
>  	.description = "ASIX AX88179 USB 3.0 Gigabit Ethernet",
>  	.bind = ax88179_bind,
> @@ -1941,9 +1949,9 @@ static const struct usb_device_id products[] = {
>  MODULE_DEVICE_TABLE(usb, products);
>  
>  static struct usb_driver ax88179_178a_driver = {
> -	.name =		"ax88179_178a",
> +	.name =		KBUILD_MODNAME,
>  	.id_table =	products,
> -	.probe =	usbnet_probe,
> +	.probe =	ax88179_probe,
>  	.suspend =	ax88179_suspend,
>  	.resume =	ax88179_resume,
>  	.reset_resume =	ax88179_resume,
> @@ -1952,7 +1960,61 @@ static struct usb_driver ax88179_178a_driver = {
>  	.disable_hub_initiated_lpm = 1,
>  };
>  
> -module_usb_driver(ax88179_178a_driver);
> +static int ax88179_cfgselector_choose_configuration(struct usb_device *udev)
> +{
> +	struct usb_host_config *c;
> +	int i, num_configs;
> +
> +	/* The vendor mode is not always config #1, so to find it out. */
> +	c = udev->config;
> +	num_configs = udev->descriptor.bNumConfigurations;
> +	for (i = 0; i < num_configs; (i++, c++)) {
> +		struct usb_interface_descriptor	*desc = NULL;
> +
> +		if (!c->desc.bNumInterfaces)
> +			continue;
> +		desc = &c->intf_cache[0]->altsetting->desc;
> +		if (desc->bInterfaceClass == USB_CLASS_VENDOR_SPEC)
> +			break;
> +	}
> +
> +	if (i == num_configs)
> +		return -ENODEV;
> +
> +	return c->desc.bConfigurationValue;
> +}

I wonder how many copies of this code would justify making it some
sort of library in usbnet or usbcore?

> +static struct usb_device_driver ax88179_cfgselector_driver = {
> +	.name =	KBUILD_MODNAME "-cfgselector",
> +	.choose_configuration =	ax88179_cfgselector_choose_configuration,
> +	.id_table = products,
> +	.generic_subclass = 1,
> +	.supports_autosuspend = 1,
> +};
> +
> +static int __init ax88179_driver_init(void)
> +{
> +	int ret;
> +
> +	ret = usb_register_device_driver(&ax88179_cfgselector_driver, THIS_MODULE);
> +	if (ret)
> +		return ret;
> +
> +	ret = usb_register(&ax88179_178a_driver);
> +	if (ret)
> +		usb_deregister_device_driver(&ax88179_cfgselector_driver);

Any problems if the order of registration is reversed, to ensure that
the interface driver always exists if the device driver exists?

> +
> +	return 0;

return ret perhaps?

> +}
> +
> +static void __exit ax88179_driver_exit(void)
> +{
> +	usb_deregister_device_driver(&ax88179_cfgselector_driver);
> +	usb_deregister(&ax88179_178a_driver);
> +}
> +
> +module_init(ax88179_driver_init);
> +module_exit(ax88179_driver_exit);
>  
>  MODULE_DESCRIPTION("ASIX AX88179/178A based USB 3.0/2.0 Gigabit Ethernet Devices");
>  MODULE_LICENSE("GPL");
> -- 
> 2.25.1
>