Return-path: Received: from mail.atheros.com ([12.36.123.2]:15219 "EHLO mail.atheros.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753449Ab0FUIcj (ORCPT ); Mon, 21 Jun 2010 04:32:39 -0400 Received: from mail.atheros.com ([10.10.20.108]) by sidewinder.atheros.com for ; Mon, 21 Jun 2010 01:32:39 -0700 Date: Mon, 21 Jun 2010 14:02:33 +0530 From: Vasanthakumar Thiagarajan To: "linville@tuxdriver.com" CC: "linux-wireless@vger.kernel.org" Subject: Re: [PATCH] ath9k: Fix kernel panic during rmmod ath9k Message-ID: <20100621083233.GB9292@vasanth-laptop> References: <1277105768-1980-1-git-send-email-vasanth@atheros.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" In-Reply-To: <1277105768-1980-1-git-send-email-vasanth@atheros.com> Sender: linux-wireless-owner@vger.kernel.org List-ID: On Mon, Jun 21, 2010 at 01:06:08PM +0530, Vasanthakumar Thiagarajan wrote: > This panic was introduced in ar9003 family chipsets > by the following commit > > Author: Felix Fietkau > Date: Sat Jun 12 00:34:01 2010 -0400 > > ath9k: implement PA predistortion support > > Above patch does kfree_skb on a PA predistortion frame > in ath_paprd_calibrate(). This is fine for the cases > where this frame could not be queued onto sw/hw queues > or the tx of this frame is completed. But freeing this > frame upon a failed completion event will result in > dereferencing a freed memory in ath_tx_complete_buf() > while draining pending tx frames. > > This patch fixes this issue by moving kfree_skb to > ath_tx_complete_buf() once the frame is successfully > queued. > > Signed-off-by: Vasanthakumar Thiagarajan > --- > drivers/net/wireless/ath/ath9k/main.c | 5 +++-- > drivers/net/wireless/ath/ath9k/xmit.c | 1 + > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c > index c8de50f..37933d3 100644 > --- a/drivers/net/wireless/ath/ath9k/main.c > +++ b/drivers/net/wireless/ath/ath9k/main.c > @@ -306,8 +306,10 @@ void ath_paprd_calibrate(struct work_struct *work) > init_completion(&sc->paprd_complete); > ar9003_paprd_setup_gain_table(ah, chain); > txctl.paprd = BIT(chain); > - if (ath_tx_start(hw, skb, &txctl) != 0) > + if (ath_tx_start(hw, skb, &txctl) != 0) { > + kfree_skb(skb); > break; > + } > > time_left = wait_for_completion_timeout(&sc->paprd_complete, > 100); > @@ -327,7 +329,6 @@ void ath_paprd_calibrate(struct work_struct *work) > > chain_ok = 1; > } > - kfree_skb(skb); > > if (chain_ok) { > ah->curchan->paprd_done = true; > diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c > index 8c7c615..197e898 100644 > --- a/drivers/net/wireless/ath/ath9k/xmit.c > +++ b/drivers/net/wireless/ath/ath9k/xmit.c > @@ -1946,6 +1946,7 @@ static void ath_tx_complete_buf(struct ath_softc *sc, struct ath_buf *bf, > if (bf->bf_state.bfs_paprd) { > sc->paprd_txok = txok; > complete(&sc->paprd_complete); > + dev_kfree_skb_any(skb); Please ignore this patch, this is buggy. Vasanth