public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/2] vsock/virtio: fix MSG_PEEK calculation on bytes to copy
@ 2026-04-02  8:18 Luigi Leonardi
  2026-04-02  8:18 ` [PATCH net 1/2] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating " Luigi Leonardi
  2026-04-02  8:18 ` [PATCH net 2/2] vsock/test: add MSG_PEEK after partial recv test Luigi Leonardi
  0 siblings, 2 replies; 9+ messages in thread
From: Luigi Leonardi @ 2026-04-02  8:18 UTC (permalink / raw)
  To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
	Jason Wang, Xuan Zhuo, Eugenio Pérez, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Arseniy Krasnov
  Cc: kvm, virtualization, netdev, linux-kernel, Luigi Leonardi

`virtio_transport_stream_do_peek`, when calculating the number of bytes to copy,
didn't consider the `offset`, caused by partial reads that happend before.
This might cause out-of-bounds read that lead to an EFAULT.
More details in the commit.

Commit 1 introduces the fix
Commit 2 introduces a test that checks for this bug to avoid future
regressions.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
Luigi Leonardi (2):
      vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating bytes to copy
      vsock/test: add MSG_PEEK after partial recv test

 net/vmw_vsock/virtio_transport_common.c |  5 ++-
 tools/testing/vsock/vsock_test.c        | 64 +++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 3 deletions(-)
---
base-commit: 9147566d801602c9e7fc7f85e989735735bf38ba
change-id: 20260401-fix_peek-6837b83469e3

Best regards,
-- 
Luigi Leonardi <leonardi@redhat.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH net 1/2] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating bytes to copy
  2026-04-02  8:18 [PATCH net 0/2] vsock/virtio: fix MSG_PEEK calculation on bytes to copy Luigi Leonardi
@ 2026-04-02  8:18 ` Luigi Leonardi
  2026-04-02 13:08   ` Stefano Garzarella
  2026-04-05 19:22   ` Arseniy Krasnov
  2026-04-02  8:18 ` [PATCH net 2/2] vsock/test: add MSG_PEEK after partial recv test Luigi Leonardi
  1 sibling, 2 replies; 9+ messages in thread
From: Luigi Leonardi @ 2026-04-02  8:18 UTC (permalink / raw)
  To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
	Jason Wang, Xuan Zhuo, Eugenio Pérez, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Arseniy Krasnov
  Cc: kvm, virtualization, netdev, linux-kernel, Luigi Leonardi

`virtio_transport_stream_do_peek()` does not account for the skb offset
when computing the number of bytes to copy.

This means that, after a partial recv() that advances the offset, a peek
requesting more bytes than are available in the sk_buff causes
`skb_copy_datagram_iter()` to go past the valid payload, resulting in a -EFAULT.

The dequeue path already handles this correctly.
Apply the same logic to the peek path.

Fixes: 0df7cd3c13e4 ("vsock/virtio/vhost: read data from non-linear skb")
Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
 net/vmw_vsock/virtio_transport_common.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index 8a9fb23c6e853dfea0a24d3787f7d6eb351dd6c6..4b65bfe5d875111f115e0fc4c6727adb66f34830 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -547,9 +547,8 @@ virtio_transport_stream_do_peek(struct vsock_sock *vsk,
 	skb_queue_walk(&vvs->rx_queue, skb) {
 		size_t bytes;
 
-		bytes = len - total;
-		if (bytes > skb->len)
-			bytes = skb->len;
+		bytes = min_t(size_t, len - total,
+			      skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset);
 
 		spin_unlock_bh(&vvs->rx_lock);
 

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH net 2/2] vsock/test: add MSG_PEEK after partial recv test
  2026-04-02  8:18 [PATCH net 0/2] vsock/virtio: fix MSG_PEEK calculation on bytes to copy Luigi Leonardi
  2026-04-02  8:18 ` [PATCH net 1/2] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating " Luigi Leonardi
@ 2026-04-02  8:18 ` Luigi Leonardi
  2026-04-02 13:28   ` Stefano Garzarella
  2026-04-05 19:14   ` Arseniy Krasnov
  1 sibling, 2 replies; 9+ messages in thread
From: Luigi Leonardi @ 2026-04-02  8:18 UTC (permalink / raw)
  To: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
	Jason Wang, Xuan Zhuo, Eugenio Pérez, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
	Arseniy Krasnov
  Cc: kvm, virtualization, netdev, linux-kernel, Luigi Leonardi

Add a test that verifies MSG_PEEK works correctly after a partial
recv().

This is to test a bug that was present in the `virtio_transport_stream_do_peek()`
when computing the number of bytes to copy: After a partial read, the
peek function didn't take into consideration the number of bytes that
were already read. So peeking the whole buffer would cause a out-of-bounds read,
that resulted in a -EFAULT.

This test does exactly this: do a partial recv on a buffer, then try to
peek the whole buffer content.

Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
 tools/testing/vsock/vsock_test.c | 64 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 5bd20ccd9335caafe68e8b7a5d02a4deb3d2deec..308f9f8f30d22bec5aaa282356e400d8438fe321 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -346,6 +346,65 @@ static void test_stream_msg_peek_server(const struct test_opts *opts)
 	return test_msg_peek_server(opts, false);
 }
 
