netdev - Re: [PATCH v4 3/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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <666ef6bf-46f0-4b3e-9c28-9c9b7e602900@suse.com>
Date: Tue, 30 Sep 2025 10:57:05 +0200
From: Oliver Neukum <oneukum@...e.com>
To: yicongsrfy@....com, oneukum@...e.com, andrew+netdev@...n.ch
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 linux-usb@...r.kernel.org, marcan@...can.st, netdev@...r.kernel.org,
 pabeni@...hat.com, yicong@...inos.cn
Subject: Re: [PATCH v4 3/3] net: usb: ax88179_178a: add USB device driver for
 config selection

Hi,

On 30.09.25 10:07, yicongsrfy@....com wrote:
   
> +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;

This check is only necessary because you have disabled the checking
done in usbcore with the first patch in your series.
The patch you are reverting with first patch is correct however.
ax88179 can drive these devices only if they are in the vendor
specific mode.

> +	return usbnet_probe(intf, i);
> +}

> +
>   static const struct driver_info ax88179_info = {
>   	.description = "ASIX AX88179 USB 3.0 Gigabit Ethernet",
>   	.bind = ax88179_bind,
> @@ -1941,9 +1950,9 @@ static const struct usb_device_id products[] = {
>   MODULE_DEVICE_TABLE(usb, products);
>   
>   static struct usb_driver ax88179_178a_driver = {
> -	.name =		"ax88179_178a",
> +	.name =		MODULENAME,
>   	.id_table =	products,
> -	.probe =	usbnet_probe,
> +	.probe =	ax88179_probe,
>   	.suspend =	ax88179_suspend,
>   	.resume =	ax88179_resume,
>   	.reset_resume =	ax88179_resume,
> @@ -1952,7 +1961,62 @@ static struct usb_driver ax88179_178a_driver = {
>   	.disable_hub_initiated_lpm = 1,
>   };
>   
> -module_usb_driver(ax88179_178a_driver);
> +static int ax88179_cfgselector_probe(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;
> +
> +	if (usb_set_configuration(udev, c->desc.bConfigurationValue)) {
> +		dev_err(&udev->dev, "Failed to set configuration %d\n",
> +			c->desc.bConfigurationValue);
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +static struct usb_device_driver ax88179_cfgselector_driver = {
> +	.name =		MODULENAME "-cfgselector",
> +	.probe =	ax88179_cfgselector_probe,

You want this to only run if the device is in the generic mode.

> +	.id_table =	products,
But here you check only for the product ID, not for the class code.> +	.generic_subclass = 1,> +	.supports_autosuspend = 1,

You do not. It does not matter, but it is wrong.

> +};
> +
> +static int __init ax88179_driver_init(void)
> +{
> +	int ret;
> +
> +	ret = usb_register_device_driver(&ax88179_cfgselector_driver, THIS_MODULE);
> +	if (ret)
> +		return ret;
> +	return usb_register(&ax88179_178a_driver);

Missing error handling. If you cannot register ax88179_178a_driver
you definitely do not want to keep ax88179_cfgselector_driver

> +}
> +
> +static void __exit ax88179_driver_exit(void)
> +{
> +	usb_deregister(&ax88179_178a_driver);

The window for the race

> +	usb_deregister_device_driver(&ax88179_cfgselector_driver);

Wrong order. I you remove ax88179_178a_driver before you remove
ax88179_cfgselector_driver, you'll leave a window during which
devices would be switched to a mode no driver exists for.

	Regards
		Oliver