* [PATCH net v3 0/3] vsock/virtio: fix MSG_PEEK calculation on bytes to copy
@ 2026-04-14 16:10 Luigi Leonardi
2026-04-14 16:10 ` [PATCH net v3 1/3] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating " Luigi Leonardi
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Luigi Leonardi @ 2026-04-14 16:10 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 happened
before.
This might cause out-of-bounds read that lead to an EFAULT.
More details in the commits.
Commit 1 introduces the fix
Commit 2 introduces some preliminary work for adding a test and fixes a
problem in existing tests.
Commit 3 introduces a test that checks for this bug to avoid future
regressions.
For disclosure: this bug was found initially by claude opus 4.6, I then analzyed
it and worked on the fix and the test.
Signed-off-by: Luigi Leonardi <leonardi@redhat.com>
---
Changes in v3:
- Addressed reviwers omment
- Dropped test client, reusing the one already existing
- Minor changes: added comment, improved commit messages
- Rebased to latest net-next
- Link to v2: https://lore.kernel.org/r/20260407-fix_peek-v2-0-2e2581dc8b7c@redhat.com
Changes in v2:
- Addressed reviewers comment
- Test now uses the recv_buf utils.
- Removed unnecessary barrier
- Checkpatch warnings.
- Added new commit that allows to use recv_buf with MSG_PEEK
- Picked up RoBs
- Link to v1: https://lore.kernel.org/r/20260402-fix_peek-v1-0-ad274fcef77b@redhat.com
---
Luigi Leonardi (3):
vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating bytes to copy
vsock/test: fix MSG_PEEK handling in recv_buf()
vsock/test: add MSG_PEEK after partial recv test
net/vmw_vsock/virtio_transport_common.c | 5 ++--
tools/testing/vsock/util.c | 15 ++++++++++
tools/testing/vsock/vsock_test.c | 50 +++++++++++++++++++++++++--------
3 files changed, 55 insertions(+), 15 deletions(-)
---
base-commit: bc28831d7a09f7058cdca4658d81e5faf635bed7
change-id: 20260401-fix_peek-6837b83469e3
Best regards,
--
Luigi Leonardi <leonardi@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH net v3 1/3] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating bytes to copy 2026-04-14 16:10 [PATCH net v3 0/3] vsock/virtio: fix MSG_PEEK calculation on bytes to copy Luigi Leonardi @ 2026-04-14 16:10 ` Luigi Leonardi 2026-04-14 16:10 ` [PATCH net v3 2/3] vsock/test: fix MSG_PEEK handling in recv_buf() Luigi Leonardi 2026-04-14 16:10 ` [PATCH net v3 3/3] vsock/test: add MSG_PEEK after partial recv test Luigi Leonardi 2 siblings, 0 replies; 8+ messages in thread From: Luigi Leonardi @ 2026-04-14 16:10 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") Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Acked-by: Arseniy Krasnov <avkrasnov@salutedevices.com> 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 a152a9e208d0..b5015ab2ee1e 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -545,9 +545,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] 8+ messages in thread
* [PATCH net v3 2/3] vsock/test: fix MSG_PEEK handling in recv_buf() 2026-04-14 16:10 [PATCH net v3 0/3] vsock/virtio: fix MSG_PEEK calculation on bytes to copy Luigi Leonardi 2026-04-14 16:10 ` [PATCH net v3 1/3] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating " Luigi Leonardi @ 2026-04-14 16:10 ` Luigi Leonardi 2026-04-15 11:31 ` Stefano Garzarella 2026-04-14 16:10 ` [PATCH net v3 3/3] vsock/test: add MSG_PEEK after partial recv test Luigi Leonardi 2 siblings, 1 reply; 8+ messages in thread From: Luigi Leonardi @ 2026-04-14 16:10 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 `recv_buf` does not handle the MSG_PEEK flag correctly: it keeps calling `recv` until all requested bytes are available or an error occurs. The problem is how it calculates the amount of bytes read: MSG_PEEK doesn't consume any bytes, will re-read the same bytes from the buffer head, so, summing the return value every time is wrong. Moreover, MSG_PEEK doesn't consume the bytes in the buffer, so if the requested amount is more than the bytes available, the loop will never terminate, because `recv` will never return EOF. For this reason we need to compare the amount of read bytes with the number of bytes expected. Add a check, and if the MSG_PEEK flag is present, update the counter of read bytes differently, and break if we read the expected amount. This allows us to simplify the `test_stream_credit_update_test`, by reusing `recv_buf`, like some other tests already do. This also fixes callers that pass MSG_PEEK to recv_buf(). Suggested-by: Stefano Garzarella <sgarzare@redhat.com> Signed-off-by: Luigi Leonardi <leonardi@redhat.com> --- tools/testing/vsock/util.c | 15 +++++++++++++++ tools/testing/vsock/vsock_test.c | 13 +------------ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c index 1fe1338c79cd..2c9ee3210090 100644 --- a/tools/testing/vsock/util.c +++ b/tools/testing/vsock/util.c @@ -381,7 +381,13 @@ void send_buf(int fd, const void *buf, size_t len, int flags, } } +#define RECV_PEEK_RETRY_USEC 10 + /* Receive bytes in a buffer and check the return value. + * + * MSG_PEEK note: MSG_PEEK doesn't consume bytes from the buffer, so partial + * reads cannot be summed. Instead, the function retries until recv() returns + * exactly expected_ret bytes in a single call. * * expected_ret: * <0 Negative errno (for testing errors) @@ -403,6 +409,15 @@ void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret) if (ret <= 0) break; + if (flags & MSG_PEEK) { + if (ret == expected_ret) { + nread = ret; + break; + } + timeout_usleep(RECV_PEEK_RETRY_USEC); + continue; + } + nread += ret; } while (nread < len); timeout_end(); diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c index 5bd20ccd9335..bdb0754965df 100644 --- a/tools/testing/vsock/vsock_test.c +++ b/tools/testing/vsock/vsock_test.c @@ -1500,18 +1500,7 @@ static void test_stream_credit_update_test(const struct test_opts *opts, } /* Wait until there will be 128KB of data in rx queue. */ - while (1) { - ssize_t res; - - res = recv(fd, buf, buf_size, MSG_PEEK); - if (res == buf_size) - break; - - if (res <= 0) { - fprintf(stderr, "unexpected 'recv()' return: %zi\n", res); - exit(EXIT_FAILURE); - } - } + recv_buf(fd, buf, buf_size, MSG_PEEK, buf_size); /* There is 128KB of data in the socket's rx queue, dequeue first * 64KB, credit update is sent if 'low_rx_bytes_test' == true. -- 2.53.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 2/3] vsock/test: fix MSG_PEEK handling in recv_buf() 2026-04-14 16:10 ` [PATCH net v3 2/3] vsock/test: fix MSG_PEEK handling in recv_buf() Luigi Leonardi @ 2026-04-15 11:31 ` Stefano Garzarella 2026-04-15 11:54 ` Stefano Garzarella 0 siblings, 1 reply; 8+ messages in thread From: Stefano Garzarella @ 2026-04-15 11:31 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 Tue, Apr 14, 2026 at 06:10:22PM +0200, Luigi Leonardi wrote: >`recv_buf` does not handle the MSG_PEEK flag correctly: it keeps calling >`recv` until all requested bytes are available or an error occurs. > >The problem is how it calculates the amount of bytes read: MSG_PEEK >doesn't consume any bytes, will re-read the same bytes from the buffer >head, so, summing the return value every time is wrong. > >Moreover, MSG_PEEK doesn't consume the bytes in the buffer, so if the >requested amount is more than the bytes available, the loop will never >terminate, because `recv` will never return EOF. For this reason we need >to compare the amount of read bytes with the number of bytes expected. > >Add a check, and if the MSG_PEEK flag is present, update the counter of >read bytes differently, and break if we read the expected amount. nit: "..., update the counter for bytes read only after all expected bytes have been read and break out of the loop; otherwise, try again after a short delay to avoid consuming too many CPU cycles." > >This allows us to simplify the `test_stream_credit_update_test`, by >reusing `recv_buf`, like some other tests already do. > >This also fixes callers that pass MSG_PEEK to recv_buf(). nit: this is implicit from the first part of the description. > >Suggested-by: Stefano Garzarella <sgarzare@redhat.com> >Signed-off-by: Luigi Leonardi <leonardi@redhat.com> >--- > tools/testing/vsock/util.c | 15 +++++++++++++++ > tools/testing/vsock/vsock_test.c | 13 +------------ > 2 files changed, 16 insertions(+), 12 deletions(-) > >diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c >index 1fe1338c79cd..2c9ee3210090 100644 >--- a/tools/testing/vsock/util.c >+++ b/tools/testing/vsock/util.c >@@ -381,7 +381,13 @@ void send_buf(int fd, const void *buf, size_t len, int flags, > } > } > >+#define RECV_PEEK_RETRY_USEC 10 10 usec IMO are a bit low, it could be the same order of the syscalls involved in the loop, I'd go to some milliseconds like we do for SEND_SLEEP_USEC. >+ > /* Receive bytes in a buffer and check the return value. >+ * >+ * MSG_PEEK note: MSG_PEEK doesn't consume bytes from the buffer, so partial >+ * reads cannot be summed. Instead, the function retries until recv() returns >+ * exactly expected_ret bytes in a single call. I'd replace with something like this: * When MSG_PEEK is set, recv() is retried until it returns exactly * expected_ret bytes. The function returns on error, EOF, or timeout * as usual. Thanks, Stefano > * > * expected_ret: > * <0 Negative errno (for testing errors) >@@ -403,6 +409,15 @@ void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret) > if (ret <= 0) > break; > >+ if (flags & MSG_PEEK) { >+ if (ret == expected_ret) { >+ nread = ret; >+ break; >+ } >+ timeout_usleep(RECV_PEEK_RETRY_USEC); >+ continue; >+ } >+ > nread += ret; > } while (nread < len); > timeout_end(); >diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c >index 5bd20ccd9335..bdb0754965df 100644 >--- a/tools/testing/vsock/vsock_test.c >+++ b/tools/testing/vsock/vsock_test.c >@@ -1500,18 +1500,7 @@ static void test_stream_credit_update_test(const struct test_opts *opts, > } > > /* Wait until there will be 128KB of data in rx queue. */ >- while (1) { >- ssize_t res; >- >- res = recv(fd, buf, buf_size, MSG_PEEK); >- if (res == buf_size) >- break; >- >- if (res <= 0) { >- fprintf(stderr, "unexpected 'recv()' return: %zi\n", res); >- exit(EXIT_FAILURE); >- } >- } >+ recv_buf(fd, buf, buf_size, MSG_PEEK, buf_size); > > /* There is 128KB of data in the socket's rx queue, dequeue first > * 64KB, credit update is sent if 'low_rx_bytes_test' == true. > >-- >2.53.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 2/3] vsock/test: fix MSG_PEEK handling in recv_buf() 2026-04-15 11:31 ` Stefano Garzarella @ 2026-04-15 11:54 ` Stefano Garzarella 2026-04-15 13:11 ` Luigi Leonardi 0 siblings, 1 reply; 8+ messages in thread From: Stefano Garzarella @ 2026-04-15 11:54 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 Wed, Apr 15, 2026 at 01:31:11PM +0200, Stefano Garzarella wrote: >On Tue, Apr 14, 2026 at 06:10:22PM +0200, Luigi Leonardi wrote: >>`recv_buf` does not handle the MSG_PEEK flag correctly: it keeps calling >>`recv` until all requested bytes are available or an error occurs. >> >>The problem is how it calculates the amount of bytes read: MSG_PEEK >>doesn't consume any bytes, will re-read the same bytes from the buffer >>head, so, summing the return value every time is wrong. >> >>Moreover, MSG_PEEK doesn't consume the bytes in the buffer, so if the >>requested amount is more than the bytes available, the loop will never >>terminate, because `recv` will never return EOF. For this reason we need >>to compare the amount of read bytes with the number of bytes expected. >> >>Add a check, and if the MSG_PEEK flag is present, update the counter of >>read bytes differently, and break if we read the expected amount. > >nit: "..., update the counter for bytes read only after all expected >bytes have been read and break out of the loop; otherwise, try again >after a short delay to avoid consuming too many CPU cycles." > >> >>This allows us to simplify the `test_stream_credit_update_test`, by >>reusing `recv_buf`, like some other tests already do. >> >>This also fixes callers that pass MSG_PEEK to recv_buf(). > >nit: this is implicit from the first part of the description. > >> >>Suggested-by: Stefano Garzarella <sgarzare@redhat.com> >>Signed-off-by: Luigi Leonardi <leonardi@redhat.com> >>--- >>tools/testing/vsock/util.c | 15 +++++++++++++++ >>tools/testing/vsock/vsock_test.c | 13 +------------ >>2 files changed, 16 insertions(+), 12 deletions(-) >> >>diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c >>index 1fe1338c79cd..2c9ee3210090 100644 >>--- a/tools/testing/vsock/util.c >>+++ b/tools/testing/vsock/util.c >>@@ -381,7 +381,13 @@ void send_buf(int fd, const void *buf, size_t len, int flags, >> } >>} >> >>+#define RECV_PEEK_RETRY_USEC 10 > >10 usec IMO are a bit low, it could be the same order of the syscalls >involved in the loop, I'd go to some milliseconds like we do for >SEND_SLEEP_USEC. > >>+ >>/* Receive bytes in a buffer and check the return value. >>+ * >>+ * MSG_PEEK note: MSG_PEEK doesn't consume bytes from the buffer, so partial >>+ * reads cannot be summed. Instead, the function retries until recv() returns >>+ * exactly expected_ret bytes in a single call. > >I'd replace with something like this: > > * When MSG_PEEK is set, recv() is retried until it returns exactly > * expected_ret bytes. The function returns on error, EOF, or timeout > * as usual. > >Thanks, >Stefano > >> * >> * expected_ret: >> * <0 Negative errno (for testing errors) >>@@ -403,6 +409,15 @@ void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret) >> if (ret <= 0) >> break; >> >>+ if (flags & MSG_PEEK) { >>+ if (ret == expected_ret) { On second thought, I think it would be more appropriate to check for `ret >= expected_ret` here, because all subsequent recv() will definitely return more bytes, so there’s no point in continuing the loop... and anyway, we’ll check the result later, so just that change should be fine. And of course I'd update the comment on top in this way: * When MSG_PEEK is set, recv() is retried until it returns at least * expected_ret bytes. The function returns on error, EOF, or timeout * as usual. Thanks, Stefano >>+ nread = ret; >>+ break; >>+ } >>+ timeout_usleep(RECV_PEEK_RETRY_USEC); >>+ continue; >>+ } >>+ >> nread += ret; >> } while (nread < len); >> timeout_end(); >>diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c >>index 5bd20ccd9335..bdb0754965df 100644 >>--- a/tools/testing/vsock/vsock_test.c >>+++ b/tools/testing/vsock/vsock_test.c >>@@ -1500,18 +1500,7 @@ static void test_stream_credit_update_test(const struct test_opts *opts, >> } >> >> /* Wait until there will be 128KB of data in rx queue. */ >>- while (1) { >>- ssize_t res; >>- >>- res = recv(fd, buf, buf_size, MSG_PEEK); >>- if (res == buf_size) >>- break; >>- >>- if (res <= 0) { >>- fprintf(stderr, "unexpected 'recv()' return: %zi\n", res); >>- exit(EXIT_FAILURE); >>- } >>- } >>+ recv_buf(fd, buf, buf_size, MSG_PEEK, buf_size); >> >> /* There is 128KB of data in the socket's rx queue, dequeue first >> * 64KB, credit update is sent if 'low_rx_bytes_test' == true. >> >>-- >>2.53.0 >> ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 2/3] vsock/test: fix MSG_PEEK handling in recv_buf() 2026-04-15 11:54 ` Stefano Garzarella @ 2026-04-15 13:11 ` Luigi Leonardi 0 siblings, 0 replies; 8+ messages in thread From: Luigi Leonardi @ 2026-04-15 13:11 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 Wed, Apr 15, 2026 at 01:54:43PM +0200, Stefano Garzarella wrote: >On Wed, Apr 15, 2026 at 01:31:11PM +0200, Stefano Garzarella wrote: >>On Tue, Apr 14, 2026 at 06:10:22PM +0200, Luigi Leonardi wrote: >>>`recv_buf` does not handle the MSG_PEEK flag correctly: it keeps calling >>>`recv` until all requested bytes are available or an error occurs. >>> >>>The problem is how it calculates the amount of bytes read: MSG_PEEK >>>doesn't consume any bytes, will re-read the same bytes from the buffer >>>head, so, summing the return value every time is wrong. >>> >>>Moreover, MSG_PEEK doesn't consume the bytes in the buffer, so if the >>>requested amount is more than the bytes available, the loop will never >>>terminate, because `recv` will never return EOF. For this reason we need >>>to compare the amount of read bytes with the number of bytes expected. >>> >>>Add a check, and if the MSG_PEEK flag is present, update the counter of >>>read bytes differently, and break if we read the expected amount. >> >>nit: "..., update the counter for bytes read only after all expected >>bytes have been read and break out of the loop; otherwise, try again >>after a short delay to avoid consuming too many CPU cycles." >> >>> >>>This allows us to simplify the `test_stream_credit_update_test`, by >>>reusing `recv_buf`, like some other tests already do. >>> >>>This also fixes callers that pass MSG_PEEK to recv_buf(). >> >>nit: this is implicit from the first part of the description. >> >>> >>>Suggested-by: Stefano Garzarella <sgarzare@redhat.com> >>>Signed-off-by: Luigi Leonardi <leonardi@redhat.com> >>>--- >>>tools/testing/vsock/util.c | 15 +++++++++++++++ >>>tools/testing/vsock/vsock_test.c | 13 +------------ >>>2 files changed, 16 insertions(+), 12 deletions(-) >>> >>>diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c >>>index 1fe1338c79cd..2c9ee3210090 100644 >>>--- a/tools/testing/vsock/util.c >>>+++ b/tools/testing/vsock/util.c >>>@@ -381,7 +381,13 @@ void send_buf(int fd, const void *buf, size_t len, int flags, >>> } >>>} >>> >>>+#define RECV_PEEK_RETRY_USEC 10 >> >>10 usec IMO are a bit low, it could be the same order of the >>syscalls involved in the loop, I'd go to some milliseconds like we >>do for SEND_SLEEP_USEC. >> >>>+ >>>/* Receive bytes in a buffer and check the return value. >>>+ * >>>+ * MSG_PEEK note: MSG_PEEK doesn't consume bytes from the buffer, so partial >>>+ * reads cannot be summed. Instead, the function retries until recv() returns >>>+ * exactly expected_ret bytes in a single call. >> >>I'd replace with something like this: >> >> * When MSG_PEEK is set, recv() is retried until it returns exactly >> * expected_ret bytes. The function returns on error, EOF, or timeout >> * as usual. >> >>Thanks, >>Stefano >> >>>* >>>* expected_ret: >>>* <0 Negative errno (for testing errors) >>>@@ -403,6 +409,15 @@ void recv_buf(int fd, void *buf, size_t len, int flags, ssize_t expected_ret) >>> if (ret <= 0) >>> break; >>> >>>+ if (flags & MSG_PEEK) { >>>+ if (ret == expected_ret) { > >On second thought, I think it would be more appropriate to check for >`ret >= expected_ret` here, because all subsequent recv() will >definitely return more bytes, so there’s no point in continuing the >loop... and anyway, we’ll check the result later, so just that change >should be fine. > >And of course I'd update the comment on top in this way: > > * When MSG_PEEK is set, recv() is retried until it returns at least > * expected_ret bytes. The function returns on error, EOF, or timeout > * as usual. > >Thanks, >Stefano > Good idea, will do. Thanks! Luigi ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net v3 3/3] vsock/test: add MSG_PEEK after partial recv test 2026-04-14 16:10 [PATCH net v3 0/3] vsock/virtio: fix MSG_PEEK calculation on bytes to copy Luigi Leonardi 2026-04-14 16:10 ` [PATCH net v3 1/3] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating " Luigi Leonardi 2026-04-14 16:10 ` [PATCH net v3 2/3] vsock/test: fix MSG_PEEK handling in recv_buf() Luigi Leonardi @ 2026-04-14 16:10 ` Luigi Leonardi 2026-04-15 11:40 ` Stefano Garzarella 2 siblings, 1 reply; 8+ messages in thread From: Luigi Leonardi @ 2026-04-14 16:10 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 an 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 | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c index bdb0754965df..ab387a13f0ae 100644 --- a/tools/testing/vsock/vsock_test.c +++ b/tools/testing/vsock/vsock_test.c @@ -346,6 +346,38 @@ static void test_stream_msg_peek_server(const struct test_opts *opts) return test_msg_peek_server(opts, false); } +static void test_stream_peek_after_recv_server(const struct test_opts *opts) +{ + unsigned char buf_normal[MSG_PEEK_BUF_LEN]; + unsigned char buf_peek[MSG_PEEK_BUF_LEN]; + 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_normal, 1, 0, 1); + + /* Ask more bytes than available */ + recv_buf(fd, buf_peek, sizeof(buf_peek), MSG_PEEK, sizeof(buf_peek) - 1); + + /* Recv rest of the data */ + recv_buf(fd, buf_normal, sizeof(buf_normal) - 1, 0, sizeof(buf_normal) - 1); + + /* Compare full peek and normal read. */ + if (memcmp(buf_peek, buf_normal, sizeof(buf_peek) - 1)) { + fprintf(stderr, "Full peek data mismatch\n"); + exit(EXIT_FAILURE); + } + + close(fd); +} + #define SOCK_BUF_SIZE (2 * 1024 * 1024) #define SOCK_BUF_SIZE_SMALL (64 * 1024) #define MAX_MSG_PAGES 4 @@ -2509,6 +2541,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_msg_peek_client, + .run_server = test_stream_peek_after_recv_server, + }, {}, }; -- 2.53.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH net v3 3/3] vsock/test: add MSG_PEEK after partial recv test 2026-04-14 16:10 ` [PATCH net v3 3/3] vsock/test: add MSG_PEEK after partial recv test Luigi Leonardi @ 2026-04-15 11:40 ` Stefano Garzarella 0 siblings, 0 replies; 8+ messages in thread From: Stefano Garzarella @ 2026-04-15 11:40 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 Tue, Apr 14, 2026 at 06:10:23PM +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()` 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 an 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. nit: I think it's better to mention also that we are re-using test_stream_msg_peek_client() also for this test. > >Signed-off-by: Luigi Leonardi <leonardi@redhat.com> >--- > tools/testing/vsock/vsock_test.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > >diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c >index bdb0754965df..ab387a13f0ae 100644 >--- a/tools/testing/vsock/vsock_test.c >+++ b/tools/testing/vsock/vsock_test.c >@@ -346,6 +346,38 @@ static void test_stream_msg_peek_server(const struct test_opts *opts) > return test_msg_peek_server(opts, false); > } > >+static void test_stream_peek_after_recv_server(const struct test_opts *opts) >+{ >+ unsigned char buf_normal[MSG_PEEK_BUF_LEN]; >+ unsigned char buf_peek[MSG_PEEK_BUF_LEN]; >+ 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_normal, 1, 0, 1); >+ >+ /* Ask more bytes than available */ nit: /* Peek with a buffer larger than the remaining data */ >+ recv_buf(fd, buf_peek, sizeof(buf_peek), MSG_PEEK, sizeof(buf_peek) - 1); >+ >+ /* Recv rest of the data */ nit: /* Consume the remaining data */ >+ recv_buf(fd, buf_normal, sizeof(buf_normal) - 1, 0, sizeof(buf_normal) - 1); >+ >+ /* Compare full peek and normal read. */ >+ if (memcmp(buf_peek, buf_normal, sizeof(buf_peek) - 1)) { >+ fprintf(stderr, "Full peek data mismatch\n"); >+ exit(EXIT_FAILURE); >+ } >+ >+ close(fd); >+} >+ > #define SOCK_BUF_SIZE (2 * 1024 * 1024) > #define SOCK_BUF_SIZE_SMALL (64 * 1024) > #define MAX_MSG_PAGES 4 >@@ -2509,6 +2541,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_msg_peek_client, >+ .run_server = test_stream_peek_after_recv_server, I left just minor comments, the test LGTM: Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-04-15 13:11 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-14 16:10 [PATCH net v3 0/3] vsock/virtio: fix MSG_PEEK calculation on bytes to copy Luigi Leonardi 2026-04-14 16:10 ` [PATCH net v3 1/3] vsock/virtio: fix MSG_PEEK ignoring skb offset when calculating " Luigi Leonardi 2026-04-14 16:10 ` [PATCH net v3 2/3] vsock/test: fix MSG_PEEK handling in recv_buf() Luigi Leonardi 2026-04-15 11:31 ` Stefano Garzarella 2026-04-15 11:54 ` Stefano Garzarella 2026-04-15 13:11 ` Luigi Leonardi 2026-04-14 16:10 ` [PATCH net v3 3/3] vsock/test: add MSG_PEEK after partial recv test Luigi Leonardi 2026-04-15 11:40 ` Stefano Garzarella
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox