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] [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



Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