+#define PEEK_AFTER_RECV_LEN 100
+
+static void test_stream_peek_after_recv_client(const struct test_opts *opts)
+{
+	unsigned char buf[PEEK_AFTER_RECV_LEN];
+	int fd;
+	int i;
+
+	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
+	if (fd < 0) {
+		perror("connect");
+		exit(EXIT_FAILURE);
+	}
+
+	for (i = 0; i < sizeof(buf); i++)
+		buf[i] = (unsigned char)i;
+
+	control_expectln("SRVREADY");
+
+	send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
+
+	close(fd);
+}
+
+static void test_stream_peek_after_recv_server(const struct test_opts *opts)
+{
+	unsigned char buf[PEEK_AFTER_RECV_LEN];
+	int half = PEEK_AFTER_RECV_LEN / 2;
+	ssize_t ret;
+	int fd;
+
+	fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
+	if (fd < 0) {
+		perror("accept");
+		exit(EXIT_FAILURE);
+	}
+
+	control_writeln("SRVREADY");
+
+	/* Partial recv to advance offset within the skb */
+	recv_buf(fd, buf, half, 0, half);
+
+	/* Try to peek more than what remains: should return only 'half'
+	 * bytes. Note: we can't use recv_buf() because it loops until
+	 * all requested bytes are returned.
+	 */
+	ret = recv(fd, buf, sizeof(buf), MSG_PEEK);
+	if (ret < 0) {
+		perror("recv");
+		exit(EXIT_FAILURE);
+	} else if (ret != half) {
+		fprintf(stderr, "MSG_PEEK after partial recv returned %d (expected %d)\n",
+			ret, half);
+		exit(EXIT_FAILURE);
+	}
+
+	close(fd);
+}
+
 #define SOCK_BUF_SIZE (2 * 1024 * 1024)
 #define SOCK_BUF_SIZE_SMALL (64 * 1024)
 #define MAX_MSG_PAGES 4
@@ -2520,6 +2579,11 @@ static struct test_case test_cases[] = {
 		.run_client = test_stream_tx_credit_bounds_client,
 		.run_server = test_stream_tx_credit_bounds_server,
 	},
+	{
+		.name = "SOCK_STREAM MSG_PEEK after partial recv",
+		.run_client = test_stream_peek_after_recv_client,
+		.run_server = test_stream_peek_after_recv_server,
+	},
 	{},
 };
 

