Skip to contentSkip to conversation

macsec fixes

PATCH In review Sabrina Dubroca 3 participants 7 messages Jun 14, 2016 – Jun 15, 2016

Subsystems (3)
Networking Network drivers Networking macsec
lore
Share & tools
lore

Sabrina Dubroca

author maintainer
#0 lore raw

cover macsec fixes

Patch 1 adds rcu_barrier() during module unload to prevent possible panics.

Patch 2 allocates memory for scattergather lists and the IV on the heap, since they can escape the current function's context during crypto callbacks.

Patch 3 fixes a failure to create secure associations.

Sabrina Dubroca (3):
macsec: add rcu_barrier() on module exit
macsec: allocate sg and iv on the heap
macsec: fix SA initialization

 drivers/net/macsec.c | 49 +++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 39 insertions(+), 10 deletions(-)
Signature · 1 line
2.8.3
author maintainer
#1 lore raw

PATCH 1/3 macsec: add rcu_barrier() on module exit

Without this, the various uses of call_rcu could cause a kernel panic.

Fixes c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by Sabrina Dubroca <[email protected]>
---
 drivers/net/macsec.c | 1 +
 1 file changed, 1 insertion(+)
drivers/net/macsec.c +1 -0
index 47ee2c840b55..e80736f6acd7 100644
@@ -3361,6 +3361,7 @@ static void __exit macsec_exit(void)
 	genl_unregister_family(&macsec_fam);
 	rtnl_link_unregister(&macsec_link_ops);
 	unregister_netdevice_notifier(&macsec_notifier);
+	rcu_barrier();
 }
 
 module_init(macsec_init);
Signature · 1 line
2.8.3
author maintainer
#3 lore raw

PATCH 2/3 macsec: allocate sg and iv on the heap

In reply to Sabrina Dubroca

For the crypto callbacks to work properly, we cannot have sg and iv on the stack. Use kmalloc instead, with a single allocation for aead_request + scatterlist + iv.

Fixes c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by Sabrina Dubroca <[email protected]>
---
 drivers/net/macsec.c | 46 +++++++++++++++++++++++++++++++++++++---------
 1 file changed, 37 insertions(+), 9 deletions(-)
drivers/net/macsec.c +37 -9
index e80736f6acd7..189ea3e8e8a0 100644
@@ -605,12 +605,41 @@ static void macsec_encrypt_done(struct crypto_async_request *base, int err)
 	dev_put(dev);
 }
 
+static struct aead_request *macsec_alloc_req(struct crypto_aead *tfm,
+					     unsigned char **iv,
+					     struct scatterlist **sg)
+{
+	size_t size, iv_offset, sg_offset;
+	struct aead_request *req;
+	void *tmp;
+
+	size = sizeof(struct aead_request) + crypto_aead_reqsize(tfm);
+	iv_offset = size;
+	size += GCM_AES_IV_LEN;
+
+	size = ALIGN(size, __alignof__(struct scatterlist));
+	sg_offset = size;
+	size += sizeof(struct scatterlist) * (MAX_SKB_FRAGS + 1);
+
+	tmp = kmalloc(size, GFP_ATOMIC);
+	if (!tmp)
+		return NULL;
+
+	*iv = (unsigned char *)(tmp + iv_offset);
+	*sg = (struct scatterlist *)(tmp + sg_offset);
+	req = tmp;
+
+	aead_request_set_tfm(req, tfm);
+
+	return req;
+}
+
 static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
 				      struct net_device *dev)
 {
 	int ret;
-	struct scatterlist sg[MAX_SKB_FRAGS + 1];
-	unsigned char iv[GCM_AES_IV_LEN];
+	struct scatterlist *sg;
+	unsigned char *iv;
 	struct ethhdr *eth;
 	struct macsec_eth_header *hh;
 	size_t unprotected_len;

@@ -668,8 +697,6 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
 	macsec_fill_sectag(hh, secy, pn);
 	macsec_set_shortlen(hh, unprotected_len - 2 * ETH_ALEN);
 
-	macsec_fill_iv(iv, secy->sci, pn);
-
 	skb_put(skb, secy->icv_len);
 
 	if (skb->len - ETH_HLEN > macsec_priv(dev)->real_dev->mtu) {

@@ -684,13 +711,15 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
 		return ERR_PTR(-EINVAL);
 	}
 
-	req = aead_request_alloc(tx_sa->key.tfm, GFP_ATOMIC);
+	req = macsec_alloc_req(tx_sa->key.tfm, &iv, &sg);
 	if (!req) {
 		macsec_txsa_put(tx_sa);
 		kfree_skb(skb);
 		return ERR_PTR(-ENOMEM);
 	}
 
+	macsec_fill_iv(iv, secy->sci, pn);
+
 	sg_init_table(sg, MAX_SKB_FRAGS + 1);
 	skb_to_sgvec(skb, sg, 0, skb->len);
 

@@ -861,7 +890,6 @@ static void macsec_decrypt_done(struct crypto_async_request *base, int err)
 out:
 	macsec_rxsa_put(rx_sa);
 	dev_put(dev);
-	return;
 }
 
 static struct sk_buff *macsec_decrypt(struct sk_buff *skb,

@@ -871,8 +899,8 @@ static struct sk_buff *macsec_decrypt(struct sk_buff *skb,
 				      struct macsec_secy *secy)
 {
 	int ret;
-	struct scatterlist sg[MAX_SKB_FRAGS + 1];
-	unsigned char iv[GCM_AES_IV_LEN];
+	struct scatterlist *sg;
+	unsigned char *iv;
 	struct aead_request *req;
 	struct macsec_eth_header *hdr;
 	u16 icv_len = secy->icv_len;

@@ -882,7 +910,7 @@ static struct sk_buff *macsec_decrypt(struct sk_buff *skb,
 	if (!skb)
 		return ERR_PTR(-ENOMEM);
 
-	req = aead_request_alloc(rx_sa->key.tfm, GFP_ATOMIC);
+	req = macsec_alloc_req(rx_sa->key.tfm, &iv, &sg);
 	if (!req) {
 		kfree_skb(skb);
 		return ERR_PTR(-ENOMEM);
Signature · 1 line
2.8.3
author maintainer
#5 lore raw

PATCH 3/3 macsec: fix SA initialization

In reply to Sabrina Dubroca

The ASYNC flag prevents initialization on some physical machines.

Fixes c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver")
Signed-off-by Sabrina Dubroca <[email protected]>
---
 drivers/net/macsec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/macsec.c +1 -1
index 189ea3e8e8a0..0e7eff7f1cd2 100644
@@ -1262,7 +1262,7 @@ static struct crypto_aead *macsec_alloc_tfm(char *key, int key_len, int icv_len)
 	struct crypto_aead *tfm;
 	int ret;
 
-	tfm = crypto_alloc_aead("gcm(aes)", 0, CRYPTO_ALG_ASYNC);
+	tfm = crypto_alloc_aead("gcm(aes)", 0, 0);
 	if (!tfm || IS_ERR(tfm))
 		return NULL;
Signature · 1 line
2.8.3
maintainer
#6 lore raw
In reply to Sabrina Dubroca

From: Sabrina Dubroca <[email protected]>
Date: Tue, 14 Jun 2016 15:25:13 +0200

Quoted 8 lines

Patch 1 adds rcu_barrier() during module unload to prevent possible panics.

Patch 2 allocates memory for scattergather lists and the IV on the heap, since they can escape the current function's context during crypto callbacks.

Patch 3 fixes a failure to create secure associations.

Series applied.