2010-03-04 13:29:46

by Senthil Balasubramanian

Subject: [PATCH] ar9170: Fix illegal kfree_skb

Signed-off-by: Senthil Balasubramanian <[email protected]>
---
drivers/net/wireless/ath/ar9170/main.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ar9170/main.c b/drivers/net/wireless/ath/ar9170/main.c
index f4650fc..d47450a 100644
--- a/drivers/net/wireless/ath/ar9170/main.c
+++ b/drivers/net/wireless/ath/ar9170/main.c
@@ -2517,7 +2517,7 @@ void *ar9170_alloc(size_t priv_size)

skb = __dev_alloc_skb(AR9170_MAX_RX_BUFFER_SIZE, GFP_KERNEL);
if (!skb)
- goto err_nomem;
+ goto err_out;

hw = ieee80211_alloc_hw(priv_size, &ar9170_ops);
if (!hw)
@@ -2574,6 +2574,7 @@ void *ar9170_alloc(size_t priv_size)

err_nomem:
kfree_skb(skb);
+err_out:
return ERR_PTR(-ENOMEM);
}

--
1.7.0.1

2010-03-08 15:22:42

by Christian Lamparter

Subject: Re: [PATCH] ar9170: Fix illegal kfree_skb

On Thursday 04 March 2010 14:28:33 Senthil Balasubramanian wrote:
> Signed-off-by: Senthil Balasubramanian <[email protected]>
> ---
> drivers/net/wireless/ath/ar9170/main.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ar9170/main.c b/drivers/net/wireless/ath/ar9170/main.c
> index f4650fc..d47450a 100644
> --- a/drivers/net/wireless/ath/ar9170/main.c
> +++ b/drivers/net/wireless/ath/ar9170/main.c
> @@ -2517,7 +2517,7 @@ void *ar9170_alloc(size_t priv_size)
>
> skb = __dev_alloc_skb(AR9170_MAX_RX_BUFFER_SIZE, GFP_KERNEL);
> if (!skb)
> - goto err_nomem;
> + goto err_out;
>
> hw = ieee80211_alloc_hw(priv_size, &ar9170_ops);
> if (!hw)
> @@ -2574,6 +2574,7 @@ void *ar9170_alloc(size_t priv_size)
>
> err_nomem:
> kfree_skb(skb);
> +err_out:
> return ERR_PTR(-ENOMEM);
> }

err, wait. It's perfectly legal to call kfree_skb(NULL). This is
also true for most other kfree* functions.

But, if you can't stand this freeing NULL business (e.g.: prop. mem checker?),
then why not got the direct path and replace "goto err_nomem" with
"return ERR_PTR(-ENOMEM)"?

Regards,
Chr