-- 
2.53.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH net 1/2] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating bytes to copy
  2026-04-02  8:18 ` [PATCH net 1/2] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating " Luigi Leonardi
@ 2026-04-02 13:08   ` Stefano Garzarella
  2026-04-05 19:22   ` Arseniy Krasnov
  1 sibling, 0 replies; 9+ messages in thread
From: Stefano Garzarella @ 2026-04-02 13:08 UTC (permalink / raw)
  To: Luigi Leonardi
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
	Eugenio Pérez, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Arseniy Krasnov, kvm, virtualization,
	netdev, linux-kernel

On Thu, Apr 02, 2026 at 10:18:01AM +0200, Luigi Leonardi wrote:
>`virtio_transport_stream_do_peek()` does not account for the skb offset
>when computing the number of bytes to copy.
>
>This means that, after a partial recv() that advances the offset, a peek
>requesting more bytes than are available in the sk_buff causes
>`skb_copy_datagram_iter()` to go past the valid payload, resulting in a -EFAULT.

nit:

WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#14: `skb_copy_datagram_iter()` to go past the valid payload, resulting 
in a -EFAULT.

>
>The dequeue path already handles this correctly.
>Apply the same logic to the peek path.
>
>Fixes: 0df7cd3c13e4 ("vsock/virtio/vhost: read data from non-linear skb")
>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>---
> net/vmw_vsock/virtio_transport_common.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
>index 8a9fb23c6e853dfea0a24d3787f7d6eb351dd6c6..4b65bfe5d875111f115e0fc4c6727adb66f34830 100644
>--- a/net/vmw_vsock/virtio_transport_common.c
>+++ b/net/vmw_vsock/virtio_transport_common.c
>@@ -547,9 +547,8 @@ virtio_transport_stream_do_peek(struct vsock_sock *vsk,
> 	skb_queue_walk(&vvs->rx_queue, skb) {
> 		size_t bytes;
>
>-		bytes = len - total;
>-		if (bytes > skb->len)
>-			bytes = skb->len;
>+		bytes = min_t(size_t, len - total,
>+			      skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset);

I think we should add some helper like virtio_transport_skb_remain() or 
virtio_transport_skb_bytes() and use it here but also in places where we 
check offset < len or offset == len. But maybe better to do that in 
another patch/series for net-next.

This fix LGTM:

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 2/2] vsock/test: add MSG_PEEK after partial recv test
  2026-04-02  8:18 ` [PATCH net 2/2] vsock/test: add MSG_PEEK after partial recv test Luigi Leonardi
@ 2026-04-02 13:28   ` Stefano Garzarella
  2026-04-03 11:40     ` Luigi Leonardi
  2026-04-05 19:14   ` Arseniy Krasnov
  1 sibling, 1 reply; 9+ messages in thread
From: Stefano Garzarella @ 2026-04-02 13:28 UTC (permalink / raw)
  To: Luigi Leonardi
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
	Eugenio Pérez, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Arseniy Krasnov, kvm, virtualization,
	netdev, linux-kernel

On Thu, Apr 02, 2026 at 10:18:02AM +0200, Luigi Leonardi wrote:
>Add a test that verifies MSG_PEEK works correctly after a partial
>recv().
>
>This is to test a bug that was present in the `virtio_transport_stream_do_peek()`

WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#11: This is to test a bug that was present in the 
`virtio_transport_stream_do_peek()`

>when computing the number of bytes to copy: After a partial read, the
>peek function didn't take into consideration the number of bytes that
>were already read. So peeking the whole buffer would cause a out-of-bounds read,
>that resulted in a -EFAULT.
>
>This test does exactly this: do a partial recv on a buffer, then try to
>peek the whole buffer content.
>
>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>---
> tools/testing/vsock/vsock_test.c | 64 ++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 64 insertions(+)
>
>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>index 5bd20ccd9335caafe68e8b7a5d02a4deb3d2deec..308f9f8f30d22bec5aaa282356e400d8438fe321 100644
>--- a/tools/testing/vsock/vsock_test.c
>+++ b/tools/testing/vsock/vsock_test.c
>@@ -346,6 +346,65 @@ static void test_stream_msg_peek_server(const struct test_opts *opts)
> 	return test_msg_peek_server(opts, false);
> }
>
>+#define PEEK_AFTER_RECV_LEN 100

Why 100 ?
Better to use a power of 2 IMO like we do in all other cases IIRC.

>+
>+static void test_stream_peek_after_recv_client(const struct test_opts *opts)
>+{
>+	unsigned char buf[PEEK_AFTER_RECV_LEN];
>+	int fd;
>+	int i;

nit: int fd, i;

>+
>+	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
>+	if (fd < 0) {
>+		perror("connect");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	for (i = 0; i < sizeof(buf); i++)
>+		buf[i] = (unsigned char)i;

Why setting the payload in this way ? Can we just do a memset() ?

>+
>+	control_expectln("SRVREADY");

Why we need this barrier ?

>+
>+	send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
>+
>+	close(fd);
>+}
>+
>+static void test_stream_peek_after_recv_server(const struct test_opts *opts)
>+{
>+	unsigned char buf[PEEK_AFTER_RECV_LEN];
>+	int half = PEEK_AFTER_RECV_LEN / 2;
>+	ssize_t ret;
>+	int fd;
>+
>+	fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
>+	if (fd < 0) {
>+		perror("accept");
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	control_writeln("SRVREADY");
>+
>+	/* Partial recv to advance offset within the skb */
>+	recv_buf(fd, buf, half, 0, half);

Why reading half of the size ?

IMO is better to read just 1 byte, since it is almost certain that an 
skb does not have a 1-byte payload.

>+
>+	/* Try to peek more than what remains: should return only 'half'

How we are sure that the sender sent all the bytes ?

>+	 * bytes. Note: we can't use recv_buf() because it loops until
>+	 * all requested bytes are returned.

Why this is a problem ? (an useful comment should explain the reason)

>+	 */
>+	ret = recv(fd, buf, sizeof(buf), MSG_PEEK);
>+	if (ret < 0) {

Should we handle EINTR like we do in recv_buf() ?
But I still don't understand why we can't use it directly.

Thanks,
Stefano

>+		perror("recv");
>+		exit(EXIT_FAILURE);
>+	} else if (ret != half) {
>+		fprintf(stderr, "MSG_PEEK after partial recv returned %d (expected %d)\n",
>+			ret, half);
>+		exit(EXIT_FAILURE);
>+	}
>+
>+	close(fd);
>+}
>+
> #define SOCK_BUF_SIZE (2 * 1024 * 1024)
> #define SOCK_BUF_SIZE_SMALL (64 * 1024)
> #define MAX_MSG_PAGES 4
>@@ -2520,6 +2579,11 @@ static struct test_case test_cases[] = {
> 		.run_client = test_stream_tx_credit_bounds_client,
> 		.run_server = test_stream_tx_credit_bounds_server,
> 	},
>+	{
>+		.name = "SOCK_STREAM MSG_PEEK after partial recv",
>+		.run_client = test_stream_peek_after_recv_client,
>+		.run_server = test_stream_peek_after_recv_server,
>+	},
> 	{},
> };
>
>
>-- 
>2.53.0
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 2/2] vsock/test: add MSG_PEEK after partial recv test
  2026-04-02 13:28   ` Stefano Garzarella
