The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH net] net: au1000: move free_irq out of the close-time spinlocked section
@ 2026-06-18 14:19 Runyu Xiao
  0 siblings, 0 replies; 4+ messages in thread
From: Runyu Xiao @ 2026-06-18 14:19 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: netdev, linux-kernel, jianhao.xu, runyu.xiao, stable

au1000_close() calls free_irq() while aup->lock is still held with
spin_lock_irqsave().  free_irq() can sleep because it takes the IRQ
descriptor request mutex, so it does not belong inside the close-time
spinlocked section.

This issue was found by our static analysis tool and then manually
reviewed against the current tree.

The grounded PoC kept the ndo_stop carrier and the au1000_close() ->
free_irq(dev->irq, dev) path while the driver lock was held.  Lockdep
reported:

  BUG: sleeping function called from invalid context
  1 lock held by exploit/192:
   #0: (&aup->lock){....}-{2:2}, at: au1000_close+0x23/0x83 [vuln_msv]
  [ BUG: Invalid wait context ]
  exploit/192 is trying to lock:
  (&desc->request_mutex){+.+.}-{3:3}, at: free_irq+0x63/0x360
  free_irq+0x63/0x360
  au1000_close+0x65/0x83 [vuln_msv]

Drop aup->lock before freeing the IRQ.  The protected close-time work
still stops the device and queue before IRQ teardown, but the sleepable
IRQ core path now runs outside the spinlocked section.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/net/ethernet/amd/au1000_eth.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/au1000_eth.c b/drivers/net/ethernet/amd/au1000_eth.c
index 9d35ac348ebe..5a04056e38fa 100644
--- a/drivers/net/ethernet/amd/au1000_eth.c
+++ b/drivers/net/ethernet/amd/au1000_eth.c
@@ -943,9 +943,10 @@ static int au1000_close(struct net_device *dev)
 	/* stop the device */
 	netif_stop_queue(dev);
 
+	spin_unlock_irqrestore(&aup->lock, flags);
+
 	/* disable the interrupt */
 	free_irq(dev->irq, dev);
-	spin_unlock_irqrestore(&aup->lock, flags);
 
 	return 0;
 }
-- 
2.34.1


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

* [PATCH net] net: au1000: move free_irq out of the close-time spinlocked section
@ 2026-06-19 15:18 Runyu Xiao
  2026-06-22 12:47 ` Simon Horman
  2026-06-22 15:56 ` Jakub Kicinski
  0 siblings, 2 replies; 4+ messages in thread
From: Runyu Xiao @ 2026-06-19 15:18 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: netdev, linux-kernel, Runyu Xiao, stable

au1000_close() calls free_irq() while aup->lock is still held with
spin_lock_irqsave(). free_irq() can sleep because it takes the IRQ
descriptor request mutex, so it does not belong inside the close-time
spinlocked section.

This was found by our static analysis tool and then confirmed by manual
review of the in-tree au1000_close() .ndo_stop path. The reviewed path
keeps aup->lock held across the MAC reset, queue stop and
free_irq(dev->irq, dev).

A directed runtime validation kept that ndo_stop carrier and the same
free_irq(dev->irq, dev) operation under the driver lock. Lockdep reported
"BUG: sleeping function called from invalid context" and "Invalid wait
context" while free_irq() was taking desc->request_mutex, with
au1000_close() and free_irq() on the stack.

Drop aup->lock before freeing the IRQ. The protected close-time work still
stops the device and queue before IRQ teardown, but the sleepable IRQ core
path now runs outside the spinlocked section.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 drivers/net/ethernet/amd/au1000_eth.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/au1000_eth.c b/drivers/net/ethernet/amd/au1000_eth.c
index 9d35ac348ebe..5a04056e38fa 100644
--- a/drivers/net/ethernet/amd/au1000_eth.c
+++ b/drivers/net/ethernet/amd/au1000_eth.c
@@ -943,9 +943,10 @@ static int au1000_close(struct net_device *dev)
 	/* stop the device */
 	netif_stop_queue(dev);
 
+	spin_unlock_irqrestore(&aup->lock, flags);
+
 	/* disable the interrupt */
 	free_irq(dev->irq, dev);
-	spin_unlock_irqrestore(&aup->lock, flags);
 
 	return 0;
 }
-- 
2.34.1


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

* Re: [PATCH net] net: au1000: move free_irq out of the close-time spinlocked section
  2026-06-19 15:18 [PATCH net] net: au1000: move free_irq out of the close-time spinlocked section Runyu Xiao
@ 2026-06-22 12:47 ` Simon Horman
  2026-06-22 15:56 ` Jakub Kicinski
  1 sibling, 0 replies; 4+ messages in thread
From: Simon Horman @ 2026-06-22 12:47 UTC (permalink / raw)
  To: Runyu Xiao
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel, stable

On Fri, Jun 19, 2026 at 11:18:16PM +0800, Runyu Xiao wrote:
> au1000_close() calls free_irq() while aup->lock is still held with
> spin_lock_irqsave(). free_irq() can sleep because it takes the IRQ
> descriptor request mutex, so it does not belong inside the close-time
> spinlocked section.
> 
> This was found by our static analysis tool and then confirmed by manual
> review of the in-tree au1000_close() .ndo_stop path. The reviewed path
> keeps aup->lock held across the MAC reset, queue stop and
> free_irq(dev->irq, dev).
> 
> A directed runtime validation kept that ndo_stop carrier and the same
> free_irq(dev->irq, dev) operation under the driver lock. Lockdep reported
> "BUG: sleeping function called from invalid context" and "Invalid wait
> context" while free_irq() was taking desc->request_mutex, with
> au1000_close() and free_irq() on the stack.
> 
> Drop aup->lock before freeing the IRQ. The protected close-time work still
> stops the device and queue before IRQ teardown, but the sleepable IRQ core
> path now runs outside the spinlocked section.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>

Reviewed-by: Simon Horman <horms@kernel.org>

FTR, I notice that there is an AI-generated review of this patch on
sashiko.dev. However, I don't think that the issues raised there should
block progress of this patch.

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

* Re: [PATCH net] net: au1000: move free_irq out of the close-time spinlocked section
  2026-06-19 15:18 [PATCH net] net: au1000: move free_irq out of the close-time spinlocked section Runyu Xiao
  2026-06-22 12:47 ` Simon Horman
@ 2026-06-22 15:56 ` Jakub Kicinski
  1 sibling, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2026-06-22 15:56 UTC (permalink / raw)
  To: Runyu Xiao
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni, netdev,
	linux-kernel, stable

On Fri, 19 Jun 2026 23:18:16 +0800 Runyu Xiao wrote:
> au1000_close() calls free_irq() while aup->lock is still held with
> spin_lock_irqsave(). free_irq() can sleep because it takes the IRQ
> descriptor request mutex, so it does not belong inside the close-time
> spinlocked section.
> 
> This was found by our static analysis tool and then confirmed by manual
> review of the in-tree au1000_close() .ndo_stop path. The reviewed path
> keeps aup->lock held across the MAC reset, queue stop and
> free_irq(dev->irq, dev).
> 
> A directed runtime validation kept that ndo_stop carrier and the same
> free_irq(dev->irq, dev) operation under the driver lock. Lockdep reported
> "BUG: sleeping function called from invalid context" and "Invalid wait
> context" while free_irq() was taking desc->request_mutex, with
> au1000_close() and free_irq() on the stack.
> 
> Drop aup->lock before freeing the IRQ. The protected close-time work still
> stops the device and queue before IRQ teardown, but the sleepable IRQ core
> path now runs outside the spinlocked section.

Do you really think that this bug matters if nobody fixed it on
a 20+ year old platform?

Please do not point your AI scanning tools at old code!
The patch is valid I guess but we have heaps of bugs like this
that _nobody care about in practice_! You're wasting everyone's
time.

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

end of thread, other threads:[~2026-06-22 15:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-19 15:18 [PATCH net] net: au1000: move free_irq out of the close-time spinlocked section Runyu Xiao
2026-06-22 12:47 ` Simon Horman
2026-06-22 15:56 ` Jakub Kicinski
  -- strict thread matches above, loose matches on Subject: below --
2026-06-18 14:19 Runyu Xiao

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