[PATCH 1/2] mac80211: fix unbalanced locking in ieee80211_get_buffered_bc

[PATCH 1/2] mac80211: fix unbalanced locking in ieee80211_get_buffered_bc

[Tomas Winkler]

This patch fixes unbalanced locking in ieee80211_get_buffered_bc

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 net/mac80211/tx.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 16af308..8a5b24d 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2008,8 +2008,10 @@ ieee80211_get_buffered_bc(struct ieee80211_hw *hw,
 			break;
 	}
 
-	if (WARN_ON(tx.skb != skb))
-		return NULL;
+	if (WARN_ON(tx.skb != skb)) {
+		skb = NULL;
+		goto out;
+	}
 
 	if (res == TX_DROP) {
 		I802_DEBUG_INC(local->tx_handlers_drop);
-- 
1.5.4.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

[PATCH 2/2] mac80211: removing shadowed sband

[Tomas Winkler]

This patch removes doubly defined sband variable

Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
 net/mac80211/tx.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 8a5b24d..f857d66 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -612,7 +612,6 @@ ieee80211_tx_h_misc(struct ieee80211_tx_data *tx)
 
 	if ((info->flags & IEEE80211_TX_CTL_USE_RTS_CTS) ||
 	    (info->flags & IEEE80211_TX_CTL_USE_CTS_PROTECT)) {
-		struct ieee80211_supported_band *sband;
 		struct ieee80211_rate *rate;
 		s8 baserate = -1;
 		int idx;
-- 
1.5.4.1

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

Re: [PATCH 2/2] mac80211: removing shadowed sband

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 525 bytes --]

On Mon, 2008-06-02 at 18:44 +0300, Tomas Winkler wrote:
> This patch removes doubly defined sband variable
 
>  	if ((info->flags & IEEE80211_TX_CTL_USE_RTS_CTS) ||
>  	    (info->flags & IEEE80211_TX_CTL_USE_CTS_PROTECT)) {
> -		struct ieee80211_supported_band *sband;
>  		struct ieee80211_rate *rate;
>  		s8 baserate = -1;
>  		int idx;

I guess we can remove the double assignment too since it's the same as
already done. I must have added that when doing the mechanical tx rate
idx changes. :/

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 2/2] mac80211: removing shadowed sband

[Tomas Winkler]

On Mon, Jun 2, 2008 at 7:54 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2008-06-02 at 18:44 +0300, Tomas Winkler wrote:
>> This patch removes doubly defined sband variable
>
>>       if ((info->flags & IEEE80211_TX_CTL_USE_RTS_CTS) ||
>>           (info->flags & IEEE80211_TX_CTL_USE_CTS_PROTECT)) {
>> -             struct ieee80211_supported_band *sband;
>>               struct ieee80211_rate *rate;
>>               s8 baserate = -1;
>>               int idx;
>
> I guess we can remove the double assignment too since it's the same as
> already done. I must have added that when doing the mechanical tx rate
> idx changes. :/
>
Good point. I also did only mechanical fix for the compiler warning ;)
Tomas

Re: [PATCH 2/2] mac80211: removing shadowed sband

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1025 bytes --]

On Mon, 2008-06-02 at 20:04 +0300, Tomas Winkler wrote:
> On Mon, Jun 2, 2008 at 7:54 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> > On Mon, 2008-06-02 at 18:44 +0300, Tomas Winkler wrote:
> >> This patch removes doubly defined sband variable
> >
> >>       if ((info->flags & IEEE80211_TX_CTL_USE_RTS_CTS) ||
> >>           (info->flags & IEEE80211_TX_CTL_USE_CTS_PROTECT)) {
> >> -             struct ieee80211_supported_band *sband;
> >>               struct ieee80211_rate *rate;
> >>               s8 baserate = -1;
> >>               int idx;
> >
> > I guess we can remove the double assignment too since it's the same as
> > already done. I must have added that when doing the mechanical tx rate
> > idx changes. :/
> >
> Good point. I also did only mechanical fix for the compiler warning ;)

Heh :) Was that sparse, or do you actually get compiler warnings?
Anyhow, here's my Acked-by: Johannes Berg <johannes@sipsolutions.net>,
feel free to add to the patches right away.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 1/2] mac80211: fix unbalanced locking in ieee80211_get_buffered_bc

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 146 bytes --]

Good catch. However, we'll leak skb in that case, so how about:
 
	if (WARN_ON(tx.skb != skb))
-		return NULL;
+		res = TX_DROP;

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 1/2] mac80211: fix unbalanced locking in ieee80211_get_buffered_bc

[Tomas Winkler]

On Mon, Jun 2, 2008 at 7:53 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> Good catch. However, we'll leak skb in that case, so how about:
>
>        if (WARN_ON(tx.skb != skb))
> -               return NULL;
> +               res = TX_DROP;
>
Looks better, will send V2.

> johannes
>