* [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
` (2 more replies)
0 siblings, 3 replies; 10+ 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] 10+ 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
2026-06-15 19:48 ` [PATCH net 0/2] vsock/virtio: fix MSG_PEEK calculation on bytes to copy Michael S. Tsirkin
2 siblings, 2 replies; 10+ 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] 10+ 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
2026-06-15 19:48 ` [PATCH net 0/2] vsock/virtio: fix MSG_PEEK calculation on bytes to copy Michael S. Tsirkin
2 siblings, 2 replies; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ 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; 10+ 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] 10+ messages in thread
* Re: [PATCH net 0/2] vsock/virtio: fix MSG_PEEK calculation on 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 ` [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
@ 2026-06-15 19:48 ` Michael S. Tsirkin
2 siblings, 0 replies; 10+ messages in thread
From: Michael S. Tsirkin @ 2026-06-15 19:48 UTC (permalink / raw)
To: Luigi Leonardi
Cc: Stefan Hajnoczi, Stefano Garzarella, 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:00AM +0200, Luigi Leonardi wrote:
> `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>
Acked-by: Michael S. Tsirkin <mst@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] 10+ messages in thread
end of thread, other threads:[~2026-06-15 19:48 UTC | newest]
Thread overview: 10+ 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
2026-06-15 19:48 ` [PATCH net 0/2] vsock/virtio: fix MSG_PEEK calculation on bytes to copy Michael S. Tsirkin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.