The backport of 48a1df65334b ("skbuff: return -EMSGSIZE in skb_to_sgvec to prevent overflow") was missing three supplemental commits:
3f29770723fe ("ipsec: check return value of skb_to_sgvec always") 89a5ea996625 ("rxrpc: check return value of skb_to_sgvec always") e2fcad58fd23 ("virtio_net: check return value of skb_to_sgvec always")
I already sent the first one, this thread contains the other two for 3.18, 4.4, and 4.9. There is an additional patch because these trees do not contain f6b10209b90d ("virtio-net: switch to use build_skb() for small buffer"), which removed another call to skb_to_sgvec, which was before f6b10209b90d. If you feel it should be squashed in f6b10209b90d, please by all means feel free to do so. I was not 100% confident on if the dev_kfree_skb call was necessary in the error path so I hope one of the authors can comment on that.
Thanks! Nathan
From: "Jason A. Donenfeld" Jason@zx2c4.com
commit 89a5ea99662505d2d61f2a3030a6896c2cb3cdb0 upstream.
Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com Acked-by: David Howells dhowells@redhat.com Signed-off-by: David S. Miller davem@davemloft.net [natechancellor: backport to 3.18] Signed-off-by: Nathan Chancellor natechancellor@gmail.com --- net/rxrpc/rxkad.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c index f226709ebd8f..ca5f3662a485 100644 --- a/net/rxrpc/rxkad.c +++ b/net/rxrpc/rxkad.c @@ -209,7 +209,7 @@ static int rxkad_secure_packet_encrypt(const struct rxrpc_call *call, struct sk_buff *trailer; unsigned int len; u16 check; - int nsg; + int nsg, err;
sp = rxrpc_skb(skb);
@@ -240,7 +240,9 @@ static int rxkad_secure_packet_encrypt(const struct rxrpc_call *call, len &= ~(call->conn->size_align - 1);
sg_init_table(sg, nsg); - skb_to_sgvec(skb, sg, 0, len); + err = skb_to_sgvec(skb, sg, 0, len); + if (unlikely(err < 0)) + return err; crypto_blkcipher_encrypt_iv(&desc, sg, sg, len);
_leave(" = 0"); @@ -336,7 +338,7 @@ static int rxkad_verify_packet_auth(const struct rxrpc_call *call, struct sk_buff *trailer; u32 data_size, buf; u16 check; - int nsg; + int nsg, ret;
_enter("");
@@ -348,7 +350,9 @@ static int rxkad_verify_packet_auth(const struct rxrpc_call *call, goto nomem;
sg_init_table(sg, nsg); - skb_to_sgvec(skb, sg, 0, 8); + ret = skb_to_sgvec(skb, sg, 0, 8); + if (unlikely(ret < 0)) + return ret;
/* start the decryption afresh */ memset(&iv, 0, sizeof(iv)); @@ -411,7 +415,7 @@ static int rxkad_verify_packet_encrypt(const struct rxrpc_call *call, struct sk_buff *trailer; u32 data_size, buf; u16 check; - int nsg; + int nsg, ret;
_enter(",{%d}", skb->len);
@@ -430,7 +434,12 @@ static int rxkad_verify_packet_encrypt(const struct rxrpc_call *call, }
sg_init_table(sg, nsg); - skb_to_sgvec(skb, sg, 0, skb->len); + ret = skb_to_sgvec(skb, sg, 0, skb->len); + if (unlikely(ret < 0)) { + if (sg != _sg) + kfree(sg); + return ret; + }
/* decrypt from the session key */ token = call->conn->key->payload.data;
From: "Jason A. Donenfeld" Jason@zx2c4.com
commit e2fcad58fd230f635a74e4e983c6f4ea893642d2 upstream.
Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com Reviewed-by: Sergei Shtylyov sergei.shtylyov@cogentembedded.com Cc: "Michael S. Tsirkin" mst@redhat.com Cc: Jason Wang jasowang@redhat.com Signed-off-by: David S. Miller davem@davemloft.net [natechancellor: backport to 3.18] Signed-off-by: Nathan Chancellor natechancellor@gmail.com --- drivers/net/virtio_net.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index ce2a29971230..ede40841e962 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -854,7 +854,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb) struct skb_vnet_hdr *hdr; const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest; struct virtnet_info *vi = sq->vq->vdev->priv; - unsigned num_sg; + int num_sg; unsigned hdr_len; bool can_push;
@@ -906,11 +906,16 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb) if (can_push) { __skb_push(skb, hdr_len); num_sg = skb_to_sgvec(skb, sq->sg, 0, skb->len); + if (unlikely(num_sg < 0)) + return num_sg; /* Pull header back to avoid skew in tx bytes calculations. */ __skb_pull(skb, hdr_len); } else { sg_set_buf(sq->sg, hdr, hdr_len); - num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1; + num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len); + if (unlikely(num_sg < 0)) + return num_sg; + num_sg++; } return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb, GFP_ATOMIC); }
Kernels that do not have f6b10209b90d ("virtio-net: switch to use build_skb() for small buffer") will have an extra call to skb_to_sgvec that is not handled by e2fcad58fd23 ("virtio_net: check return value of skb_to_sgvec always"). Since the former does not appear to be stable material, just fix the call up directly.
Cc: Jason A. Donenfeld Jason@zx2c4.com Cc: Sergei Shtylyov sergei.shtylyov@cogentembedded.com Cc: "Michael S. Tsirkin" mst@redhat.com Cc: Jason Wang jasowang@redhat.com Cc: David S. Miller davem@davemloft.net Signed-off-by: Nathan Chancellor natechancellor@gmail.com ---
Greg, if you feel this should be squashed into the previous commit, by all means, feel free. I am not entirely sure if the dev_kfree_skb is necessary here, I was hoping to get some review.
--- drivers/net/virtio_net.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index ede40841e962..1da54735681b 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -551,7 +551,12 @@ static int add_recvbuf_small(struct receive_queue *rq, gfp_t gfp) hdr = skb_vnet_hdr(skb); sg_init_table(rq->sg, MAX_SKB_FRAGS + 2); sg_set_buf(rq->sg, &hdr->hdr, sizeof hdr->hdr); - skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); + + err = skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); + if (unlikely(err < 0)) { + dev_kfree_skb(skb); + return err; + }
err = virtqueue_add_inbuf(rq->vq, rq->sg, 2, skb, gfp); if (err < 0)
This is a note to let you know that I've just added the patch titled
virtio_net: check return value of skb_to_sgvec in one more location
to the 3.18-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: virtio_net-check-return-value-of-skb_to_sgvec-in-one-more-location.patch and it can be found in the queue-3.18 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From natechancellor@gmail.com Tue Apr 10 14:13:23 2018
From: Nathan Chancellor natechancellor@gmail.com Date: Mon, 9 Apr 2018 18:21:44 -0700 Subject: virtio_net: check return value of skb_to_sgvec in one more location To: Greg Kroah-Hartman gregkh@linuxfoundation.org, stable@vger.kernel.org Cc: Nathan Chancellor natechancellor@gmail.com, "Jason A . Donenfeld" Jason@zx2c4.com, Sergei Shtylyov sergei.shtylyov@cogentembedded.com, "Michael S. Tsirkin" mst@redhat.com, Jason Wang jasowang@redhat.com, "David S . Miller" davem@davemloft.net Message-ID: 20180410012150.6573-4-natechancellor@gmail.com
From: Nathan Chancellor natechancellor@gmail.com
Kernels that do not have f6b10209b90d ("virtio-net: switch to use build_skb() for small buffer") will have an extra call to skb_to_sgvec that is not handled by e2fcad58fd23 ("virtio_net: check return value of skb_to_sgvec always"). Since the former does not appear to be stable material, just fix the call up directly.
Cc: Jason A. Donenfeld Jason@zx2c4.com Cc: Sergei Shtylyov sergei.shtylyov@cogentembedded.com Cc: "Michael S. Tsirkin" mst@redhat.com Cc: Jason Wang jasowang@redhat.com Cc: David S. Miller davem@davemloft.net Signed-off-by: Nathan Chancellor natechancellor@gmail.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- drivers/net/virtio_net.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
--- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -551,7 +551,12 @@ static int add_recvbuf_small(struct rece hdr = skb_vnet_hdr(skb); sg_init_table(rq->sg, MAX_SKB_FRAGS + 2); sg_set_buf(rq->sg, &hdr->hdr, sizeof hdr->hdr); - skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); + + err = skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); + if (unlikely(err < 0)) { + dev_kfree_skb(skb); + return err; + }
err = virtqueue_add_inbuf(rq->vq, rq->sg, 2, skb, gfp); if (err < 0)
Patches currently in stable-queue which might be from natechancellor@gmail.com are
queue-3.18/ipsec-check-return-value-of-skb_to_sgvec-always.patch queue-3.18/rxrpc-check-return-value-of-skb_to_sgvec-always.patch queue-3.18/virtio_net-check-return-value-of-skb_to_sgvec-always.patch queue-3.18/virtio_net-check-return-value-of-skb_to_sgvec-in-one-more-location.patch
From: "Jason A. Donenfeld" Jason@zx2c4.com
commit 89a5ea99662505d2d61f2a3030a6896c2cb3cdb0 upstream.
Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com Acked-by: David Howells dhowells@redhat.com Signed-off-by: David S. Miller davem@davemloft.net [natechancellor: backport to 4.4] Signed-off-by: Nathan Chancellor natechancellor@gmail.com --- net/rxrpc/rxkad.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c index d7a9ab5a9d9c..6c65fb229e50 100644 --- a/net/rxrpc/rxkad.c +++ b/net/rxrpc/rxkad.c @@ -209,7 +209,7 @@ static int rxkad_secure_packet_encrypt(const struct rxrpc_call *call, struct sk_buff *trailer; unsigned int len; u16 check; - int nsg; + int nsg, err;
sp = rxrpc_skb(skb);
@@ -240,7 +240,9 @@ static int rxkad_secure_packet_encrypt(const struct rxrpc_call *call, len &= ~(call->conn->size_align - 1);
sg_init_table(sg, nsg); - skb_to_sgvec(skb, sg, 0, len); + err = skb_to_sgvec(skb, sg, 0, len); + if (unlikely(err < 0)) + return err; crypto_blkcipher_encrypt_iv(&desc, sg, sg, len);
_leave(" = 0"); @@ -336,7 +338,7 @@ static int rxkad_verify_packet_auth(const struct rxrpc_call *call, struct sk_buff *trailer; u32 data_size, buf; u16 check; - int nsg; + int nsg, ret;
_enter("");
@@ -348,7 +350,9 @@ static int rxkad_verify_packet_auth(const struct rxrpc_call *call, goto nomem;
sg_init_table(sg, nsg); - skb_to_sgvec(skb, sg, 0, 8); + ret = skb_to_sgvec(skb, sg, 0, 8); + if (unlikely(ret < 0)) + return ret;
/* start the decryption afresh */ memset(&iv, 0, sizeof(iv)); @@ -411,7 +415,7 @@ static int rxkad_verify_packet_encrypt(const struct rxrpc_call *call, struct sk_buff *trailer; u32 data_size, buf; u16 check; - int nsg; + int nsg, ret;
_enter(",{%d}", skb->len);
@@ -430,7 +434,12 @@ static int rxkad_verify_packet_encrypt(const struct rxrpc_call *call, }
sg_init_table(sg, nsg); - skb_to_sgvec(skb, sg, 0, skb->len); + ret = skb_to_sgvec(skb, sg, 0, skb->len); + if (unlikely(ret < 0)) { + if (sg != _sg) + kfree(sg); + return ret; + }
/* decrypt from the session key */ token = call->conn->key->payload.data[0];
From: "Jason A. Donenfeld" Jason@zx2c4.com
commit e2fcad58fd230f635a74e4e983c6f4ea893642d2 upstream.
Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com Reviewed-by: Sergei Shtylyov sergei.shtylyov@cogentembedded.com Cc: "Michael S. Tsirkin" mst@redhat.com Cc: Jason Wang jasowang@redhat.com Signed-off-by: David S. Miller davem@davemloft.net [natechancellor: backport to 4.4] Signed-off-by: Nathan Chancellor natechancellor@gmail.com --- drivers/net/virtio_net.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 8dfc75250583..bb2bae522f67 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -858,7 +858,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb) struct virtio_net_hdr_mrg_rxbuf *hdr; const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest; struct virtnet_info *vi = sq->vq->vdev->priv; - unsigned num_sg; + int num_sg; unsigned hdr_len = vi->hdr_len; bool can_push;
@@ -911,11 +911,16 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb) if (can_push) { __skb_push(skb, hdr_len); num_sg = skb_to_sgvec(skb, sq->sg, 0, skb->len); + if (unlikely(num_sg < 0)) + return num_sg; /* Pull header back to avoid skew in tx bytes calculations. */ __skb_pull(skb, hdr_len); } else { sg_set_buf(sq->sg, hdr, hdr_len); - num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1; + num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len); + if (unlikely(num_sg < 0)) + return num_sg; + num_sg++; } return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb, GFP_ATOMIC); }
Kernels that do not have f6b10209b90d ("virtio-net: switch to use build_skb() for small buffer") will have an extra call to skb_to_sgvec that is not handled by e2fcad58fd23 ("virtio_net: check return value of skb_to_sgvec always"). Since the former does not appear to be stable material, just fix the call up directly.
Cc: Jason A. Donenfeld Jason@zx2c4.com Cc: Sergei Shtylyov sergei.shtylyov@cogentembedded.com Cc: "Michael S. Tsirkin" mst@redhat.com Cc: Jason Wang jasowang@redhat.com Cc: David S. Miller davem@davemloft.net Signed-off-by: Nathan Chancellor natechancellor@gmail.com ---
Greg, if you feel this should be squashed into the previous commit, by all means, feel free. I am not entirely sure if the dev_kfree_skb is necessary here, I was hoping to get some review.
--- drivers/net/virtio_net.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index bb2bae522f67..d01285250204 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -556,7 +556,12 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct receive_queue *rq, hdr = skb_vnet_hdr(skb); sg_init_table(rq->sg, 2); sg_set_buf(rq->sg, hdr, vi->hdr_len); - skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); + + err = skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); + if (unlikely(err < 0)) { + dev_kfree_skb(skb); + return err; + }
err = virtqueue_add_inbuf(rq->vq, rq->sg, 2, skb, gfp); if (err < 0)
This is a note to let you know that I've just added the patch titled
virtio_net: check return value of skb_to_sgvec in one more location
to the 4.4-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: virtio_net-check-return-value-of-skb_to_sgvec-in-one-more-location.patch and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From natechancellor@gmail.com Tue Apr 10 10:36:56 2018
From: Nathan Chancellor natechancellor@gmail.com Date: Mon, 9 Apr 2018 18:21:47 -0700 Subject: virtio_net: check return value of skb_to_sgvec in one more location To: Greg Kroah-Hartman gregkh@linuxfoundation.org, stable@vger.kernel.org Cc: Nathan Chancellor natechancellor@gmail.com, "Jason A . Donenfeld" Jason@zx2c4.com, Sergei Shtylyov sergei.shtylyov@cogentembedded.com, "Michael S. Tsirkin" mst@redhat.com, Jason Wang jasowang@redhat.com, "David S . Miller" davem@davemloft.net Message-ID: 20180410012150.6573-7-natechancellor@gmail.com
From: Nathan Chancellor natechancellor@gmail.com
Kernels that do not have f6b10209b90d ("virtio-net: switch to use build_skb() for small buffer") will have an extra call to skb_to_sgvec that is not handled by e2fcad58fd23 ("virtio_net: check return value of skb_to_sgvec always"). Since the former does not appear to be stable material, just fix the call up directly.
Cc: Jason A. Donenfeld Jason@zx2c4.com Cc: Sergei Shtylyov sergei.shtylyov@cogentembedded.com Cc: "Michael S. Tsirkin" mst@redhat.com Cc: Jason Wang jasowang@redhat.com Cc: David S. Miller davem@davemloft.net Signed-off-by: Nathan Chancellor natechancellor@gmail.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- drivers/net/virtio_net.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
--- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -556,7 +556,12 @@ static int add_recvbuf_small(struct virt hdr = skb_vnet_hdr(skb); sg_init_table(rq->sg, 2); sg_set_buf(rq->sg, hdr, vi->hdr_len); - skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); + + err = skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); + if (unlikely(err < 0)) { + dev_kfree_skb(skb); + return err; + }
err = virtqueue_add_inbuf(rq->vq, rq->sg, 2, skb, gfp); if (err < 0)
Patches currently in stable-queue which might be from natechancellor@gmail.com are
queue-4.4/ipsec-check-return-value-of-skb_to_sgvec-always.patch queue-4.4/rxrpc-check-return-value-of-skb_to_sgvec-always.patch queue-4.4/virtio_net-check-return-value-of-skb_to_sgvec-always.patch queue-4.4/virtio_net-check-return-value-of-skb_to_sgvec-in-one-more-location.patch
From: "Jason A. Donenfeld" Jason@zx2c4.com
commit 89a5ea99662505d2d61f2a3030a6896c2cb3cdb0 upstream.
Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com Acked-by: David Howells dhowells@redhat.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Nathan Chancellor natechancellor@gmail.com --- net/rxrpc/rxkad.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c index 4374e7b9c7bf..90df95e18ea4 100644 --- a/net/rxrpc/rxkad.c +++ b/net/rxrpc/rxkad.c @@ -229,7 +229,9 @@ static int rxkad_secure_packet_encrypt(const struct rxrpc_call *call, len &= ~(call->conn->size_align - 1);
sg_init_table(sg, nsg); - skb_to_sgvec(skb, sg, 0, len); + err = skb_to_sgvec(skb, sg, 0, len); + if (unlikely(err < 0)) + goto out; skcipher_request_set_crypt(req, sg, sg, len, iv.x); crypto_skcipher_encrypt(req);
@@ -325,7 +327,7 @@ static int rxkad_verify_packet_1(struct rxrpc_call *call, struct sk_buff *skb, struct sk_buff *trailer; u32 data_size, buf; u16 check; - int nsg; + int nsg, ret;
_enter("");
@@ -342,7 +344,9 @@ static int rxkad_verify_packet_1(struct rxrpc_call *call, struct sk_buff *skb, goto nomem;
sg_init_table(sg, nsg); - skb_to_sgvec(skb, sg, offset, 8); + ret = skb_to_sgvec(skb, sg, offset, 8); + if (unlikely(ret < 0)) + return ret;
/* start the decryption afresh */ memset(&iv, 0, sizeof(iv)); @@ -405,7 +409,7 @@ static int rxkad_verify_packet_2(struct rxrpc_call *call, struct sk_buff *skb, struct sk_buff *trailer; u32 data_size, buf; u16 check; - int nsg; + int nsg, ret;
_enter(",{%d}", skb->len);
@@ -429,7 +433,12 @@ static int rxkad_verify_packet_2(struct rxrpc_call *call, struct sk_buff *skb, }
sg_init_table(sg, nsg); - skb_to_sgvec(skb, sg, offset, len); + ret = skb_to_sgvec(skb, sg, offset, len); + if (unlikely(ret < 0)) { + if (sg != _sg) + kfree(sg); + return ret; + }
/* decrypt from the session key */ token = call->conn->params.key->payload.data[0];
From: "Jason A. Donenfeld" Jason@zx2c4.com
commit e2fcad58fd230f635a74e4e983c6f4ea893642d2 upstream.
Signed-off-by: Jason A. Donenfeld Jason@zx2c4.com Reviewed-by: Sergei Shtylyov sergei.shtylyov@cogentembedded.com Cc: "Michael S. Tsirkin" mst@redhat.com Cc: Jason Wang jasowang@redhat.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Nathan Chancellor natechancellor@gmail.com --- drivers/net/virtio_net.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 1568aedddfc9..0a2f3ac36be2 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -831,7 +831,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb) struct virtio_net_hdr_mrg_rxbuf *hdr; const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest; struct virtnet_info *vi = sq->vq->vdev->priv; - unsigned num_sg; + int num_sg; unsigned hdr_len = vi->hdr_len; bool can_push;
@@ -858,11 +858,16 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb) if (can_push) { __skb_push(skb, hdr_len); num_sg = skb_to_sgvec(skb, sq->sg, 0, skb->len); + if (unlikely(num_sg < 0)) + return num_sg; /* Pull header back to avoid skew in tx bytes calculations. */ __skb_pull(skb, hdr_len); } else { sg_set_buf(sq->sg, hdr, hdr_len); - num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1; + num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len); + if (unlikely(num_sg < 0)) + return num_sg; + num_sg++; } return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb, GFP_ATOMIC); }
Kernels that do not have f6b10209b90d ("virtio-net: switch to use build_skb() for small buffer") will have an extra call to skb_to_sgvec that is not handled by e2fcad58fd23 ("virtio_net: check return value of skb_to_sgvec always"). Since the former does not appear to be stable material, just fix the call up directly.
Cc: Jason A. Donenfeld Jason@zx2c4.com Cc: Sergei Shtylyov sergei.shtylyov@cogentembedded.com Cc: "Michael S. Tsirkin" mst@redhat.com Cc: Jason Wang jasowang@redhat.com Cc: David S. Miller davem@davemloft.net Signed-off-by: Nathan Chancellor natechancellor@gmail.com ---
Greg, if you feel this should be squashed into the previous commit, by all means, feel free. I am not entirely sure if the dev_kfree_skb is necessary here, I was hoping to get some review.
--- drivers/net/virtio_net.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 0a2f3ac36be2..472ed6df2221 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -529,7 +529,12 @@ static int add_recvbuf_small(struct virtnet_info *vi, struct receive_queue *rq, hdr = skb_vnet_hdr(skb); sg_init_table(rq->sg, 2); sg_set_buf(rq->sg, hdr, vi->hdr_len); - skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); + + err = skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); + if (unlikely(err < 0)) { + dev_kfree_skb(skb); + return err; + }
err = virtqueue_add_inbuf(rq->vq, rq->sg, 2, skb, gfp); if (err < 0)
On Mon, Apr 09, 2018 at 06:21:50PM -0700, Nathan Chancellor wrote:
Kernels that do not have f6b10209b90d ("virtio-net: switch to use build_skb() for small buffer") will have an extra call to skb_to_sgvec that is not handled by e2fcad58fd23 ("virtio_net: check return value of skb_to_sgvec always"). Since the former does not appear to be stable material, just fix the call up directly.
Cc: Jason A. Donenfeld Jason@zx2c4.com Cc: Sergei Shtylyov sergei.shtylyov@cogentembedded.com Cc: "Michael S. Tsirkin" mst@redhat.com Cc: Jason Wang jasowang@redhat.com Cc: David S. Miller davem@davemloft.net Signed-off-by: Nathan Chancellor natechancellor@gmail.com
Greg, if you feel this should be squashed into the previous commit, by all means, feel free. I am not entirely sure if the dev_kfree_skb is necessary here, I was hoping to get some review.
You did the right thing here by "not squashing" this into the previous patch, thanks!
greg k-h
On Tue, Apr 10, 2018 at 08:35:37AM +0200, Greg Kroah-Hartman wrote:
On Mon, Apr 09, 2018 at 06:21:50PM -0700, Nathan Chancellor wrote:
Kernels that do not have f6b10209b90d ("virtio-net: switch to use build_skb() for small buffer") will have an extra call to skb_to_sgvec that is not handled by e2fcad58fd23 ("virtio_net: check return value of skb_to_sgvec always"). Since the former does not appear to be stable material, just fix the call up directly.
Cc: Jason A. Donenfeld Jason@zx2c4.com Cc: Sergei Shtylyov sergei.shtylyov@cogentembedded.com Cc: "Michael S. Tsirkin" mst@redhat.com Cc: Jason Wang jasowang@redhat.com Cc: David S. Miller davem@davemloft.net Signed-off-by: Nathan Chancellor natechancellor@gmail.com
Greg, if you feel this should be squashed into the previous commit, by all means, feel free. I am not entirely sure if the dev_kfree_skb is necessary here, I was hoping to get some review.
You did the right thing here by "not squashing" this into the previous patch, thanks!
And all 3 of these fixed up the remaining 4.9.y build issues, many thanks for these.
greg k-h
This is a note to let you know that I've just added the patch titled
virtio_net: check return value of skb_to_sgvec in one more location
to the 4.9-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git%3Ba=su...
The filename of the patch is: virtio_net-check-return-value-of-skb_to_sgvec-in-one-more-location.patch and it can be found in the queue-4.9 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let stable@vger.kernel.org know about it.
From natechancellor@gmail.com Tue Apr 10 08:34:50 2018
From: Nathan Chancellor natechancellor@gmail.com Date: Mon, 9 Apr 2018 18:21:50 -0700 Subject: virtio_net: check return value of skb_to_sgvec in one more location To: Greg Kroah-Hartman gregkh@linuxfoundation.org, stable@vger.kernel.org Cc: Nathan Chancellor natechancellor@gmail.com, "Jason A . Donenfeld" Jason@zx2c4.com, Sergei Shtylyov sergei.shtylyov@cogentembedded.com, "Michael S. Tsirkin" mst@redhat.com, Jason Wang jasowang@redhat.com, "David S . Miller" davem@davemloft.net Message-ID: 20180410012150.6573-10-natechancellor@gmail.com
From: Nathan Chancellor natechancellor@gmail.com
Kernels that do not have f6b10209b90d ("virtio-net: switch to use build_skb() for small buffer") will have an extra call to skb_to_sgvec that is not handled by e2fcad58fd23 ("virtio_net: check return value of skb_to_sgvec always"). Since the former does not appear to be stable material, just fix the call up directly.
Cc: Jason A. Donenfeld Jason@zx2c4.com Cc: Sergei Shtylyov sergei.shtylyov@cogentembedded.com Cc: "Michael S. Tsirkin" mst@redhat.com Cc: Jason Wang jasowang@redhat.com Cc: David S. Miller davem@davemloft.net Signed-off-by: Nathan Chancellor natechancellor@gmail.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
--- drivers/net/virtio_net.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
--- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -529,7 +529,12 @@ static int add_recvbuf_small(struct virt hdr = skb_vnet_hdr(skb); sg_init_table(rq->sg, 2); sg_set_buf(rq->sg, hdr, vi->hdr_len); - skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); + + err = skb_to_sgvec(skb, rq->sg + 1, 0, skb->len); + if (unlikely(err < 0)) { + dev_kfree_skb(skb); + return err; + }
err = virtqueue_add_inbuf(rq->vq, rq->sg, 2, skb, gfp); if (err < 0)
Patches currently in stable-queue which might be from natechancellor@gmail.com are
queue-4.9/ipsec-check-return-value-of-skb_to_sgvec-always.patch queue-4.9/rxrpc-check-return-value-of-skb_to_sgvec-always.patch queue-4.9/virtio_net-check-return-value-of-skb_to_sgvec-always.patch queue-4.9/virtio_net-check-return-value-of-skb_to_sgvec-in-one-more-location.patch
On Mon, Apr 09, 2018 at 06:21:41PM -0700, Nathan Chancellor wrote:
The backport of 48a1df65334b ("skbuff: return -EMSGSIZE in skb_to_sgvec to prevent overflow") was missing three supplemental commits:
3f29770723fe ("ipsec: check return value of skb_to_sgvec always") 89a5ea996625 ("rxrpc: check return value of skb_to_sgvec always") e2fcad58fd23 ("virtio_net: check return value of skb_to_sgvec always")
I already sent the first one, this thread contains the other two for 3.18, 4.4, and 4.9. There is an additional patch because these trees do not contain f6b10209b90d ("virtio-net: switch to use build_skb() for small buffer"), which removed another call to skb_to_sgvec, which was before f6b10209b90d. If you feel it should be squashed in f6b10209b90d, please by all means feel free to do so. I was not 100% confident on if the dev_kfree_skb call was necessary in the error path so I hope one of the authors can comment on that.
Thanks for all of these, now queued up.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg Kroah-Hartman -
gregkh＠linuxfoundation.org
-
Nathan Chancellor