linux-kernel - Re: net: usb: r8152: resume-reset deadlock

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: <fnvkmao3cfavr6gijb53h77wtk2nixjjhczx5wxnu7baypbdmg@elrzevwou7dd>
Date: Wed, 28 Jan 2026 15:12:46 +0900
From: Sergey Senozhatsky <senozhatsky@...omium.org>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, 
	"David S. Miller" <davem@...emloft.net>, Realtek linux nic maintainers <nic_swsd@...ltek.com>, 
	Takashi Iwai <tiwai@...e.de>, linux-usb@...r.kernel.org, netdev@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Sergey Senozhatsky <senozhatsky@...omium.org>
Subject: Re: net: usb: r8152: resume-reset deadlock

On (26/01/28 14:58), Sergey Senozhatsky wrote:
> Something like this?
> 
> ---
> 
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 30f937527cd2..0a570b8ac7dd 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -8542,14 +8542,9 @@ static int rtl8152_system_resume(struct r8152 *tp)
>  	 * reset. This is important because the usb_lock_device_for_reset()
>  	 * that happens as a result of usb_queue_reset_device() will silently
>  	 * fail if the device was suspended or if too much time passed.
> -	 *
> -	 * NOTE: The device is locked here so we can directly do the reset.
> -	 * We don't need usb_lock_device_for_reset() because that's just a
> -	 * wrapper over device_lock() and device_resume() (which calls us)
> -	 * does that for us.
>  	 */
>  	if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
> -		usb_reset_device(tp->udev);
> +		return -ENODEV;
>  
>  	return 0;
>  }
> @@ -8674,6 +8669,14 @@ static int rtl8152_resume(struct usb_interface *intf)
>  
>  	mutex_unlock(&tp->control);
>  
> +	/*
> +	 * This only happens for !SELECTIVE_SUSPEND and RTL8152_INACCESSIBLE,
> +	 * drop tp->control, because reset path can re-acquire it (e.g. for
> +	 * MAC address/policy update).
> +	 */
> +	if (ret == -ENODEV)
> +		ret = usb_reset_device(tp->udev);
> +
>  	return ret;
>  }

Sorry, sent it too early.  Something like instead perhaps:

---

diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index 30f937527cd2..c9406b89bb12 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -8538,20 +8538,18 @@ static int rtl8152_system_resume(struct r8152 *tp)
 		usb_submit_urb(tp->intr_urb, GFP_NOIO);
 	}
 
+	if (!test_bit(RTL8152_INACCESSIBLE, &tp->flags))
+		return 0;
+
 	/* If the device is RTL8152_INACCESSIBLE here then we should do a
 	 * reset. This is important because the usb_lock_device_for_reset()
 	 * that happens as a result of usb_queue_reset_device() will silently
 	 * fail if the device was suspended or if too much time passed.
-	 *
-	 * NOTE: The device is locked here so we can directly do the reset.
-	 * We don't need usb_lock_device_for_reset() because that's just a
-	 * wrapper over device_lock() and device_resume() (which calls us)
-	 * does that for us.
 	 */
-	if (test_bit(RTL8152_INACCESSIBLE, &tp->flags))
-		usb_reset_device(tp->udev);
+	if (test_bit(SELECTIVE_SUSPEND, &tp->flags))
+		return usb_reset_device(tp->udev);
 
-	return 0;
+	return -ENODEV;
 }
 
 static int rtl8152_runtime_suspend(struct r8152 *tp)
@@ -8674,6 +8672,14 @@ static int rtl8152_resume(struct usb_interface *intf)
 
 	mutex_unlock(&tp->control);
 
+	/*
+	 * This only happens for !SELECTIVE_SUSPEND and RTL8152_INACCESSIBLE,
+	 * drop tp->control, becuase reset can re-acquire it (e.g. from
+	 * max address reset)
+	 */
+	if (ret == -ENODEV)
+		ret = usb_reset_device(tp->udev);
+
 	return ret;
 }