[PATCH] b43 add harware tkip

[PATCH] b43 add harware tkip

[gregor kowski]

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



[-- Attachment #2: tkip.diff --]
[-- Type: text/x-diff, Size: 9996 bytes --]

This add hardware tkip for b43.

It uncovered a bug in b43_write_probe_resp_template : it is writing at the
wrong shm offset, it is in the B43_SHM_SH_TKIPTSCTTAK zone. This patch
comments these writes.
There is another problem with b43_new_kidx_api, that allow 54 pairwise key
(max key is 58), but there is only 50 entry in TKIPTSCTTAK (and may be RCMTA).

Finally there is an issue in the mac80211 tkip code that won't always
call update_tkip_key.
A patch to workaround that is also provided.

Signed-off-by: Gregor Kowski <gregor.kowski@gmail.com>
Index: linux-2.6/drivers/net/wireless/b43/dma.c
===================================================================
--- linux-2.6.orig/drivers/net/wireless/b43/dma.c	2009-06-07 21:33:42.000000000 +0000
+++ linux-2.6/drivers/net/wireless/b43/dma.c	2009-06-07 21:41:58.000000000 +0000
@@ -1188,7 +1188,7 @@
 	header = &(ring->txhdr_cache[(slot / TX_SLOTS_PER_FRAME) * hdrsize]);
 	cookie = generate_cookie(ring, slot);
 	err = b43_generate_txhdr(ring->dev, header,
-				 skb->data, skb->len, info, cookie);
+				 skb, info, cookie);
 	if (unlikely(err)) {
 		ring->current_slot = old_top_slot;
 		ring->used_slots = old_used_slots;
Index: linux-2.6/drivers/net/wireless/b43/main.c
===================================================================
--- linux-2.6.orig/drivers/net/wireless/b43/main.c	2009-06-07 21:33:42.000000000 +0000
+++ linux-2.6/drivers/net/wireless/b43/main.c	2009-06-07 21:41:58.000000000 +0000
@@ -860,6 +860,66 @@
 	}
 }
 
+/* The ucode will use this with key to decrypt rx packets.
+ * It will first check if the iv32 match,
+ * - if they don't it returns the packet without decryption (and software
+ *   decryption can be done). That's what happen when iv16 wrap.
+ * - if they do, the rc4 key is computed with tkip phase2, and
+ *   the wep decryption is tried on the packet. Either it will
+ *   success and B43_RX_MAC_DEC is returned, either it fails
+ *   and B43_RX_MAC_DEC|B43_RX_MAC_DECERR is returned and the packet
+ *   is not usable (wrong key used on it).
+ * So in order to never have B43_RX_MAC_DECERR, we should provide
+ * a iv32 and phase1key that match. Because we drop packets in case of
+ * B43_RX_MAC_DECERR, if we have a correct iv32 but a wrong phase1key, all
+ * packets will be lost without higher layer knowing (ie no resync possible
+ * until next wrap).
+ *
+ * NOTE : this should support 50 key like RCMTA because
+ * (B43_SHM_SH_KEYIDXBLOCK - B43_SHM_SH_TKIPTSCTTAK)/14 = 50
+ */
+static void rx_tkip_phase1_write(struct b43_wldev *dev, u8 index, u32 iv32,
+		u16 *phase1key)
+{
+	unsigned int i;
+	u32 offset;
+	u8 per_sta_keys_start = 8;
+
+	if (b43_new_kidx_api(dev))
+		per_sta_keys_start = 4;
+
+	B43_WARN_ON(index < per_sta_keys_start);
+	/* We have two default TX keys and possibly two default RX keys.
+	 * Physical mac 0 is mapped to physical key 4 or 8, depending
+	 * on the firmware version.
+	 * So we must adjust the index here.
+	 */
+	index -= per_sta_keys_start;
+
+	b43dbg(dev->wl, "rx_tkip_phase1_write : idx 0x%x, iv32 0x%x\n",
+			index, iv32);
+	/* Write the key to the  RX tkip shared mem */
+	offset = B43_SHM_SH_TKIPTSCTTAK + index * (10 + 4);
+	for (i = 0; i < 10; i += 2) {
+		b43_shm_write16(dev, B43_SHM_SHARED, offset + i, phase1key[i/2]);
+	}
+	b43_shm_write16(dev, B43_SHM_SHARED, offset + i, iv32);
+	b43_shm_write16(dev, B43_SHM_SHARED, offset + i + 2, iv32>>16);
+}
+
+static void b43_mac_update_tkip_key(struct ieee80211_hw *hw,
+			struct ieee80211_key_conf *keyconf, const u8 *addr,
+			u32 iv32, u16 *phase1key)
+{
+	struct b43_wl *wl = hw_to_b43_wl(hw);
+	struct b43_wldev *dev = wl->current_dev;
+	int index = keyconf->hw_key_idx;
+	keymac_write(dev, index, NULL);	/* First zero out mac to avoid race */
+
+	rx_tkip_phase1_write(dev, index, iv32, phase1key);
+	keymac_write(dev, index, addr);
+}
+
 static void do_key_write(struct b43_wldev *dev,
 			 u8 index, u8 algorithm,
 			 const u8 *key, size_t key_len, const u8 *mac_addr)
