[PATCH v2] mac80211: Get IV len from key conf and not cipher scheme

[PATCH v2] mac80211: Get IV len from key conf and not cipher scheme

[Cedric Izoard]

Do not always dereference sta to get cipher scheme as
it may be null for broadcast messages.
Instead get IV length from key configuration which has been
initialized with cipher scheme.

Signed-off-by: Cedric Izoard <cedric.izoard@ceva-dsp.com>
---
  net/mac80211/wpa.c | 22 +++++++++++-----------
  1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 75de6fa..e5d7bfa 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -780,9 +780,8 @@ ieee80211_crypto_cs_encrypt(struct ieee80211_tx_data 
*tx,
  	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
  	struct ieee80211_key *key = tx->key;
  	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
-	const struct ieee80211_cipher_scheme *cs = key->sta->cipher_scheme;
  	int hdrlen;
-	u8 *pos;
+	u8 *pos, iv_len = key->conf.iv_len;

  	if (info->control.hw_key &&
  	    !(info->control.hw_key->flags & IEEE80211_KEY_FLAG_PUT_IV_SPACE)) {
@@ -790,14 +789,17 @@ ieee80211_crypto_cs_encrypt(struct 
ieee80211_tx_data *tx,
  		return TX_CONTINUE;
  	}

-	if (unlikely(skb_headroom(skb) < cs->hdr_len &&
-		     pskb_expand_head(skb, cs->hdr_len, 0, GFP_ATOMIC)))
+	if (iv_len == 0)
+		return TX_CONTINUE;
+
+	if (unlikely(skb_headroom(skb) < iv_len &&
+		     pskb_expand_head(skb, iv_len, 0, GFP_ATOMIC)))
  		return TX_DROP;

  	hdrlen = ieee80211_hdrlen(hdr->frame_control);

-	pos = skb_push(skb, cs->hdr_len);
-	memmove(pos, pos + cs->hdr_len, hdrlen);
+	pos = skb_push(skb, iv_len);
+	memmove(pos, pos + iv_len, hdrlen);

  	return TX_CONTINUE;
  }
@@ -1217,11 +1219,9 @@ ieee80211_crypto_hw_encrypt(struct 
ieee80211_tx_data *tx)
  		if (!info->control.hw_key)
  			return TX_DROP;

-		if (tx->key->sta->cipher_scheme) {
-			res = ieee80211_crypto_cs_encrypt(tx, skb);
-			if (res != TX_CONTINUE)
-				return res;
-		}
+		res = ieee80211_crypto_cs_encrypt(tx, skb);
+		if (res != TX_CONTINUE)
+			return res;
  	}

  	ieee80211_tx_set_protected(tx);
-- 
1.9.1

Re: [PATCH v2] mac80211: Get IV len from key conf and not cipher scheme

[Johannes Berg]

I'm not sure I could follow your discussion...

> @@ -790,14 +789,17 @@ ieee80211_crypto_cs_encrypt(struct 
> ieee80211_tx_data *tx,
>   		return TX_CONTINUE;
>   	}
> 
> -	if (unlikely(skb_headroom(skb) < cs->hdr_len &&
> -		     pskb_expand_head(skb, cs->hdr_len, 0, GFP_ATOMIC)))
> +	if (iv_len == 0)
> +		return TX_CONTINUE;

How can this be correct? You have a cipher scheme, so you want to
encrypt, but now you're not doing that? Perhaps you should drop the
frame instead?

johannes

Re: [PATCH v2] mac80211: Get IV len from key conf and not cipher scheme

[Cedric Izoard]

> How can this be correct? You have a cipher scheme, so you want to
> encrypt, but now you're not doing that? Perhaps you should drop the
> frame instead?
>
On Tx, cipher scheme is "only" available trough sta pointer and is only 
used in ieee80211_crypto_cs_encrypt to get security header len.
Since sta pointer is NULL for bcast messages, the proposed patch get the 
security header length using the key->conf.iv_len instead.

If the key is installed with a cs, then key->conf.iv_len is initialized 
with cs->hdr_len.

If the key is installed without a cs, then  key->conf.iv_len is 0 (hence 
the early exit in ieee80211_crypto_cs_encrypt)

rgds,
cedric

Re: [PATCH v2] mac80211: Get IV len from key conf and not cipher scheme

[Johannes Berg]

