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

gregor kowski
2009-06-10T19:42:46Z

On Tue, Jun 9, 2009 at 7:52 PM, Johannes Berg<johannes@sipsolutions.net> wrote:
> On Tue, 2009-06-09 at 19:48 +0200, gregor kowski wrote:
>
>> > Right. But drivers are free to even only _encrypt_ tkip frames and never
>> > _decrypt_ them after having accepted a hardware key, iow that is
>> > perfectly valid behaviour and I don't think we should keep uploading the
>> > key to the driver. Worst case is that the proper upload fails and we
>> > decrypt all frames in software until the next rollover.
>> >
>> What's the point of setting the tkip callback if we aren't interested
>> in decrypting data by hardware ?
>
> Might depend on something else? Anyhow I don't see the point of
> continuing to call the callback. Maybe it should just be part of the key
> todo instead when the key is initially uploaded to the hw.

Or what do you think about this. This will call the callback only once per wrap.

Gregor.

Index: linux-2.6/net/mac80211/tkip.c
===================================================================
--- linux-2.6.orig/net/mac80211/tkip.c 2009-06-08 19:37:19.000000000 +0000
+++ linux-2.6/net/mac80211/tkip.c 2009-06-10 19:28:20.000000000 +0000
@@ -274,7 +274,7 @@

if (only_iv) {
res = TKIP_DECRYPT_OK;
- key->u.tkip.rx[queue].initialized = 1;
+ key->u.tkip.rx[queue].initialized = 2;
goto done;
}

@@ -298,19 +298,22 @@
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 (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);
- }
+ }
+ /* initialized == 2 means we already call update_tkip_key */
+ if (key->local->ops->update_tkip_key &&
+ key->flags & KEY_FLAG_UPLOADED_TO_HARDWARE &&
+ key->u.tkip.rx[queue].initialized != 2) {
+ 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;
+
+ 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[queue].initialized = 2;
}

tkip_mixing_phase2(tk, &key->u.tkip.rx[queue], iv16, rc4key);