@@ -878,6 +938,19 @@
 	if (key)
 		memcpy(buf, key, key_len);
 	key_write(dev, index, algorithm, buf);
+	if (algorithm == B43_SEC_ALGO_TKIP) {
+		/*
+		 * We should provide an initial iv32, phase1key pair.
+		 * We could start with iv32=0 and compute the corresponding
+		 * phase1key, but this mean calling ieee80211_get_tkip_key
+		 * with a fake skb (or export other tkip function).
+		 * Because we are lazy we hope iv32 won't start with
+		 * 0xffff and let's b43_mac_update_tkip_key provide a
+		 * correct pair.
+		 */
+		rx_tkip_phase1_write(dev, index, 0xffff, (u16*)buf);
+	} else /* clear it */
+		rx_tkip_phase1_write(dev, index, 0, (u16*)buf);
 	if (index >= per_sta_keys_start)
 		keymac_write(dev, index, mac_addr);
 
@@ -893,6 +966,11 @@
 	int i;
 	int sta_keys_start;
 
+	if (algorithm == B43_SEC_ALGO_TKIP && key_len == 32) {
+		keyconf->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
+		keyconf->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
+		key_len = 16;
+	}
 	if (key_len > B43_SEC_KEYSIZE)
 		return -EINVAL;
 	for (i = 0; i < dev->max_nr_keys; i++) {
@@ -954,6 +1032,7 @@
 		b43_key_clear(dev, i);
 }
 
