* [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