Further fixes for 48a1df65334b

Further fixes for 48a1df65334b

[Nathan Chancellor]

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

[PATCH 3.18 1/3] rxrpc: check return value of skb_to_sgvec always

[Nathan Chancellor]

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;
-- 
2.17.0

[PATCH 3.18 2/3] virtio_net: check return value of skb_to_sgvec always

[Nathan Chancellor]

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);
 }
-- 
2.17.0

[PATCH 3.18 3/3] virtio_net: check return value of skb_to_sgvec in one more location

[Nathan Chancellor]

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)
-- 
2.17.0

[PATCH 4.4 1/3] rxrpc: check return value of skb_to_sgvec always

[Nathan Chancellor]

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];
-- 
2.17.0

[PATCH 4.4 2/3] virtio_net: check return value of skb_to_sgvec always

[Nathan Chancellor]

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);
 }
-- 
2.17.0

[PATCH 4.4 3/3] virtio_net: check return value of skb_to_sgvec in one more location

[Nathan Chancellor]

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)
-- 
2.17.0

[PATCH 4.9 1/3] rxrpc: check return value of skb_to_sgvec always

[Nathan Chancellor]

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];
-- 
2.17.0

[PATCH 4.9 2/3] virtio_net: check return value of skb_to_sgvec always

[Nathan Chancellor]

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);
 }
-- 
2.17.0

[PATCH 4.9 3/3] virtio_net: check return value of skb_to_sgvec in one more location

[Nathan Chancellor]

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)
-- 
2.17.0

Re: [PATCH 4.9 3/3] virtio_net: check return value of skb_to_sgvec in one more location

[Greg Kroah-Hartman]

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

Re: [PATCH 4.9 3/3] virtio_net: check return value of skb_to_sgvec in one more location

[Greg Kroah-Hartman]

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

Patch "virtio_net: check return value of skb_to_sgvec in one more location" has been added to the 4.9-stable tree

[gregkh]

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;a=summary

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

Patch "virtio_net: check return value of skb_to_sgvec in one more location" has been added to the 4.4-stable tree

[gregkh]

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;a=summary

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

Patch "virtio_net: check return value of skb_to_sgvec in one more location" has been added to the 3.18-stable tree

[gregkh]

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;a=summary

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

Re: Further fixes for 48a1df65334b

[Greg Kroah-Hartman]

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