@ 2026-04-03 11:40     ` Luigi Leonardi
  0 siblings, 0 replies; 9+ messages in thread
From: Luigi Leonardi @ 2026-04-03 11:40 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Stefan Hajnoczi, Michael S. Tsirkin, Jason Wang, Xuan Zhuo,
	Eugenio Pérez, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Arseniy Krasnov, kvm, virtualization,
	netdev, linux-kernel

On Thu, Apr 02, 2026 at 03:28:25PM +0200, Stefano Garzarella wrote:
>On Thu, Apr 02, 2026 at 10:18:02AM +0200, Luigi Leonardi wrote:
>>Add a test that verifies MSG_PEEK works correctly after a partial
>>recv().
>>
>>This is to test a bug that was present in the `virtio_transport_stream_do_peek()`
>
>WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
>#11: This is to test a bug that was present in the 
>`virtio_transport_stream_do_peek()`
>

oops, thanks :)

>>when computing the number of bytes to copy: After a partial read, the
>>peek function didn't take into consideration the number of bytes that
>>were already read. So peeking the whole buffer would cause a out-of-bounds read,
>>that resulted in a -EFAULT.
>>
>>This test does exactly this: do a partial recv on a buffer, then try to
>>peek the whole buffer content.
>>
>>Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>>---
>>tools/testing/vsock/vsock_test.c | 64 ++++++++++++++++++++++++++++++++++++++++
>>1 file changed, 64 insertions(+)
>>
>>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>>index 5bd20ccd9335caafe68e8b7a5d02a4deb3d2deec..308f9f8f30d22bec5aaa282356e400d8438fe321 100644
>>--- a/tools/testing/vsock/vsock_test.c
>>+++ b/tools/testing/vsock/vsock_test.c
>>@@ -346,6 +346,65 @@ static void test_stream_msg_peek_server(const struct test_opts *opts)
>>	return test_msg_peek_server(opts, false);
>>}
>>
>>+#define PEEK_AFTER_RECV_LEN 100
>
>Why 100 ?
>Better to use a power of 2 IMO like we do in all other cases IIRC.
>

Right, I'll reuse `MSG_PEEK_BUF_LEN`.

>>+
>>+static void test_stream_peek_after_recv_client(const struct test_opts *opts)
>>+{
>>+	unsigned char buf[PEEK_AFTER_RECV_LEN];
>>+	int fd;
>>+	int i;
>
>nit: int fd, i;
>
>>+
>>+	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
>>+	if (fd < 0) {
>>+		perror("connect");
>>+		exit(EXIT_FAILURE);
>>+	}
>>+
>>+	for (i = 0; i < sizeof(buf); i++)
>>+		buf[i] = (unsigned char)i;
>
>Why setting the payload in this way ? Can we just do a memset() ?

Good point.

>
>>+
>>+	control_expectln("SRVREADY");
>
>Why we need this barrier ?

leftover from development, will remove.

>
>>+
>>+	send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
>>+
>>+	close(fd);
>>+}
>>+
>>+static void test_stream_peek_after_recv_server(const struct test_opts *opts)
>>+{
>>+	unsigned char buf[PEEK_AFTER_RECV_LEN];
>>+	int half = PEEK_AFTER_RECV_LEN / 2;
>>+	ssize_t ret;
>>+	int fd;
>>+
>>+	fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
>>+	if (fd < 0) {
>>+		perror("accept");
>>+		exit(EXIT_FAILURE);
>>+	}
>>+
>>+	control_writeln("SRVREADY");
>>+
>>+	/* Partial recv to advance offset within the skb */
>>+	recv_buf(fd, buf, half, 0, half);
>
>Why reading half of the size ?
>
>IMO is better to read just 1 byte, since it is almost certain that an 
>skb does not have a 1-byte payload.
>

will do

>>+
>>+	/* Try to peek more than what remains: should return only 'half'
>
>How we are sure that the sender sent all the bytes ?
>
>>+	 * bytes. Note: we can't use recv_buf() because it loops until
>>+	 * all requested bytes are returned.
>
>Why this is a problem ? (an useful comment should explain the reason)
>

Some changes are required to `recv_buf`, I have a working v2 version
that uses that. Thanks for the hint.

>>+	 */
>>+	ret = recv(fd, buf, sizeof(buf), MSG_PEEK);
>>+	if (ret < 0) {
>
>Should we handle EINTR like we do in recv_buf() ?
>But I still don't understand why we can't use it directly.
>
>Thanks,
>Stefano
>
>>+		perror("recv");
>>+		exit(EXIT_FAILURE);
>>+	} else if (ret != half) {
>>+		fprintf(stderr, "MSG_PEEK after partial recv returned %d (expected %d)\n",
>>+			ret, half);
>>+		exit(EXIT_FAILURE);
>>+	}
>>+
>>+	close(fd);
>>+}
>>+
>>#define SOCK_BUF_SIZE (2 * 1024 * 1024)
>>#define SOCK_BUF_SIZE_SMALL (64 * 1024)
>>#define MAX_MSG_PAGES 4
>>@@ -2520,6 +2579,11 @@ static struct test_case test_cases[] = {
>>		.run_client = test_stream_tx_credit_bounds_client,
>>		.run_server = test_stream_tx_credit_bounds_server,
>>	},
>>+	{
>>+		.name = "SOCK_STREAM MSG_PEEK after partial recv",
>>+		.run_client = test_stream_peek_after_recv_client,
>>+		.run_server = test_stream_peek_after_recv_server,
>>+	},
>>	{},
>>};
>>
>>
>>-- 
>>2.53.0
>>
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 2/2] vsock/test: add MSG_PEEK after partial recv test
  2026-04-02  8:18 ` [PATCH net 2/2] vsock/test: add MSG_PEEK after partial recv test Luigi Leonardi
  2026-04-02 13:28   ` Stefano Garzarella
@ 2026-04-05 19:14   ` Arseniy Krasnov
  2026-04-07  8:45     ` Luigi Leonardi
  1 sibling, 1 reply; 9+ messages in thread
From: Arseniy Krasnov @ 2026-04-05 19:14 UTC (permalink / raw)
  To: Luigi Leonardi, Stefan Hajnoczi, Stefano Garzarella,
	Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: kvm, virtualization, netdev, linux-kernel



02.04.2026 11:18, Luigi Leonardi wrote:
> Add a test that verifies MSG_PEEK works correctly after a partial
> recv().
> 
> This is to test a bug that was present in the `virtio_transport_stream_do_peek()`
> when computing the number of bytes to copy: After a partial read, the
> peek function didn't take into consideration the number of bytes that
> were already read. So peeking the whole buffer would cause a out-of-bounds read,
> that resulted in a -EFAULT.
> 
> This test does exactly this: do a partial recv on a buffer, then try to
> peek the whole buffer content.
> 
> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
> ---
>  tools/testing/vsock/vsock_test.c | 64 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
> index 5bd20ccd9335caafe68e8b7a5d02a4deb3d2deec..308f9f8f30d22bec5aaa282356e400d8438fe321 100644
> --- a/tools/testing/vsock/vsock_test.c
> +++ b/tools/testing/vsock/vsock_test.c
> @@ -346,6 +346,65 @@ static void test_stream_msg_peek_server(const struct test_opts *opts)
>  	return test_msg_peek_server(opts, false);
>  }
>  
> +#define PEEK_AFTER_RECV_LEN 100

Hi, may be we can just reuse MSG_PEEK_BUF_LEN which was already used in MSG_PEEK tests ?

Thanks

> +
> +static void test_stream_peek_after_recv_client(const struct test_opts *opts)
> +{
> +	unsigned char buf[PEEK_AFTER_RECV_LEN];
> +	int fd;
> +	int i;
> +
> +	fd = vsock_stream_connect(opts->peer_cid, opts->peer_port);
> +	if (fd < 0) {
> +		perror("connect");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	for (i = 0; i < sizeof(buf); i++)
> +		buf[i] = (unsigned char)i;
> +
> +	control_expectln("SRVREADY");
> +
> +	send_buf(fd, buf, sizeof(buf), 0, sizeof(buf));
> +
> +	close(fd);
> +}
> +
> +static void test_stream_peek_after_recv_server(const struct test_opts *opts)
> +{
> +	unsigned char buf[PEEK_AFTER_RECV_LEN];
> +	int half = PEEK_AFTER_RECV_LEN / 2;
> +	ssize_t ret;
> +	int fd;
> +
> +	fd = vsock_stream_accept(VMADDR_CID_ANY, opts->peer_port, NULL);
> +	if (fd < 0) {
> +		perror("accept");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	control_writeln("SRVREADY");
> +
> +	/* Partial recv to advance offset within the skb */
> +	recv_buf(fd, buf, half, 0, half);
> +
> +	/* Try to peek more than what remains: should return only 'half'
> +	 * bytes. Note: we can't use recv_buf() because it loops until
> +	 * all requested bytes are returned.
> +	 */
> +	ret = recv(fd, buf, sizeof(buf), MSG_PEEK);
> +	if (ret < 0) {
> +		perror("recv");
> +		exit(EXIT_FAILURE);
> +	} else if (ret != half) {
> +		fprintf(stderr, "MSG_PEEK after partial recv returned %d (expected %d)\n",
> +			ret, half);
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	close(fd);
> +}
> +
>  #define SOCK_BUF_SIZE (2 * 1024 * 1024)
>  #define SOCK_BUF_SIZE_SMALL (64 * 1024)
>  #define MAX_MSG_PAGES 4
> @@ -2520,6 +2579,11 @@ static struct test_case test_cases[] = {
>  		.run_client = test_stream_tx_credit_bounds_client,
>  		.run_server = test_stream_tx_credit_bounds_server,
>  	},
> +	{
> +		.name = "SOCK_STREAM MSG_PEEK after partial recv",
> +		.run_client = test_stream_peek_after_recv_client,
> +		.run_server = test_stream_peek_after_recv_server,
> +	},
>  	{},
>  };
>  
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 1/2] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating bytes to copy
  2026-04-02  8:18 ` [PATCH net 1/2] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating " Luigi Leonardi
  2026-04-02 13:08   ` Stefano Garzarella
@ 2026-04-05 19:22   ` Arseniy Krasnov
  1 sibling, 0 replies; 9+ messages in thread
