Re: [PATCH] mac80211 : fix a race with update_tkip_key

[gregor kowski <gregor.kowski@gmail.com>]

Update : I changed the logic. There is a single flag per key, tracking
if we send the key instead of a flag per rx queue.

The mac80211 tkip code won't call update_tkip_key, if some rx packets
get received without KEY_FLAG_UPLOADED_TO_HARDWARE. This can happen on
first packet because the hardware key stuff is called asynchronously with
todo workqueue.

This patch workaround that by tracking if we send the key to hardware.


Signed-off-by: Gregor Kowski <gregor.kowski@gmail.com>
Index: linux-2.6/net/mac80211/tkip.c
===================================================================
--- linux-2.6.orig/net/mac80211/tkip.c	2009-06-19 19:13:47.000000000 +0000
+++ linux-2.6/net/mac80211/tkip.c	2009-06-19 19:21:50.000000000 +0000
@@ -282,6 +282,7 @@
 	    key->u.tkip.rx[queue].iv32 != iv32) {
 		/* IV16 wrapped around - perform TKIP phase 1 */
 		tkip_mixing_phase1(tk, &key->u.tkip.rx[queue], ta, iv32);
+		key->u.tkip.rx_tkip_key_sent = 0;
 #ifdef CONFIG_MAC80211_TKIP_DEBUG
 		{
 			int i;
@@ -298,19 +299,21 @@
 			printk("\n");
 		}
 #endif
-		if (key->local->ops->update_tkip_key &&
-			key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE) {
-			u8 bcast[ETH_ALEN] =
-				{0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
-			u8 *sta_addr = key->sta->sta.addr;
+	}
+	if (key->local->ops->update_tkip_key &&
+	    key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE &&
+	    !key->u.tkip.rx_tkip_key_sent) {
+		u8 bcast[ETH_ALEN] =
+			{0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+		u8 *sta_addr = key->sta->sta.addr;

-			if (is_multicast_ether_addr(ra))
-				sta_addr = bcast;
+		if (is_multicast_ether_addr(ra))
+			sta_addr = bcast;

-			key->local->ops->update_tkip_key(
-				local_to_hw(key->local), &key->conf,
-				sta_addr, iv32, key->u.tkip.rx[queue].p1k);
-		}
+		key->local->ops->update_tkip_key(
+			local_to_hw(key->local), &key->conf,
+			sta_addr, iv32, key->u.tkip.rx[queue].p1k);
+		key->u.tkip.rx_tkip_key_sent = 1;
 	}

 	tkip_mixing_phase2(tk, &key->u.tkip.rx[queue], iv16, rc4key);
Index: linux-2.6/net/mac80211/key.h
===================================================================
--- linux-2.6.orig/net/mac80211/key.h	2009-06-19 19:22:20.000000000 +0000
+++ linux-2.6/net/mac80211/key.h	2009-06-19 19:22:31.000000000 +0000
@@ -86,6 +86,7 @@

 			/* last received RSC */
 			struct tkip_ctx rx[NUM_RX_DATA_QUEUES];
+			int rx_tkip_key_sent;
 		} tkip;
 		struct {
 			u8 tx_pn[6];

On 6/12/09, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Fri, 2009-06-12 at 22:41 +0200, gregor kowski wrote:
>
>
> Please try using a proper email client that can inline patches, as
> outlined in
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt;hb=HEAD
>
> Due to you not doing that, I now have to copy/paste from your attachment
> rather than being able to simply reply.
>
>> Here is a patch that should fix all issue :
>
> Remove that sentence?
>
>> The mac80211 tkip code won't call update_tkip_key, if some rx packets
>> get received without KEY_FLAG_UPLOADED_TO_HARDWARE. This can happen on
>> first packet because the hardware key stuff is called asynchronously with
>> todo workqueue.
>
> That seems fine.
>
>> This patch workaround that by always calling once update_tkip_key if
>> the packet wasn't decrypted by the hardware.
>
> But I don't think this is actually true?
>
>> +enum {
>> +       INITIALIZED_NONE,
>> +       INITIALIZED_PHASE1,
>> +       INITIALIZED_UPDATE_KEY,
>> +};
>
> Please declare this in a header file and use the proper enum type for
> the variable as well so the compiler knows what you're doing. You should
> also prefix it then.
>
> johannes
>