2010-01-15 13:51:09

by Ming Lei

[permalink] [raw]
Subject: [PATCH] mac80211: fix memory leak in ieee80211_send_probe_req

From: Ming Lei <[email protected]>

This patch fixes memory leak in ieee80211_send_probe_req, which
is introduced in 7c12ce8b854df346388ea56d684784e3484012cf:

mac80211: use Probe Request template when sending a direct scan

The patch is against the latest wireless-test tree.

Signed-off-by: Ming Lei <[email protected]>
---
net/mac80211/util.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index e278f97..ca170b4 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -1028,6 +1028,7 @@ void ieee80211_send_probe_req(struct ieee80211_sub_if_data *sdata, u8 *dst,

IEEE80211_SKB_CB(skb)->flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT;
ieee80211_tx_skb(sdata, skb);
+ kfree(buf);
}

u32 ieee80211_sta_get_rates(struct ieee80211_local *local,
--
1.6.2.5



2010-01-15 14:19:20

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix memory leak in ieee80211_send_probe_req

"ext [email protected]" <[email protected]> writes:

> From: Ming Lei <[email protected]>
>
> This patch fixes memory leak in ieee80211_send_probe_req, which
> is introduced in 7c12ce8b854df346388ea56d684784e3484012cf:
>
> mac80211: use Probe Request template when sending a direct scan


Sorry, I introduced this bug. Thank you finding this.

> The patch is against the latest wireless-test tree.

You should add comments like this after the "---" line.

> @@ -1028,6 +1028,7 @@ void ieee80211_send_probe_req(struct ieee80211_sub_if_data *sdata, u8 *dst,
>
> IEEE80211_SKB_CB(skb)->flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT;
> ieee80211_tx_skb(sdata, skb);
> + kfree(buf);

Better to use dev_kfree_skb() here.

Can you resend, please?

--
Kalle Valo

2010-01-15 14:53:17

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix memory leak in ieee80211_send_probe_req

Ming Lei <[email protected]> writes:

>>> ? ? ? IEEE80211_SKB_CB(skb)->flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT;
>>> ? ? ? ieee80211_tx_skb(sdata, skb);
>>> + ? ? kfree(buf);
>>
>> Better to use dev_kfree_skb() here.
>
> Why is dev_kfree_skb better?
> It is 'buf' allocated in ieee80211_send_probe_req to be freed, not skb.

Yes, I misread your patch first. Please forget that comment :)

--
Kalle Valo

2010-01-15 14:36:52

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix memory leak in ieee80211_send_probe_req

Kalle Valo <[email protected]> writes:

> "ext [email protected]" <[email protected]> writes:
>
>> From: Ming Lei <[email protected]>
>>
>> This patch fixes memory leak in ieee80211_send_probe_req, which
>> is introduced in 7c12ce8b854df346388ea56d684784e3484012cf:
>>
>> mac80211: use Probe Request template when sending a direct scan
>

[...]

>> @@ -1028,6 +1028,7 @@ void ieee80211_send_probe_req(struct ieee80211_sub_if_data *sdata, u8 *dst,
>>
>> IEEE80211_SKB_CB(skb)->flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT;
>> ieee80211_tx_skb(sdata, skb);
>> + kfree(buf);
>
> Better to use dev_kfree_skb() here.

I don't know what I was thinking when I sent the previous email :)

After second look, this looks good.

Reviewed-by: Kalle Valo <[email protected]>

--
Kalle Valo

2010-01-15 14:36:45

by Ming Lei

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix memory leak in ieee80211_send_probe_req

2010/1/15 Kalle Valo <[email protected]>:
> "ext [email protected]" <[email protected]> writes:
>
>> From: Ming Lei <[email protected]>
>>
>> This patch fixes memory leak in ieee80211_send_probe_req, which
>> is introduced in 7c12ce8b854df346388ea56d684784e3484012cf:
>>
>> ? ? ? mac80211: use Probe Request template when sending a direct scan
>
>
> Sorry, I introduced this bug. Thank you finding this.
>
>> The patch is against the latest wireless-test tree.
>
> You should add comments like this after the "---" line.
>
>> @@ -1028,6 +1028,7 @@ void ieee80211_send_probe_req(struct ieee80211_sub_if_data *sdata, u8 *dst,
>>
>> ? ? ? IEEE80211_SKB_CB(skb)->flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT;
>> ? ? ? ieee80211_tx_skb(sdata, skb);
>> + ? ? kfree(buf);
>
> Better to use dev_kfree_skb() here.

Why is dev_kfree_skb better?
It is 'buf' allocated in ieee80211_send_probe_req to be freed, not skb.

>
> Can you resend, please?

OK.

--
Lei Ming