From: Arseniy Krasnov @ 2026-04-05 19:22 UTC (permalink / raw)
  To: Luigi Leonardi, Stefan Hajnoczi, Stefano Garzarella,
	Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman
  Cc: kvm, virtualization, netdev, linux-kernel



02.04.2026 11:18, Luigi Leonardi wrote:
> `virtio_transport_stream_do_peek()` does not account for the skb offset
> when computing the number of bytes to copy.
> 
> This means that, after a partial recv() that advances the offset, a peek
> requesting more bytes than are available in the sk_buff causes
> `skb_copy_datagram_iter()` to go past the valid payload, resulting in a -EFAULT.
> 
> The dequeue path already handles this correctly.
> Apply the same logic to the peek path.
> 
> Fixes: 0df7cd3c13e4 ("vsock/virtio/vhost: read data from non-linear skb")
> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
> ---
>  net/vmw_vsock/virtio_transport_common.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index 8a9fb23c6e853dfea0a24d3787f7d6eb351dd6c6..4b65bfe5d875111f115e0fc4c6727adb66f34830 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -547,9 +547,8 @@ virtio_transport_stream_do_peek(struct vsock_sock *vsk,
>  	skb_queue_walk(&vvs->rx_queue, skb) {
>  		size_t bytes;
>  
> -		bytes = len - total;
> -		if (bytes > skb->len)
> -			bytes = skb->len;
> +		bytes = min_t(size_t, len - total,
> +			      skb->len - VIRTIO_VSOCK_SKB_CB(skb)->offset);
>  
>  		spin_unlock_bh(&vvs->rx_lock);
>  
> 

Acked-by: Arseniy Krasnov <avkrasnov@salutedevices.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH net 2/2] vsock/test: add MSG_PEEK after partial recv test
  2026-04-05 19:14   ` Arseniy Krasnov
@ 2026-04-07  8:45     ` Luigi Leonardi
  0 siblings, 0 replies; 9+ messages in thread
From: Luigi Leonardi @ 2026-04-07  8:45 UTC (permalink / raw)
  To: Arseniy Krasnov
  Cc: Stefan Hajnoczi, Stefano Garzarella, Michael S. Tsirkin,
	Jason Wang, Xuan Zhuo, Eugenio Pérez, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman, kvm,
	virtualization, netdev, linux-kernel

Hi Arseniy,

On Sun, Apr 05, 2026 at 10:14:26PM +0300, Arseniy Krasnov wrote:
>
>
>02.04.2026 11:18, Luigi Leonardi wrote:
>> Add a test that verifies MSG_PEEK works correctly after a partial
>> recv().
>>
>> This is to test a bug that was present in the `virtio_transport_stream_do_peek()`
>> when computing the number of bytes to copy: After a partial read, the
>> peek function didn't take into consideration the number of bytes that
>> were already read. So peeking the whole buffer would cause a out-of-bounds read,
>> that resulted in a -EFAULT.
>>
>> This test does exactly this: do a partial recv on a buffer, then try to
>> peek the whole buffer content.
>>
>> Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
>> ---
>>  tools/testing/vsock/vsock_test.c | 64 ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 64 insertions(+)
>>
>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>> index 5bd20ccd9335caafe68e8b7a5d02a4deb3d2deec..308f9f8f30d22bec5aaa282356e400d8438fe321 100644
>> --- a/tools/testing/vsock/vsock_test.c
>> +++ b/tools/testing/vsock/vsock_test.c
>> @@ -346,6 +346,65 @@ static void test_stream_msg_peek_server(const struct test_opts *opts)
>>  	return test_msg_peek_server(opts, false);
>>  }
>>
>> +#define PEEK_AFTER_RECV_LEN 100
>
>Hi, may be we can just reuse MSG_PEEK_BUF_LEN which was already used in MSG_PEEK tests ?
>

yep, good idea!

Thanks!


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-04-07  8:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-02  8:18 [PATCH net 0/2] vsock/virtio: fix MSG_PEEK calculation on bytes to copy Luigi Leonardi
2026-04-02  8:18 ` [PATCH net 1/2] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating " Luigi Leonardi
2026-04-02 13:08   ` Stefano Garzarella
2026-04-05 19:22   ` Arseniy Krasnov
2026-04-02  8:18 ` [PATCH net 2/2] vsock/test: add MSG_PEEK after partial recv test Luigi Leonardi
2026-04-02 13:28   ` Stefano Garzarella
2026-04-03 11:40     ` Luigi Leonardi
2026-04-05 19:14   ` Arseniy Krasnov
2026-04-07  8:45     ` Luigi Leonardi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox