public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net: usb: aqc111: Do not perform PM inside runtime suspend callback
@ 2026-03-04 15:57 Nikola Z. Ivanov
  2026-03-07  0:43 ` Jakub Kicinski
  0 siblings, 1 reply; 3+ messages in thread
From: Nikola Z. Ivanov @ 2026-03-04 15:57 UTC (permalink / raw)
  To: andrew+netdev, davem, edumazet, kuba, pabeni, enelsonmoore, kees,
	oneukum, n.zhandarovich
  Cc: linux-usb, netdev, linux-kernel, Nikola Z. Ivanov,
	syzbot+48dc1e8dfc92faf1124c

syzbot reports "task hung in rpm_resume"

This is caused by aqc111_suspend calling
the PM variant of its write_cmd routine.

The simplified call trace looks like this:

rpm_suspend()
  usb_suspend_both() - here udev->dev.power.runtime_status == RPM_SUSPENDING
    aqc111_suspend() - called for the usb device interface
      aqc111_write32_cmd()
        usb_autopm_get_interface()
          pm_runtime_resume_and_get()
            rpm_resume() - here we call rpm_resume() on our parent
              rpm_resume() - Here we wait for a status change that will never happen.

At this point we block another task which holds
rtnl_lock and locks up the whole networking stack.

Fix this by replacing the write_cmd calls with their _nopm variants
in the case where we are inside a runtime suspend call.

Reported-by: syzbot+48dc1e8dfc92faf1124c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=48dc1e8dfc92faf1124c
Fixes: e58ba4544c77 ("net: usb: aqc111: Add support for wake on LAN by MAGIC packet")
Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
---
This patch is untested!
I do not have access to a real device to test it,
testing on real hardware would be appreciated,
if anyone has a device laying around.

I have found no reason for the PM variants to be
used in the ->suspend callback when it comes
to the device driver.

The PM docs suggest that PM should not be done
during runtime suspend, but I cannot find a
definitive answer for system suspend, hence the
conditional if(PMSG_IS_AUTO(message))

 drivers/net/usb/aqc111.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
index cbffa9ae1bb6..2f0d66c7ade0 100644
--- a/drivers/net/usb/aqc111.c
+++ b/drivers/net/usb/aqc111.c
@@ -1395,14 +1395,27 @@ static int aqc111_suspend(struct usb_interface *intf, pm_message_t message)
 		aqc111_write16_cmd_nopm(dev, AQ_ACCESS_MAC,
 					SFR_MEDIUM_STATUS_MODE, 2, &reg16);
 
-		aqc111_write_cmd(dev, AQ_WOL_CFG, 0, 0,
-				 WOL_CFG_SIZE, &wol_cfg);
-		aqc111_write32_cmd(dev, AQ_PHY_OPS, 0, 0,
-				   &aqc111_data->phy_cfg);
+		if (PMSG_IS_AUTO(message)) {
+			aqc111_write_cmd_nopm(dev, AQ_WOL_CFG, 0, 0,
+					      WOL_CFG_SIZE, &wol_cfg);
+			aqc111_write32_cmd_nopm(dev, AQ_PHY_OPS, 0, 0,
+						&aqc111_data->phy_cfg);
+		} else {
+			aqc111_write_cmd(dev, AQ_WOL_CFG, 0, 0,
+					 WOL_CFG_SIZE, &wol_cfg);
+			aqc111_write32_cmd(dev, AQ_PHY_OPS, 0, 0,
+					   &aqc111_data->phy_cfg);
+		}
 	} else {
 		aqc111_data->phy_cfg |= AQ_LOW_POWER;
-		aqc111_write32_cmd(dev, AQ_PHY_OPS, 0, 0,
-				   &aqc111_data->phy_cfg);
+
+		if (PMSG_IS_AUTO(message)) {
+			aqc111_write32_cmd_nopm(dev, AQ_PHY_OPS, 0, 0,
+						&aqc111_data->phy_cfg);
+		} else {
+			aqc111_write32_cmd(dev, AQ_PHY_OPS, 0, 0,
+					   &aqc111_data->phy_cfg);
+		}
 
 		/* Disable RX path */
 		aqc111_read16_cmd_nopm(dev, AQ_ACCESS_MAC,
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: usb: aqc111: Do not perform PM inside runtime suspend callback
  2026-03-04 15:57 [PATCH net] net: usb: aqc111: Do not perform PM inside runtime suspend callback Nikola Z. Ivanov
@ 2026-03-07  0:43 ` Jakub Kicinski
  2026-03-11  0:20   ` Jakub Kicinski
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Kicinski @ 2026-03-07  0:43 UTC (permalink / raw)
  To: Nikola Z. Ivanov, Dmitry Bezrukov, Igor Russkikh
  Cc: andrew+netdev, davem, edumazet, pabeni, enelsonmoore, kees,
	oneukum, n.zhandarovich, linux-usb, netdev, linux-kernel,
	syzbot+48dc1e8dfc92faf1124c

On Wed,  4 Mar 2026 17:57:34 +0200 Nikola Z. Ivanov wrote:
> syzbot reports "task hung in rpm_resume"
> 
> This is caused by aqc111_suspend calling
> the PM variant of its write_cmd routine.
> 
> The simplified call trace looks like this:
> 
> rpm_suspend()
>   usb_suspend_both() - here udev->dev.power.runtime_status == RPM_SUSPENDING
>     aqc111_suspend() - called for the usb device interface
>       aqc111_write32_cmd()
>         usb_autopm_get_interface()
>           pm_runtime_resume_and_get()
>             rpm_resume() - here we call rpm_resume() on our parent
>               rpm_resume() - Here we wait for a status change that will never happen.
> 
> At this point we block another task which holds
> rtnl_lock and locks up the whole networking stack.
> 
> Fix this by replacing the write_cmd calls with their _nopm variants
> in the case where we are inside a runtime suspend call.
> 
> Reported-by: syzbot+48dc1e8dfc92faf1124c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=48dc1e8dfc92faf1124c
> Fixes: e58ba4544c77 ("net: usb: aqc111: Add support for wake on LAN by MAGIC packet")
> Signed-off-by: Nikola Z. Ivanov <zlatistiv@gmail.com>
> ---
> This patch is untested!
> I do not have access to a real device to test it,
> testing on real hardware would be appreciated,
> if anyone has a device laying around.
> 
> I have found no reason for the PM variants to be
> used in the ->suspend callback when it comes
> to the device driver.
> 
> The PM docs suggest that PM should not be done
> during runtime suspend, but I cannot find a
> definitive answer for system suspend, hence the
> conditional if(PMSG_IS_AUTO(message))

Dmitiry, Igor, could you possibly shed some light?
Can we simply switch to the _nopm() helpers instead?

> diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
> index cbffa9ae1bb6..2f0d66c7ade0 100644
> --- a/drivers/net/usb/aqc111.c
> +++ b/drivers/net/usb/aqc111.c
> @@ -1395,14 +1395,27 @@ static int aqc111_suspend(struct usb_interface *intf, pm_message_t message)
>  		aqc111_write16_cmd_nopm(dev, AQ_ACCESS_MAC,
>  					SFR_MEDIUM_STATUS_MODE, 2, &reg16);
>  
> -		aqc111_write_cmd(dev, AQ_WOL_CFG, 0, 0,
> -				 WOL_CFG_SIZE, &wol_cfg);
> -		aqc111_write32_cmd(dev, AQ_PHY_OPS, 0, 0,
> -				   &aqc111_data->phy_cfg);
> +		if (PMSG_IS_AUTO(message)) {
> +			aqc111_write_cmd_nopm(dev, AQ_WOL_CFG, 0, 0,
> +					      WOL_CFG_SIZE, &wol_cfg);
> +			aqc111_write32_cmd_nopm(dev, AQ_PHY_OPS, 0, 0,
> +						&aqc111_data->phy_cfg);
> +		} else {
> +			aqc111_write_cmd(dev, AQ_WOL_CFG, 0, 0,
> +					 WOL_CFG_SIZE, &wol_cfg);
> +			aqc111_write32_cmd(dev, AQ_PHY_OPS, 0, 0,
> +					   &aqc111_data->phy_cfg);
> +		}
>  	} else {
>  		aqc111_data->phy_cfg |= AQ_LOW_POWER;
> -		aqc111_write32_cmd(dev, AQ_PHY_OPS, 0, 0,
> -				   &aqc111_data->phy_cfg);
> +
> +		if (PMSG_IS_AUTO(message)) {
> +			aqc111_write32_cmd_nopm(dev, AQ_PHY_OPS, 0, 0,
> +						&aqc111_data->phy_cfg);
> +		} else {
> +			aqc111_write32_cmd(dev, AQ_PHY_OPS, 0, 0,
> +					   &aqc111_data->phy_cfg);
> +		}
>  
>  		/* Disable RX path */
>  		aqc111_read16_cmd_nopm(dev, AQ_ACCESS_MAC,


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] net: usb: aqc111: Do not perform PM inside runtime suspend callback
  2026-03-07  0:43 ` Jakub Kicinski
