Re: [TIP] BUG kmalloc-4096: Poison overwritten (ath5k_rx_skb_alloc)



On Thu, Feb 26, 2009 at 05:03:39PM +0000, Sitsofe Wheeler wrote:

Note that I was able to reproduce it again : )

By the way, here's the theoretical race I was alluding to. ath5k_reset
happens pretty frequently when scanning, and it's possible that the rx
tasklet is run on another cpu after interrupts are turned off, but
it's a small window and I couldn't trigger it with any mdelays.

[PATCH] ath5k: manipulate rxlink and descriptor address under rxbuf lock

Grabbing an ath5k_buf then dropping the lock is racy because the
referenced descriptor can be obtained in another thread and released
before the buffer is handed to the hardware. Likewise, manipulating
sc->rxlink without the lock can lead to having multiple self-linked
hardware descriptors.

Signed-off-by: Bob Copeland <me@xxxxxxxxxxxxxxx>
---
drivers/net/wireless/ath5k/base.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 8d4b11c..0d3adb5 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1586,9 +1586,8 @@ ath5k_rx_start(struct ath5k_softc *sc)
ATH5K_DBG(sc, ATH5K_DEBUG_RESET, "cachelsz %u rxbufsize %u\n",
sc->cachelsz, sc->rxbufsize);

- sc->rxlink = NULL;
-
spin_lock_bh(&sc->rxbuflock);
+ sc->rxlink = NULL;
list_for_each_entry(bf, &sc->rxbuf, list) {
ret = ath5k_rxbuf_setup(sc, bf);
if (ret != 0) {
@@ -1597,9 +1596,9 @@ ath5k_rx_start(struct ath5k_softc *sc)
}
}
bf = list_first_entry(&sc->rxbuf, struct ath5k_buf, list);
+ ath5k_hw_set_rxdp(ah, bf->daddr);
spin_unlock_bh(&sc->rxbuflock);

- ath5k_hw_set_rxdp(ah, bf->daddr);
ath5k_hw_start_rx_dma(ah); /* enable recv descriptors */
ath5k_mode_setup(sc); /* set filters, etc. */
ath5k_hw_start_rx_pcu(ah); /* re-enable PCU/DMA engine */
--
1.6.0.6



--
Bob Copeland %% www.bobcopeland.com

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



Relevant Pages

  • Re: LOR with filedesc structure and Giant
    ... We hold the file descriptor lock for the duration ... > because I need to grab a vnode lock to authorize polling the vnode using ... The easiest fix is to take a reference on the descriptors in the ...
    (freebsd-current)
  • Re: Fine grain select locking.
    ... Briefly, linux uses RCU to protect the list, which is ... other hand uses the actual file lock to protect the descriptor slot. ...
    (freebsd-arch)
  • Re: Fine grain select locking.
    ... other hand uses the actual file lock to protect the descriptor slot. ... If the ref count and flags were done with atomics the main consumer of FILE_LOCK would actually be the unix domain socket garbage collection code. ...
    (freebsd-arch)
  • Re: Multiple IMAP connections to same folder
    ... it has a shared lock open on the mailbox on the server. ... So it opens another descriptor append the message to the destination, keeping the first descriptor for the open mailbox. ...
    (comp.mail.imap)