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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20251013075937.4de02dfe.michal.pecio@gmail.com>
Date: Mon, 13 Oct 2025 07:59:37 +0200
From: Michal Pecio <michal.pecio@...il.com>
To: Oliver Neukum <oneukum@...e.com>
Cc: yicongsrfy@....com, andrew+netdev@...n.ch, 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

On Tue, 30 Sep 2025 10:57:05 +0200, Oliver Neukum wrote:
> > +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.

Hmm, what about registration?

I added msleep(1000) and simulated usb_register() error, then
cfgselector binds to the device and switches configuration before
the interface driver is available. But the module fails to load
(I fixed this) and device is left with no driver whatsoever.

Moreover, according to c67cc4315a8e, config switch is irreversible
since the device reconnects with only the vendor config available.
I can't test it because my device doesn't have a CDC config at all.

There is a gotcha. I tried to test in a realistic scenario: device
hotplug, module not loaded yet. I found that udev apparently retries
loading the module, so this state would be fixed unless the module
init error is persistent. Still, better not to rely on this?

Would it make sense to swap registration order?

Regards,
Michal