+/* XXX add tkip dump */
 static void b43_dump_keymemory(struct b43_wldev *dev)
 {
 	unsigned int i, index, offset;
@@ -1545,10 +1624,13 @@
 	/* Looks like PLCP headers plus packet timings are stored for
 	 * all possible basic rates
 	 */
+	/* FIXME this is the wrong offset : it goes in tkip rx phase1 shm */
+#if 0
 	b43_write_probe_resp_plcp(dev, 0x31A, size, &b43_b_ratetable[0]);
 	b43_write_probe_resp_plcp(dev, 0x32C, size, &b43_b_ratetable[1]);
 	b43_write_probe_resp_plcp(dev, 0x33E, size, &b43_b_ratetable[2]);
 	b43_write_probe_resp_plcp(dev, 0x350, size, &b43_b_ratetable[3]);
+#endif
 
 	size = min((size_t) size, 0x200 - sizeof(struct b43_plcp_hdr6));
 	b43_write_template_common(dev, probe_resp_data,
@@ -2966,6 +3048,9 @@
 
 static void b43_security_init(struct b43_wldev *dev)
 {
+	/* FIXME : for b43_new_kidx_api, there can be 54 key
+	 * instead of 50 in RCMTA and TKIPTSCTTAK.
+	 */
 	dev->max_nr_keys = (dev->dev->id.revision >= 5) ? 58 : 20;
 	B43_WARN_ON(dev->max_nr_keys > ARRAY_SIZE(dev->key));
 	dev->ktp = b43_shm_read16(dev, B43_SHM_SHARED, B43_SHM_SH_KTP);
@@ -3651,8 +3736,10 @@
 
 	switch (cmd) {
 	case SET_KEY:
-		if (algorithm == B43_SEC_ALGO_TKIP) {
-			/* FIXME: No TKIP hardware encryption for now. */
+		if (algorithm == B43_SEC_ALGO_TKIP &&
+				(!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE) ||
+				   key->flags & IEEE80211_KEY_FLAG_WMM_STA )) {
+			/* We support only one rx queue (no QOS) and pairwise key */
 			err = -EOPNOTSUPP;
 			goto out_unlock;
 		}
@@ -4452,6 +4539,7 @@
 	.config_interface	= b43_op_config_interface,
 	.configure_filter	= b43_op_configure_filter,
 	.set_key		= b43_op_set_key,
+	.update_tkip_key	= b43_mac_update_tkip_key,
 	.get_stats		= b43_op_get_stats,
 	.get_tx_stats		= b43_op_get_tx_stats,
 	.get_tsf		= b43_op_get_tsf,
Index: linux-2.6/drivers/net/wireless/b43/pio.c
===================================================================
--- linux-2.6.orig/drivers/net/wireless/b43/pio.c	2009-06-07 21:33:42.000000000 +0000
+++ linux-2.6/drivers/net/wireless/b43/pio.c	2009-06-07 21:41:58.000000000 +0000
@@ -461,8 +461,8 @@
 
 	cookie = generate_cookie(q, pack);
 	hdrlen = b43_txhdr_size(q->dev);
-	err = b43_generate_txhdr(q->dev, (u8 *)&txhdr, skb->data,
-				 skb->len, info, cookie);
+	err = b43_generate_txhdr(q->dev, (u8 *)&txhdr, skb,
+				 info, cookie);
 	if (err)
 		return err;
 
Index: linux-2.6/drivers/net/wireless/b43/xmit.c
===================================================================
--- linux-2.6.orig/drivers/net/wireless/b43/xmit.c	2009-06-07 21:33:42.000000000 +0000
+++ linux-2.6/drivers/net/wireless/b43/xmit.c	2009-06-07 21:42:38.000000000 +0000
@@ -181,11 +181,12 @@
 /* Generate a TX data header. */
 int b43_generate_txhdr(struct b43_wldev *dev,
 		       u8 *_txhdr,
-		       const unsigned char *fragment_data,
-		       unsigned int fragment_len,
+		       struct sk_buff *skb_frag,
 		       struct ieee80211_tx_info *info,
 		       u16 cookie)
 {
+	const unsigned char *fragment_data = skb_frag->data;
+	unsigned int fragment_len = skb_frag->len;
 	struct b43_txhdr *txhdr = (struct b43_txhdr *)_txhdr;
 	const struct b43_phy *phy = &dev->phy;
 	const struct ieee80211_hdr *wlhdr =
@@ -259,9 +260,25 @@
 		mac_ctl |= (key->algorithm << B43_TXH_MAC_KEYALG_SHIFT) &
 			   B43_TXH_MAC_KEYALG;
 		wlhdr_len = ieee80211_hdrlen(fctl);
-		iv_len = min((size_t) info->control.hw_key->iv_len,
-			     ARRAY_SIZE(txhdr->iv));
-		memcpy(txhdr->iv, ((u8 *) wlhdr) + wlhdr_len, iv_len);
+		if (key->algorithm == B43_SEC_ALGO_TKIP) {
+			u16 phase1key[5];
+			int i;
+			/* we give the phase1key and iv16 here, the key is stored in
+			 * shm. With that the hardware can do phase 2 and encryption.
+			 */
+			ieee80211_get_tkip_key(info->control.hw_key, skb_frag, IEEE80211_TKIP_P1_KEY, (u8*)phase1key);
+			/* phase1key is in host endian */
+			for (i = 0; i < 5; i++)
+				phase1key[i] = cpu_to_le16(phase1key[i]);
+
+			memcpy(txhdr->iv, phase1key, 10);
+			/* iv16 */
+			memcpy(txhdr->iv+10, ((u8 *) wlhdr) + wlhdr_len, 3);
+		} else {
+			iv_len = min((size_t) info->control.hw_key->iv_len,
+				     ARRAY_SIZE(txhdr->iv));
+			memcpy(txhdr->iv, ((u8 *) wlhdr) + wlhdr_len, iv_len);
+		}
 	}
 	if (b43_is_old_txhdr_format(dev)) {
 		b43_generate_plcp_hdr((struct b43_plcp_hdr4 *)(&txhdr->old_format.plcp),
Index: linux-2.6/drivers/net/wireless/b43/xmit.h
===================================================================
--- linux-2.6.orig/drivers/net/wireless/b43/xmit.h	2009-06-07 21:33:42.000000000 +0000
+++ linux-2.6/drivers/net/wireless/b43/xmit.h	2009-06-07 21:41:58.000000000 +0000
@@ -176,8 +176,7 @@
 
 int b43_generate_txhdr(struct b43_wldev *dev,
 		       u8 * txhdr,
-		       const unsigned char *fragment_data,
-		       unsigned int fragment_len,
+		       struct sk_buff *skb_frag,
 		       struct ieee80211_tx_info *txctl, u16 cookie);
 
 /* Transmit Status */

Re: [PATCH] b43 add harware tkip

[Gábor Stefanik]

On Sun, Jun 7, 2009 at 11:50 PM, gregor kowski<gregor.kowski@gmail.com> wrote:
>
>

This is your entire mail body.
Please inline the patch, or at least include a description.
Also, CC bcm43xx-dev@berlios.de

--Gábor

-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [PATCH] b43 add harware tkip

[Johannes Berg]

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

On Mon, 2009-06-08 at 00:34 +0200, Gábor Stefanik wrote:
> On Sun, Jun 7, 2009 at 11:50 PM, gregor kowski<gregor.kowski@gmail.com> wrote:

> This is your entire mail body.
> Please inline the patch, or at least include a description.

This is ridiculous, you've never managed to do that correctly yourself.

johannes

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

Re: [PATCH] b43 add harware tkip

[Michael Buesch]

Well, first thing is that I do think there is a reason Broadcom removed hw TKIP
support from their drivers. ;)

But well, let's look at the patch.

> +	/* FIXME this is the wrong offset : it goes in tkip rx phase1 shm */
> +#if 0
>  	b43_write_probe_resp_plcp(dev, 0x31A, size, &b43_b_ratetable[0]);
>  	b43_write_probe_resp_plcp(dev, 0x32C, size, &b43_b_ratetable[1]);
>  	b43_write_probe_resp_plcp(dev, 0x33E, size, &b43_b_ratetable[2]);
>  	b43_write_probe_resp_plcp(dev, 0x350, size, &b43_b_ratetable[3]);
> +#endif

This looks interesting. Care to find out the correct offsets and submit
this fix as a separate patch?

> +	if (algorithm == B43_SEC_ALGO_TKIP) {
> +		/*
> +		 * We should provide an initial iv32, phase1key pair.
> +		 * We could start with iv32=0 and compute the corresponding
> +		 * phase1key, but this mean calling ieee80211_get_tkip_key
> +		 * with a fake skb (or export other tkip function).
> +		 * Because we are lazy we hope iv32 won't start with
> +		 * 0xffff and let's b43_mac_update_tkip_key provide a
> +		 * correct pair.
> +		 */
> +		rx_tkip_phase1_write(dev, index, 0xffff, (u16*)buf);
> +	} else /* clear it */
> +		rx_tkip_phase1_write(dev, index, 0, (u16*)buf);

Why do you write phase1, if TKIP is not used?

> +	/* FIXME : for b43_new_kidx_api, there can be 54 key
> +	 * instead of 50 in RCMTA and TKIPTSCTTAK.
> +	 */

I don't understand this comment.

> -		if (algorithm == B43_SEC_ALGO_TKIP) {
> -			/* FIXME: No TKIP hardware encryption for now. */
> +		if (algorithm == B43_SEC_ALGO_TKIP &&
> +				(!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE) ||
> +				   key->flags & IEEE80211_KEY_FLAG_WMM_STA )) {
> +			/* We support only one rx queue (no QOS) and pairwise key */

This comment doesn't really make sense to me, too.
What does QoS have to do with the RX queue?


Next time please inline the patch ;)

-- 
Greetings, Michael.

Re: [PATCH] b43 add harware tkip

[gregor kowski]

Hi,

thanks for your review

On Mon, Jun 8, 2009 at 5:20 PM, Michael Buesch<mb@bu3sch.de> wrote:
> Well, first thing is that I do think there is a reason Broadcom removed hw TKIP
> support from their drivers. ;)
>
> But well, let's look at the patch.
>
>> +     /* FIXME this is the wrong offset : it goes in tkip rx phase1 shm */
>> +#if 0
>>       b43_write_probe_resp_plcp(dev, 0x31A, size, &b43_b_ratetable[0]);
>>       b43_write_probe_resp_plcp(dev, 0x32C, size, &b43_b_ratetable[1]);
>>       b43_write_probe_resp_plcp(dev, 0x33E, size, &b43_b_ratetable[2]);
>>       b43_write_probe_resp_plcp(dev, 0x350, size, &b43_b_ratetable[3]);
>> +#endif
>
> This looks interesting. Care to find out the correct offsets and submit
> this fix as a separate patch?
>
May be if I have time. But a comment suggest it is not used ?
>> +     if (algorithm == B43_SEC_ALGO_TKIP) {
>> +             /*
>> +              * We should provide an initial iv32, phase1key pair.
>> +              * We could start with iv32=0 and compute the corresponding
>> +              * phase1key, but this mean calling ieee80211_get_tkip_key
>> +              * with a fake skb (or export other tkip function).
>> +              * Because we are lazy we hope iv32 won't start with
>> +              * 0xffff and let's b43_mac_update_tkip_key provide a
>> +              * correct pair.
>> +              */
>> +             rx_tkip_phase1_write(dev, index, 0xffff, (u16*)buf);
>> +     } else /* clear it */
>> +             rx_tkip_phase1_write(dev, index, 0, (u16*)buf);
>
> Why do you write phase1, if TKIP is not used?
I clear the value in shared memory. This help debugging. But I could
remove it if you want.
>
>> +     /* FIXME : for b43_new_kidx_api, there can be 54 key
>> +      * instead of 50 in RCMTA and TKIPTSCTTAK.
>> +      */
>
> I don't understand this comment.
if  b43_new_kidx_api is true :
- we set max_nr_keys to 58
- we program B43_MMIO_RCMTA_COUNT to 50
- in b43_key_write we can allocate key up to index 58 (4 for default,
54 for sta)

But there is only 50 entries for TKIPTSCTTAK, and a comment on bcm-v4
suggest there is 50 entries for RCMTA. So if there more than 50
station we will overflow RCMTA and TKIPTSCTTAK.

So the fix will be do to something like
dev->max_nr_keys = (dev->dev->id.revision >= 5) ? 58 : 20;
if (b43_new_kidx_api())
dev->max_nr_keys -= 4;

>
>> -             if (algorithm == B43_SEC_ALGO_TKIP) {
>> -                     /* FIXME: No TKIP hardware encryption for now. */
>> +             if (algorithm == B43_SEC_ALGO_TKIP &&
>> +                             (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE) ||
>> +                                key->flags & IEEE80211_KEY_FLAG_WMM_STA )) {
>> +                     /* We support only one rx queue (no QOS) and pairwise key */
>
> This comment doesn't really make sense to me, too.
> What does QoS have to do with the RX queue?
each QOS queue got it's own tkip counters (iv16, iv32) (check tkip.c
software implementation for more info).

> Next time please inline the patch ;)
I will try, but I don't know if my mailer handle it.

Gregor

Re: [PATCH] b43 add harware tkip

[Michael Buesch]

On Monday 08 June 2009 20:04:33 gregor kowski wrote:
> >> +     if (algorithm == B43_SEC_ALGO_TKIP) {
> >> +             /*
> >> +              * We should provide an initial iv32, phase1key pair.
> >> +              * We could start with iv32=0 and compute the corresponding
> >> +              * phase1key, but this mean calling ieee80211_get_tkip_key
> >> +              * with a fake skb (or export other tkip function).
> >> +              * Because we are lazy we hope iv32 won't start with
> >> +              * 0xffff and let's b43_mac_update_tkip_key provide a
> >> +              * correct pair.
> >> +              */
> >> +             rx_tkip_phase1_write(dev, index, 0xffff, (u16*)buf);
> >> +     } else /* clear it */
> >> +             rx_tkip_phase1_write(dev, index, 0, (u16*)buf);
> >
> > Why do you write phase1, if TKIP is not used?
> I clear the value in shared memory.

Yeah, that's what I don't understand here. Does buf contain only zero bytes in this case?

> >> +     /* FIXME : for b43_new_kidx_api, there can be 54 key
> >> +      * instead of 50 in RCMTA and TKIPTSCTTAK.
> >> +      */
> >
> > I don't understand this comment.
> if  b43_new_kidx_api is true :
> - we set max_nr_keys to 58
> - we program B43_MMIO_RCMTA_COUNT to 50
> - in b43_key_write we can allocate key up to index 58 (4 for default,
> 54 for sta)
> 
> But there is only 50 entries for TKIPTSCTTAK, and a comment on bcm-v4
> suggest there is 50 entries for RCMTA. So if there more than 50
> station we will overflow RCMTA and TKIPTSCTTAK.

Yeah well. The key handling is pretty weird.
There are 50 pairwise keys available. max_nr_keys includes pairwise keys, group keys (4)
and another copy of the group keys (4) used with older firmware. So we end up at 58.

To summarize it, in practice we have 50 pairwise keys and 4 group keys. You can ignore
the additional 4 "rx keys" as they are not used in recent fw and are just a copy of the
group keys.

> So the fix will be do to something like
> dev->max_nr_keys = (dev->dev->id.revision >= 5) ? 58 : 20;
> if (b43_new_kidx_api())
> dev->max_nr_keys -= 4;

Hm, I don't think this is correct. There are lots of weird assumptions all over the code.
We should probably remove that new_kidx_api crap alltogether.

> >> -             if (algorithm == B43_SEC_ALGO_TKIP) {
> >> -                     /* FIXME: No TKIP hardware encryption for now. */
> >> +             if (algorithm == B43_SEC_ALGO_TKIP &&
> >> +                             (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE) ||
> >> +                                key->flags & IEEE80211_KEY_FLAG_WMM_STA )) {
> >> +                     /* We support only one rx queue (no QOS) and pairwise key */
> >
> > This comment doesn't really make sense to me, too.
> > What does QoS have to do with the RX queue?
> each QOS queue got it's own tkip counters (iv16, iv32) (check tkip.c
> software implementation for more info).

Yeah, but what does --> ___RX___ <-- (receive) have to do with that?
We do always only have one RX queue.

-- 
Greetings, Michael.

Re: [PATCH] b43 add harware tkip

[Gábor Stefanik]

2009/6/8 Johannes Berg <johannes@sipsolutions.net>:
> On Mon, 2009-06-08 at 00:34 +0200, Gábor Stefanik wrote:
>> On Sun, Jun 7, 2009 at 11:50 PM, gregor kowski<gregor.kowski@gmail.com> wrote:
>
>> This is your entire mail body.
>> Please inline the patch, or at least include a description.
>
> This is ridiculous, you've never managed to do that correctly yourself.

Well, I never sent an empty mail with only an attachment and nothing
in the body...

>
> johannes
>



-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [PATCH] b43 add harware tkip

[gregor kowski]

On Mon, Jun 8, 2009 at 8:16 PM, Michael Buesch<mb@bu3sch.de> wrote:
> On Monday 08 June 2009 20:04:33 gregor kowski wrote:
>> > Why do you write phase1, if TKIP is not used?
>> I clear the value in shared memory.
>
> Yeah, that's what I don't understand here. Does buf contain only zero bytes in this case?

Opps, in a version version the code was before the memcpy into buf, so
it was always zero. Now if can have other value...
I will change that.

>>
>> But there is only 50 entries for TKIPTSCTTAK, and a comment on bcm-v4
>> suggest there is 50 entries for RCMTA. So if there more than 50
>> station we will overflow RCMTA and TKIPTSCTTAK.
>
> Yeah well. The key handling is pretty weird.
> There are 50 pairwise keys available. max_nr_keys includes pairwise keys, group keys (4)
> and another copy of the group keys (4) used with older firmware. So we end up at 58.
>
> To summarize it, in practice we have 50 pairwise keys and 4 group keys. You can ignore
> the additional 4 "rx keys" as they are not used in recent fw and are just a copy of the
> group keys.
But do you agree that index=55 is an valid sta index that can be
returned by b43_key_write ?
In that case both keymac_write and rx_tkip_phase1_write will use the
55-4=51 entry and overflow.


>
>> So the fix will be do to something like
>> dev->max_nr_keys = (dev->dev->id.revision >= 5) ? 58 : 20;
>> if (b43_new_kidx_api())
>> dev->max_nr_keys -= 4;
>
> Hm, I don't think this is correct. There are lots of weird assumptions all over the code.
> We should probably remove that new_kidx_api crap alltogether.
Yes I think it could simply the code.

>
>> >> -             if (algorithm == B43_SEC_ALGO_TKIP) {
>> >> -                     /* FIXME: No TKIP hardware encryption for now. */
>> >> +             if (algorithm == B43_SEC_ALGO_TKIP &&
>> >> +                             (!(key->flags & IEEE80211_KEY_FLAG_PAIRWISE) ||
>> >> +                                key->flags & IEEE80211_KEY_FLAG_WMM_STA )) {
>> >> +                     /* We support only one rx queue (no QOS) and pairwise key */
>> >
>> > This comment doesn't really make sense to me, too.
>> > What does QoS have to do with the RX queue?
>> each QOS queue got it's own tkip counters (iv16, iv32) (check tkip.c
>> software implementation for more info).
>
> Yeah, but what does --> ___RX___ <-- (receive) have to do with that?
> We do always only have one RX queue.
>From what I understood in case of QOS, the 80211 frame contain an
header that contains the queue number (what is parsed by
ieee80211_parse_qos). For each queue tkip counter are different : look
at soft implementation, it got an array of iv (one for each queue).
What is strange is that I did see only one counter for TX.

Gregor

Re: [PATCH] b43 add harware tkip

[gregor kowski]

Update : work with qos, implement dump key, fix an issue with setting
random value on tkip key clear.
PS : this depends on "b43 : remove old kidx API"

This add hardware tkip for b43.

It uncovered a bug in b43_write_probe_resp_template : it is writing at the
wrong shm offset, it is in the B43_SHM_SH_TKIPTSCTTAK zone. This patch
comments these writes.

Signed-off-by: Gregor Kowski <gregor.kowski@gmail.com>
Index: linux-2.6/drivers/net/wireless/b43/dma.c
===================================================================
--- linux-2.6.orig/drivers/net/wireless/b43/dma.c	2009-06-19
20:31:33.000000000 +0000
+++ linux-2.6/drivers/net/wireless/b43/dma.c	2009-06-20 18:34:23.000000000 +0000
@@ -1188,7 +1188,7 @@
 	header = &(ring->txhdr_cache[(slot / TX_SLOTS_PER_FRAME) * hdrsize]);
 	cookie = generate_cookie(ring, slot);
 	err = b43_generate_txhdr(ring->dev, header,
-				 skb->data, skb->len, info, cookie);
+				 skb, info, cookie);
 	if (unlikely(err)) {
 		ring->current_slot = old_top_slot;
 		ring->used_slots = old_used_slots;
Index: linux-2.6/drivers/net/wireless/b43/main.c
===================================================================
--- linux-2.6.orig/drivers/net/wireless/b43/main.c	2009-06-19
20:31:33.000000000 +0000
+++ linux-2.6/drivers/net/wireless/b43/main.c	2009-06-22
20:50:37.000000000 +0000
@@ -855,6 +855,64 @@
 	}
 }

+/* The ucode will use this with key to decrypt rx packets.
+ * It will first check if the iv32 match,
+ * - if they don't it returns the packet without decryption (and software
+ *   decryption can be done). That's what happen when iv16 wrap.
+ * - if they do, the rc4 key is computed with tkip phase2, and
+ *   the wep decryption is tried on the packet. Either it will
+ *   success and B43_RX_MAC_DEC is returned, either it fails
+ *   and B43_RX_MAC_DEC|B43_RX_MAC_DECERR is returned and the packet
+ *   is not usable (wrong key used on it).
+ * So in order to never have B43_RX_MAC_DECERR, we should provide
+ * a iv32 and phase1key that match. Because we drop packets in case of
+ * B43_RX_MAC_DECERR, if we have a correct iv32 but a wrong phase1key, all
+ * packets will be lost without higher layer knowing (ie no resync possible
+ * until next wrap).
+ *
+ * NOTE : this should support 50 key like RCMTA because
+ * (B43_SHM_SH_KEYIDXBLOCK - B43_SHM_SH_TKIPTSCTTAK)/14 = 50
+ */
+static void rx_tkip_phase1_write(struct b43_wldev *dev, u8 index, u32 iv32,
+		u16 *phase1key)
+{
+	unsigned int i;
+	u32 offset;
+	const u8 per_sta_keys_start = 4;
+
+	B43_WARN_ON(index < per_sta_keys_start);
+	/* We have two default TX keys and possibly two default RX keys.
+	 * Physical mac 0 is mapped to physical key 4 or 8, depending
+	 * on the firmware version.
+	 * So we must adjust the index here.
+	 */
+	index -= per_sta_keys_start;
+
+	if (b43_debug(dev, B43_DBG_KEYS))
+		b43dbg(dev->wl, "rx_tkip_phase1_write : idx 0x%x, iv32 0x%x\n",
+				index, iv32);
+	/* Write the key to the  RX tkip shared mem */
+	offset = B43_SHM_SH_TKIPTSCTTAK + index * (10 + 4);
+	for (i = 0; i < 10; i += 2) {
+		b43_shm_write16(dev, B43_SHM_SHARED, offset + i, phase1key[i/2]);
+	}
+	b43_shm_write16(dev, B43_SHM_SHARED, offset + i, iv32);
+	b43_shm_write16(dev, B43_SHM_SHARED, offset + i + 2, iv32>>16);
+}
+
+static void b43_mac_update_tkip_key(struct ieee80211_hw *hw,
+			struct ieee80211_key_conf *keyconf, const u8 *addr,
+			u32 iv32, u16 *phase1key)
+{
+	struct b43_wl *wl = hw_to_b43_wl(hw);
+	struct b43_wldev *dev = wl->current_dev;
+	int index = keyconf->hw_key_idx;
+	keymac_write(dev, index, NULL);	/* First zero out mac to avoid race */
+
+	rx_tkip_phase1_write(dev, index, iv32, phase1key);
+	keymac_write(dev, index, addr);
+}
+
 static void do_key_write(struct b43_wldev *dev,
 			 u8 index, u8 algorithm,
 			 const u8 *key, size_t key_len, const u8 *mac_addr)
@@ -867,6 +925,19 @@

 	if (index >= per_sta_keys_start)
 		keymac_write(dev, index, NULL);	/* First zero out mac. */
+	if (algorithm == B43_SEC_ALGO_TKIP) {
+		/*
+		 * We should provide an initial iv32, phase1key pair.
+		 * We could start with iv32=0 and compute the corresponding
+		 * phase1key, but this mean calling ieee80211_get_tkip_key
+		 * with a fake skb (or export other tkip function).
+		 * Because we are lazy we hope iv32 won't start with
+		 * 0xffffffff and let's b43_mac_update_tkip_key provide a
+		 * correct pair.
+		 */
+		rx_tkip_phase1_write(dev, index, 0xffffffff, (u16*)buf);
+	} else if (index >= per_sta_keys_start) /* clear it */
+		rx_tkip_phase1_write(dev, index, 0, (u16*)buf);
 	if (key)
 		memcpy(buf, key, key_len);
 	key_write(dev, index, algorithm, buf);
@@ -884,6 +955,8 @@
 {
 	int i;

+	if (algorithm == B43_SEC_ALGO_TKIP && key_len == 32)
+		key_len = 16;
 	if (key_len > B43_SEC_KEYSIZE)
 		return -EINVAL;
 	for (i = 0; i < dev->max_nr_keys; i++) {
@@ -965,6 +1038,14 @@
 		printk("   Algo: %04X/%02X", algo, key->algorithm);

 		if (index >= 4) {
+			if (key->algorithm == B43_SEC_ALGO_TKIP) {
+				printk("   TKIP: ");
+				offset = B43_SHM_SH_TKIPTSCTTAK + (index - 4) * (10 + 4);
+				for (i = 0; i < 14; i+=2) {
+					u16 tmp = b43_shm_read16(dev, B43_SHM_SHARED, offset + i);
+					printk("%02X%02X", (tmp & 0xFF), ((tmp >> 8) & 0xFF));
+				}
+			}
 			rcmta0 = b43_shm_read32(dev, B43_SHM_RCMTA,
 						((index - 4) * 2) + 0);
 			rcmta1 = b43_shm_read16(dev, B43_SHM_RCMTA,
@@ -1524,10 +1605,13 @@
 	/* Looks like PLCP headers plus packet timings are stored for
 	 * all possible basic rates
 	 */
+	/* FIXME this is the wrong offset : it goes in tkip rx phase1 shm */
+#if 0
 	b43_write_probe_resp_plcp(dev, 0x31A, size, &b43_b_ratetable[0]);
 	b43_write_probe_resp_plcp(dev, 0x32C, size, &b43_b_ratetable[1]);
 	b43_write_probe_resp_plcp(dev, 0x33E, size, &b43_b_ratetable[2]);
 	b43_write_probe_resp_plcp(dev, 0x350, size, &b43_b_ratetable[3]);
+#endif

 	size = min((size_t) size, 0x200 - sizeof(struct b43_plcp_hdr6));
 	b43_write_template_common(dev, probe_resp_data,
@@ -3631,8 +3715,9 @@

 	switch (cmd) {
 	case SET_KEY:
-		if (algorithm == B43_SEC_ALGO_TKIP) {
-			/* FIXME: No TKIP hardware encryption for now. */
+		if (algorithm == B43_SEC_ALGO_TKIP &&
+		    !(key->flags & IEEE80211_KEY_FLAG_PAIRWISE)) {
+			/* We support only pairwise key */
 			err = -EOPNOTSUPP;
 			goto out_unlock;
 		}
@@ -3662,6 +3747,8 @@
 				     b43_hf_read(dev) & ~B43_HF_USEDEFKEYS);
 		}
 		key->flags |= IEEE80211_KEY_FLAG_GENERATE_IV;
+		if (algorithm == B43_SEC_ALGO_TKIP)
+			key->flags |= IEEE80211_KEY_FLAG_GENERATE_MMIC;
 		break;
 	case DISABLE_KEY: {
 		err = b43_key_clear(dev, key->hw_key_idx);
@@ -4432,6 +4519,7 @@
 	.config_interface	= b43_op_config_interface,
 	.configure_filter	= b43_op_configure_filter,
 	.set_key		= b43_op_set_key,
+	.update_tkip_key	= b43_mac_update_tkip_key,
 	.get_stats		= b43_op_get_stats,
 	.get_tx_stats		= b43_op_get_tx_stats,
 	.get_tsf		= b43_op_get_tsf,
Index: linux-2.6/drivers/net/wireless/b43/pio.c
===================================================================
--- linux-2.6.orig/drivers/net/wireless/b43/pio.c	2009-06-19
20:31:33.000000000 +0000
+++ linux-2.6/drivers/net/wireless/b43/pio.c	2009-06-20 18:34:23.000000000 +0000
@@ -461,8 +461,8 @@

 	cookie = generate_cookie(q, pack);
 	hdrlen = b43_txhdr_size(q->dev);
-	err = b43_generate_txhdr(q->dev, (u8 *)&txhdr, skb->data,
-				 skb->len, info, cookie);
+	err = b43_generate_txhdr(q->dev, (u8 *)&txhdr, skb,
+				 info, cookie);
 	if (err)
 		return err;

Index: linux-2.6/drivers/net/wireless/b43/xmit.c
===================================================================
--- linux-2.6.orig/drivers/net/wireless/b43/xmit.c	2009-06-19
20:31:33.000000000 +0000
+++ linux-2.6/drivers/net/wireless/b43/xmit.c	2009-06-20
18:34:23.000000000 +0000
@@ -181,11 +181,12 @@
 /* Generate a TX data header. */
 int b43_generate_txhdr(struct b43_wldev *dev,
 		       u8 *_txhdr,
-		       const unsigned char *fragment_data,
-		       unsigned int fragment_len,
+		       struct sk_buff *skb_frag,
 		       struct ieee80211_tx_info *info,
 		       u16 cookie)
 {
+	const unsigned char *fragment_data = skb_frag->data;
+	unsigned int fragment_len = skb_frag->len;
 	struct b43_txhdr *txhdr = (struct b43_txhdr *)_txhdr;
 	const struct b43_phy *phy = &dev->phy;
 	const struct ieee80211_hdr *wlhdr =
@@ -258,9 +259,25 @@
 		mac_ctl |= (key->algorithm << B43_TXH_MAC_KEYALG_SHIFT) &
 			   B43_TXH_MAC_KEYALG;
 		wlhdr_len = ieee80211_hdrlen(fctl);
-		iv_len = min((size_t) info->control.hw_key->iv_len,
-			     ARRAY_SIZE(txhdr->iv));
-		memcpy(txhdr->iv, ((u8 *) wlhdr) + wlhdr_len, iv_len);
+		if (key->algorithm == B43_SEC_ALGO_TKIP) {
+			u16 phase1key[5];
+			int i;
+			/* we give the phase1key and iv16 here, the key is stored in
+			 * shm. With that the hardware can do phase 2 and encryption.
+			 */
+			ieee80211_get_tkip_key(info->control.hw_key, skb_frag,
IEEE80211_TKIP_P1_KEY, (u8*)phase1key);
+			/* phase1key is in host endian */
+			for (i = 0; i < 5; i++)
+				phase1key[i] = cpu_to_le16(phase1key[i]);
+
+			memcpy(txhdr->iv, phase1key, 10);
+			/* iv16 */
+			memcpy(txhdr->iv+10, ((u8 *) wlhdr) + wlhdr_len, 3);
+		} else {
+			iv_len = min((size_t) info->control.hw_key->iv_len,
+				     ARRAY_SIZE(txhdr->iv));
+			memcpy(txhdr->iv, ((u8 *) wlhdr) + wlhdr_len, iv_len);
+		}
 	}
 	if (b43_is_old_txhdr_format(dev)) {
 		b43_generate_plcp_hdr((struct b43_plcp_hdr4 *)(&txhdr->old_format.plcp),
Index: linux-2.6/drivers/net/wireless/b43/xmit.h
===================================================================
--- linux-2.6.orig/drivers/net/wireless/b43/xmit.h	2009-06-19
20:31:33.000000000 +0000
+++ linux-2.6/drivers/net/wireless/b43/xmit.h	2009-06-20
18:34:23.000000000 +0000
@@ -176,8 +176,7 @@

 int b43_generate_txhdr(struct b43_wldev *dev,
 		       u8 * txhdr,
-		       const unsigned char *fragment_data,
-		       unsigned int fragment_len,
+		       struct sk_buff *skb_frag,
 		       struct ieee80211_tx_info *txctl, u16 cookie);

 /* Transmit Status */