netdev - Re: [PATCH] sky2: Fix a race between sky2_down and sky2

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 for Android: free password hash cracker in your pocket

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

Date:	Fri, 12 Jun 2009 22:16:42 +0900
From:	Mike McCormack <mikem@...g3k.org>
To:	Stephen Hemminger <shemminger@...ux-foundation.org>
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCH] sky2: Fix a race between sky2_down and sky2_poll

2009/6/12 Stephen Hemminger <shemminger@...ux-foundation.org>

> Does the following fix the problem?

I tried your patch out, but it still got a crash.

The patch below does fix the problem. Does this look better?

thanks,

Mike

-----------------------------

Subject: [PATCH] sky2: Shutdown receive path cleanly in sky2_down

If sky2_down was called while receiving a packet, receive interrupts
would still occur after the receive buffer was free'd causing a crash in
sky2_status_intr.

To avoid further receive interrupts after sky2_down:

* make sure NAPI and interrupts are fully disabled during shutdown
* call sky2_rx_stop after shutting down the receiver
* clear received packets after shutdown

Signed-off-by: Mike McCormack <mikem@...g3k.org>
---
 drivers/net/sky2.c |   21 +++++++++++++++------
 1 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index a2ff9cb..136b362 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1805,10 +1805,13 @@ static int sky2_down(struct net_device *dev)
 	/* Disable port IRQ */
 	imask = sky2_read32(hw, B0_IMSK);
 	imask &= ~portirq_msk[port];
-	sky2_write32(hw, B0_IMSK, imask);
+	sky2_write32(hw, B0_IMSK, 0);

 	synchronize_irq(hw->pdev->irq);

+	/* disable soft interrupts */
+	napi_disable(&hw->napi);
+
 	sky2_gmac_reset(hw, port);

 	/* Stop transmitter */
@@ -1822,9 +1825,6 @@ static int sky2_down(struct net_device *dev)
 	ctrl &= ~(GM_GPCR_TX_ENA | GM_GPCR_RX_ENA);
 	gma_write16(hw, port, GM_GP_CTRL, ctrl);

-	/* Make sure no packets are pending */
-	napi_synchronize(&hw->napi);
-
 	sky2_write8(hw, SK_REG(port, GPHY_CTRL), GPC_RST_SET);

 	/* Workaround shared GMAC reset */
@@ -1850,11 +1850,11 @@ static int sky2_down(struct net_device *dev)

 	sky2_write32(hw, RB_ADDR(txqaddr[port], RB_CTRL), RB_RST_SET);

-	sky2_rx_stop(sky2);
-
 	sky2_write8(hw, SK_REG(port, RX_GMF_CTRL_T), GMF_RST_SET);
 	sky2_write8(hw, SK_REG(port, TX_GMF_CTRL_T), GMF_RST_SET);

+	sky2_rx_stop(sky2);
+
 	sky2_phy_power_down(hw, port);

 	/* turn off LED's */
@@ -1878,6 +1878,15 @@ static int sky2_down(struct net_device *dev)
 	sky2->rx_ring = NULL;
 	sky2->tx_ring = NULL;

+	/* clear receive interrupt condition and IRQ */
+	hw->st_idx = sky2_read16(hw, STAT_PUT_IDX);
+
+	/* re-enable interrupts */
+	sky2_write32(hw, B0_IMSK, imask);
+
+	/* re-enable soft interrupts */
+	napi_enable(&hw->napi);
+
 	return 0;
 }

-- 
1.5.6.5
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html