netdev - Re: [PATCH] net: usbnet: skip info->stop() callback if suspend

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] [day] [month] [year] [list]

Message-ID: <zmxxhhtk7uqpchkwtzshq4quxts22l7a3vgijgvxk4ip7jf3pt@a4u4f45ka5x3>
Date: Fri, 5 Sep 2025 18:18:34 +0800
From: Xu Yang <xu.yang_2@....com>
To: Oliver Neukum <oneukum@...e.com>
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com, 
	kuba@...nel.org, pabeni@...hat.com, linux-usb@...r.kernel.org, 
	netdev@...r.kernel.org
Subject: Re: [PATCH] net: usbnet: skip info->stop() callback if suspend_count
 is not 0

On Thu, Sep 04, 2025 at 09:52:03AM +0200, Oliver Neukum wrote:
> 
> 
> On 9/4/25 09:26, Xu Yang wrote:
> > > >    	/* allow minidriver to stop correctly (wireless devices to turn off
> > > >    	 * radio etc) */
> > > > -	if (info->stop) {
> > > > +	if (info->stop && !dev->suspend_count) {
> > > ... for !dev->suspend_count to be false
> > The suspend_count won't go to 0 because there is no chance to call
> > usbnet_resume() if the USB device is physically disconnected from the
> > USB port during system suspend.
> 
> Sorry for the delay.
> 
> While you are correct that a physical disconnect
> will make the PM call fail, you cannot assume that
> a physical disconnect is the only reason disconnect()
> would be called.
> It would also be called if
> 
> - the module is unloaded
> - usbfs is used to disconnect the driver from the device

I understand your concern, however, it seems that only disconnect during
the PM operations then the suspend_count will be non-zero.

> 
> Hence it seems to me that using suspend_count is false.
> You need to use the return of the PM operation.

And a non-zero suspend_count itself is also a PM operation result, isn't it?

Besides, the another way we can determine whether the device has been
unattached, like following:

@@ -839,7 +839,7 @@ int usbnet_stop (struct net_device *net)
        pm = usb_autopm_get_interface(dev->intf);
        /* allow minidriver to stop correctly (wireless devices to turn off
         * radio etc) */
-       if (info->stop) {
+       if (info->stop && dev->udev->state != USB_STATE_NOTATTACHED) {
                retval = info->stop(dev);
                if (retval < 0)
                        netif_info(dev, ifdown, dev->net,

So if it's already in NOTATTACHED state, no needs to stop the device too.

Do you prefer this way?

Thanks,
Xu Yang

> 
> 	Regards
> 		Oliver
> 
>