[1/2] mac80211: Populate RSC counter in set_key method

Commit Message

Govind Singh Dec. 8, 2017, 1:32 p.m. UTC

Send RSC counter to driver in set_key method, so that FW/driver
can drop the packet in PN check if received packet sequence
no is less than current RSC counter during group keys(GTK)
exchange.

Signed-off-by: Govind Singh <[e-mail]>
---
 include/net/mac80211.h | 6 ++++--
 net/mac80211/key.c     | 6 ++++++
 2 files changed, 10 insertions(+), 2 deletions(-)

Comments

Johannes Berg Dec. 9, 2017, 7:58 p.m. UTC | #1

On Fri, 2017-12-08 at 19:02 +0530, Govind Singh wrote:
> Send RSC counter to driver in set_key method, so that FW/driver
> can drop the packet in PN check if received packet sequence
> no is less than current RSC counter during group keys(GTK)
> exchange.

Good idea, but ... does it really work this way? :-)

>  /**
>   * struct ieee80211_key_conf - key information
>   *
> @@ -1586,6 +1588,7 @@ enum ieee80211_key_flags {
>   * 	- Temporal Authenticator Rx MIC Key (64 bits)
>   * @icv_len: The ICV length for this key type
>   * @iv_len: The IV length for this key type
> + * @rx_pn: Last received packet number, must be in little endian.
>   */
>  struct ieee80211_key_conf {
>  	atomic64_t tx_pn;
> @@ -1596,11 +1599,10 @@ struct ieee80211_key_conf {
>  	u8 flags;
>  	s8 keyidx;
>  	u8 keylen;
> +	u8 rx_pn[IEEE80211_MAX_PN_LEN];
>  	u8 key[0];
>  };

I don't think you should put that here, we keep this around in the key
structure forever. We can pass it out of band, I'd say?

> diff --git a/net/mac80211/key.c b/net/mac80211/key.c
> index 9380493..15e1822 100644
> --- a/net/mac80211/key.c
> +++ b/net/mac80211/key.c
> @@ -538,6 +538,12 @@ struct ieee80211_key *
>  	}
>  	memcpy(key->conf.key, key_data, key_len);
>  	INIT_LIST_HEAD(&key->list);
> +	/* Assign receive packet sequence no, rx_pn remains in
> +	 * little endian format as seq is guaranteed to be in little
> +	 * endian format.
> +	 */
> +	if (seq)
> +		memcpy(&key->conf.rx_pn, seq, seq_len);

It seems that perhaps we should make this more robust - there are
sometimes cases where the key can enable HW acceleration only after
some time. Not the case with ath10k though, but we should probably
handle that.

And once we do, we probably need this per TID, because the RX PN might
have moved around, or we look up the max over all TIDs. All the more
reason to pass it out of band.

Another reason to pass it out of band would be to tell the driver that
indeed it's *not* maintained over the lifetime of the key, unlike the
other fields which are maintained (because constant) over the lifetime
of the key.

johannes

Patch

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 2ee4af2..2f0c91d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1564,6 +1564,8 @@  enum ieee80211_key_flags {
 	IEEE80211_KEY_FLAG_RESERVE_TAILROOM	= BIT(7),
 };
 
+#define IEEE80211_MAX_PN_LEN	16
+
 /**
  * struct ieee80211_key_conf - key information
  *
@@ -1586,6 +1588,7 @@  enum ieee80211_key_flags {
  * 	- Temporal Authenticator Rx MIC Key (64 bits)
  * @icv_len: The ICV length for this key type
  * @iv_len: The IV length for this key type
+ * @rx_pn: Last received packet number, must be in little endian.
  */
 struct ieee80211_key_conf {
 	atomic64_t tx_pn;
@@ -1596,11 +1599,10 @@  struct ieee80211_key_conf {
 	u8 flags;
 	s8 keyidx;
 	u8 keylen;
+	u8 rx_pn[IEEE80211_MAX_PN_LEN];
 	u8 key[0];
 };
 
-#define IEEE80211_MAX_PN_LEN	16
-
 #define TKIP_PN_TO_IV16(pn) ((u16)(pn & 0xffff))
 #define TKIP_PN_TO_IV32(pn) ((u32)((pn >> 16) & 0xffffffff))
 
diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 9380493..15e1822 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -538,6 +538,12 @@  struct ieee80211_key *
 	}
 	memcpy(key->conf.key, key_data, key_len);
 	INIT_LIST_HEAD(&key->list);
+	/* Assign receive packet sequence no, rx_pn remains in
+	 * little endian format as seq is guaranteed to be in little
+	 * endian format.
+	 */
+	if (seq)
+		memcpy(&key->conf.rx_pn, seq, seq_len);
 
 	return key;
 }