[PATCH] ath9k: Fix kernel panic during rmmod ath9k

[PATCH] ath9k: Fix kernel panic during rmmod ath9k

[Vasanthakumar Thiagarajan]

This panic was introduced in ar9003 family chipsets
by the following commit

	Author: Felix Fietkau <nbd@openwrt.org>
	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 <vasanth@atheros.com>
---
 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);
 	} else {
 		ath_tx_complete(sc, skb, bf->aphy, tx_flags);
 		ath_debug_stat_tx(sc, txq, bf, ts);
-- 
1.7.0.4

Re: [PATCH] ath9k: Fix kernel panic during rmmod ath9k

[Vasanthakumar Thiagarajan]

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 <nbd@openwrt.org>
> 	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 <vasanth@atheros.com>
> ---
>  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