@ 2026-03-11  0:20   ` Jakub Kicinski
  0 siblings, 0 replies; 3+ messages in thread
From: Jakub Kicinski @ 2026-03-11  0:20 UTC (permalink / raw)
  To: Nikola Z. Ivanov, Dmitry Bezrukov, Igor Russkikh
  Cc: andrew+netdev, davem, edumazet, pabeni, enelsonmoore, kees,
	oneukum, n.zhandarovich, linux-usb, netdev, linux-kernel,
	syzbot+48dc1e8dfc92faf1124c

On Fri, 6 Mar 2026 16:43:51 -0800 Jakub Kicinski wrote:
> > The PM docs suggest that PM should not be done
> > during runtime suspend, but I cannot find a
> > definitive answer for system suspend, hence the
> > conditional if(PMSG_IS_AUTO(message))  
> 
> Dmitiry, Igor, could you possibly shed some light?
> Can we simply switch to the _nopm() helpers instead?

Hi Nikola, seems like we may not get an answer..
Let's switch to _nopm() and let people complain if this is wrong.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-11  0:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-04 15:57 [PATCH net] net: usb: aqc111: Do not perform PM inside runtime suspend callback Nikola Z. Ivanov
2026-03-07  0:43 ` Jakub Kicinski
2026-03-11  0:20   ` Jakub Kicinski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox