The previous fix for this issue, commit 4d6fa57b4dab ("macsec: avoid heap overflow in skb_to_sgvec"), doesn't really fix much. It removed the NETIF_F_FRAGLIST flag from MACsec device features, but this flag isn't checked anywhere in the codepaths leading to a macsec_decrypt() call.
On TX, macsec could already handle a frag_list because an skb with a frag_list will get linearized in skb_copy_expand() since it lacks the necessary tailroom. Removing the NETIF_F_FRAGLIST makes sure macsec_encrypt() will never see a frag_list. On RX, we can simply get the number of necessary scatterlist items by calling skb_cow_data(). Fixes: c09440f7dcb3 ("macsec: introduce IEEE 802.1AE driver") Fixes: CVE-2017-7477 Reported-by: "Jason A. Donenfeld" <ja...@zx2c4.com> Signed-off-by: Sabrina Dubroca <s...@queasysnail.net> --- drivers/net/macsec.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c index dbab05afcdbe..c9cc40d2349c 100644 --- a/drivers/net/macsec.c +++ b/drivers/net/macsec.c @@ -617,7 +617,8 @@ static void macsec_encrypt_done(struct crypto_async_request *base, int err) static struct aead_request *macsec_alloc_req(struct crypto_aead *tfm, unsigned char **iv, - struct scatterlist **sg) + struct scatterlist **sg, + int nfrags) { size_t size, iv_offset, sg_offset; struct aead_request *req; @@ -629,7 +630,7 @@ static struct aead_request *macsec_alloc_req(struct crypto_aead *tfm, size = ALIGN(size, __alignof__(struct scatterlist)); sg_offset = size; - size += sizeof(struct scatterlist) * (MAX_SKB_FRAGS + 1); + size += sizeof(struct scatterlist) * nfrags; tmp = kmalloc(size, GFP_ATOMIC); if (!tmp) @@ -723,7 +724,7 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb, return ERR_PTR(-EINVAL); } - req = macsec_alloc_req(tx_sa->key.tfm, &iv, &sg); + req = macsec_alloc_req(tx_sa->key.tfm, &iv, &sg, MAX_SKB_FRAGS + 1); if (!req) { macsec_txsa_put(tx_sa); kfree_skb(skb); @@ -921,13 +922,21 @@ static struct sk_buff *macsec_decrypt(struct sk_buff *skb, struct aead_request *req; struct macsec_eth_header *hdr; u16 icv_len = secy->icv_len; + struct sk_buff *trailer; + int nfrags; macsec_skb_cb(skb)->valid = false; skb = skb_share_check(skb, GFP_ATOMIC); if (!skb) return ERR_PTR(-ENOMEM); - req = macsec_alloc_req(rx_sa->key.tfm, &iv, &sg); + nfrags = skb_cow_data(skb, 0, &trailer); + if (nfrags < 0) { + kfree_skb(skb); + return ERR_PTR(nfrags); + } + + req = macsec_alloc_req(rx_sa->key.tfm, &iv, &sg, nfrags); if (!req) { kfree_skb(skb); return ERR_PTR(-ENOMEM); @@ -936,7 +945,7 @@ static struct sk_buff *macsec_decrypt(struct sk_buff *skb, hdr = (struct macsec_eth_header *)skb->data; macsec_fill_iv(iv, sci, ntohl(hdr->packet_number)); - sg_init_table(sg, MAX_SKB_FRAGS + 1); + sg_init_table(sg, nfrags); skb_to_sgvec(skb, sg, 0, skb->len); if (hdr->tci_an & MACSEC_TCI_E) { -- 2.12.2