On Tue, 2015-03-10 at 16:46 +0000, Cedric Izoard wrote:
> > How can this be correct? You have a cipher scheme, so you want to
> > encrypt, but now you're not doing that? Perhaps you should drop the
> > frame instead?
> >
> On Tx, cipher scheme is "only" available trough sta pointer and is only 
> used in ieee80211_crypto_cs_encrypt to get security header len.
> Since sta pointer is NULL for bcast messages, the proposed patch get the 
> security header length using the key->conf.iv_len instead.
> 
> If the key is installed with a cs, then key->conf.iv_len is initialized 
> with cs->hdr_len.
> 
> If the key is installed without a cs, then  key->conf.iv_len is 0 (hence 
> the early exit in ieee80211_crypto_cs_encrypt)

So in reality, you're checking "is this a CS key"? Perhaps there's a
better way to do that?

johannes

Re: [PATCH v2] mac80211: Get IV len from key conf and not cipher scheme

[Cedric Izoard]

On 10/03/2015 17:48, Johannes Berg wrote:
> On Tue, 2015-03-10 at 16:46 +0000, Cedric Izoard wrote:
>>> How can this be correct? You have a cipher scheme, so you want to
>>> encrypt, but now you're not doing that? Perhaps you should drop the
>>> frame instead?
>>>
>> On Tx, cipher scheme is "only" available trough sta pointer and is only
>> used in ieee80211_crypto_cs_encrypt to get security header len.
>> Since sta pointer is NULL for bcast messages, the proposed patch get the
>> security header length using the key->conf.iv_len instead.
>>
>> If the key is installed with a cs, then key->conf.iv_len is initialized
>> with cs->hdr_len.
>>
>> If the key is installed without a cs, then  key->conf.iv_len is 0 (hence
>> the early exit in ieee80211_crypto_cs_encrypt)
>
> So in reality, you're checking "is this a CS key"? Perhaps there's a
> better way to do that?
>
> johannes
>
>
The other option I see, would be to add a pointer to the cipher scheme 
in struct ieee80211_key. So that in TX cipher_scheme would be 
test/accessed using key ptr. sta->cipher_scheme would still be used for RX.
Do you consider this as a better option ?

rgds,
cedric

Re: [PATCH v2] mac80211: Get IV len from key conf and not cipher scheme

[Johannes Berg]

On Tue, 2015-03-10 at 17:28 +0000, Cedric Izoard wrote:

> The other option I see, would be to add a pointer to the cipher scheme 
> in struct ieee80211_key. So that in TX cipher_scheme would be 
> test/accessed using key ptr. sta->cipher_scheme would still be used for RX.
> Do you consider this as a better option ?

A whole pointer seems a bit pointless, maybe a key flag would be better?

johannes

RE: [PATCH v2] mac80211: Get IV len from key conf and not cipher scheme

[Stepanov, Max]

PiBPbiBUdWUsIDIwMTUtMDMtMTAgYXQgMTk6MzMgKzAwMDAsIEpvaGFubmVzIEJlcmcgd3JvdGU6
DQo+PiBPbiBUdWUsIDIwMTUtMDMtMTAgYXQgMTc6MjggKzAwMDAsIENlZHJpYyBJem9hcmQgd3Jv
dGU6DQo+Pg0KPj4gVGhlIG90aGVyIG9wdGlvbiBJIHNlZSwgd291bGQgYmUgdG8gYWRkIGEgcG9p
bnRlciB0byB0aGUgY2lwaGVyIHNjaGVtZSANCj4+IGluIHN0cnVjdCBpZWVlODAyMTFfa2V5LiBT
byB0aGF0IGluIFRYIGNpcGhlcl9zY2hlbWUgd291bGQgYmUgDQo+PiB0ZXN0L2FjY2Vzc2VkIHVz
aW5nIGtleSBwdHIuIHN0YS0+Y2lwaGVyX3NjaGVtZSB3b3VsZCBzdGlsbCBiZSB1c2VkIGZvciBS
WC4NCj4+IERvIHlvdSBjb25zaWRlciB0aGlzIGFzIGEgYmV0dGVyIG9wdGlvbiA/DQo+DQo+IEEg
d2hvbGUgcG9pbnRlciBzZWVtcyBhIGJpdCBwb2ludGxlc3MsIG1heWJlIGEga2V5IGZsYWcgd291
bGQgYmUgYmV0dGVyPw0KDQpJIGFncmVlIHdpdGggSm9oYW5uZXMgLSBhIGtleSBmbGFnIGlzIGJl
dHRlci4gQW5vdGhlciBwb2ludCBpcyB0aGF0IGllZWU4MDIxMV9jcnlwdG9fY3NfZW5jcnlwdCgp
IGZ1bmN0aW9uIGlzIGNhbGxlZCBmb3Igbm9uIGNzIGNhc2VzLiBJdCBtYWtlcyBzZW5zZSB0byBj
YWxsIGl0IG9ubHkgaWYgdGhlIGtleSBmbGFnIGlzIHNldC4NCg==