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