* [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers
@ 2026-03-05 12:56 Laurent Vivier
2026-03-06 0:05 ` David Gibson
2026-03-06 7:35 ` Stefano Brivio
0 siblings, 2 replies; 10+ messages in thread
From: Laurent Vivier @ 2026-03-05 12:56 UTC (permalink / raw)
To: passt-dev; +Cc: Laurent Vivier
Add a generic iov_truncate() function that truncates an IO vector to a
given number of bytes, returning the number of iov entries that contain
data after truncation.
Use it in udp_vu_sock_recv() and tcp_vu_sock_recv() to replace the
open-coded truncation logic that adjusted iov entries after recvmsg().
Also convert the direct iov_len assignment in tcp_vu_send_flag() to use
iov_truncate() for consistency.
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
Notes:
v3: use in tcp_vu_send_flag() too
v2: use iov_truncate() in udp_vu_sock_recv() too
iov.c | 22 ++++++++++++++++++++++
iov.h | 1 +
tcp_vu.c | 14 +++-----------
udp_vu.c | 12 +++---------
4 files changed, 29 insertions(+), 20 deletions(-)
diff --git a/iov.c b/iov.c
index ad726daa4cd8..31a3f5bc29e5 100644
--- a/iov.c
+++ b/iov.c
@@ -147,6 +147,28 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
return len;
}
+/**
+ * iov_truncate() - Truncate an IO vector to a given number of bytes
+ * @iov: IO vector (modified)
+ * @iov_cnt: Number of entries in @iov
+ * @size: Total number of bytes to keep
+ *
+ * Return: number of iov entries that contain data after truncation
+ */
+size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size)
+{
+ size_t i, offset;
+
+ i = iov_skip_bytes(iov, iov_cnt, size, &offset);
+
+ if (i < iov_cnt) {
+ iov[i].iov_len = offset;
+ i += !!offset;
+ }
+
+ return i;
+}
+
/**
* iov_tail_prune() - Remove any unneeded buffers from an IOV tail
* @tail: IO vector tail (modified)
diff --git a/iov.h b/iov.h
index d1ab91a94e22..b4e50b0fca5a 100644
--- a/iov.h
+++ b/iov.h
@@ -29,6 +29,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
size_t offset, void *buf, size_t bytes);
size_t iov_size(const struct iovec *iov, size_t iov_cnt);
+size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size);
/*
* DOC: Theory of Operation, struct iov_tail
diff --git a/tcp_vu.c b/tcp_vu.c
index 88be232dca66..8ca4170f13f6 100644
--- a/tcp_vu.c
+++ b/tcp_vu.c
@@ -131,7 +131,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
return ret;
}
- flags_elem[0].in_sg[0].iov_len = hdrlen + optlen;
+ iov_truncate(&flags_iov[0], 1, hdrlen + optlen);
payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
if (flags & KEEPALIVE)
@@ -192,9 +192,9 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
struct msghdr mh_sock = { 0 };
uint16_t mss = MSS_GET(conn);
int s = conn->sock;
- ssize_t ret, len;
size_t hdrlen;
int elem_cnt;
+ ssize_t ret;
int i;
*iov_cnt = 0;
@@ -247,15 +247,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
ret -= already_sent;
/* adjust iov number and length of the last iov */
- len = ret;
- for (i = 0; len && i < elem_cnt; i++) {
- struct iovec *iov = &elem[i].in_sg[0];
-
- if (iov->iov_len > (size_t)len)
- iov->iov_len = len;
-
- len -= iov->iov_len;
- }
+ i = iov_truncate(&iov_vu[DISCARD_IOV_NUM], elem_cnt, ret);
/* adjust head count */
while (*head_cnt > 0 && head[*head_cnt - 1] >= i)
diff --git a/udp_vu.c b/udp_vu.c
index 3520f89e5671..5effca777e0a 100644
--- a/udp_vu.c
+++ b/udp_vu.c
@@ -71,9 +71,9 @@ static int udp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq, int s,
bool v6, ssize_t *dlen)
{
const struct vu_dev *vdev = c->vdev;
- int iov_cnt, idx, iov_used;
- size_t off, hdrlen, l2len;
struct msghdr msg = { 0 };
+ int iov_cnt, iov_used;
+ size_t hdrlen, l2len;
ASSERT(!c->no_udp);
@@ -115,13 +115,7 @@ static int udp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq, int s,
iov_vu[0].iov_base = (char *)iov_vu[0].iov_base - hdrlen;
iov_vu[0].iov_len += hdrlen;
- /* count the numbers of buffer filled by recvmsg() */
- idx = iov_skip_bytes(iov_vu, iov_cnt, *dlen + hdrlen, &off);
-
- /* adjust last iov length */
- if (idx < iov_cnt)
- iov_vu[idx].iov_len = off;
- iov_used = idx + !!off;
+ iov_used = iov_truncate(iov_vu, iov_cnt, *dlen + hdrlen);
/* pad frame to 60 bytes: first buffer is at least ETH_ZLEN long */
l2len = *dlen + hdrlen - VNET_HLEN;
--
2.53.0
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers
2026-03-05 12:56 [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers Laurent Vivier
@ 2026-03-06 0:05 ` David Gibson
2026-03-06 7:35 ` Stefano Brivio
1 sibling, 0 replies; 10+ messages in thread
From: David Gibson @ 2026-03-06 0:05 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 4963 bytes --]
On Thu, Mar 05, 2026 at 01:56:48PM +0100, Laurent Vivier wrote:
> Add a generic iov_truncate() function that truncates an IO vector to a
> given number of bytes, returning the number of iov entries that contain
> data after truncation.
>
> Use it in udp_vu_sock_recv() and tcp_vu_sock_recv() to replace the
> open-coded truncation logic that adjusted iov entries after recvmsg().
> Also convert the direct iov_len assignment in tcp_vu_send_flag() to use
> iov_truncate() for consistency.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>
> Notes:
> v3: use in tcp_vu_send_flag() too
> v2: use iov_truncate() in udp_vu_sock_recv() too
>
> iov.c | 22 ++++++++++++++++++++++
> iov.h | 1 +
> tcp_vu.c | 14 +++-----------
> udp_vu.c | 12 +++---------
> 4 files changed, 29 insertions(+), 20 deletions(-)
>
> diff --git a/iov.c b/iov.c
> index ad726daa4cd8..31a3f5bc29e5 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -147,6 +147,28 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
> return len;
> }
>
> +/**
> + * iov_truncate() - Truncate an IO vector to a given number of bytes
> + * @iov: IO vector (modified)
> + * @iov_cnt: Number of entries in @iov
> + * @size: Total number of bytes to keep
> + *
> + * Return: number of iov entries that contain data after truncation
> + */
> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size)
> +{
> + size_t i, offset;
> +
> + i = iov_skip_bytes(iov, iov_cnt, size, &offset);
> +
> + if (i < iov_cnt) {
> + iov[i].iov_len = offset;
> + i += !!offset;
> + }
> +
> + return i;
> +}
> +
> /**
> * iov_tail_prune() - Remove any unneeded buffers from an IOV tail
> * @tail: IO vector tail (modified)
> diff --git a/iov.h b/iov.h
> index d1ab91a94e22..b4e50b0fca5a 100644
> --- a/iov.h
> +++ b/iov.h
> @@ -29,6 +29,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
> size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
> size_t offset, void *buf, size_t bytes);
> size_t iov_size(const struct iovec *iov, size_t iov_cnt);
> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size);
>
> /*
> * DOC: Theory of Operation, struct iov_tail
> diff --git a/tcp_vu.c b/tcp_vu.c
> index 88be232dca66..8ca4170f13f6 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -131,7 +131,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
> return ret;
> }
>
> - flags_elem[0].in_sg[0].iov_len = hdrlen + optlen;
> + iov_truncate(&flags_iov[0], 1, hdrlen + optlen);
> payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
>
> if (flags & KEEPALIVE)
> @@ -192,9 +192,9 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
> struct msghdr mh_sock = { 0 };
> uint16_t mss = MSS_GET(conn);
> int s = conn->sock;
> - ssize_t ret, len;
> size_t hdrlen;
> int elem_cnt;
> + ssize_t ret;
> int i;
>
> *iov_cnt = 0;
> @@ -247,15 +247,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
> ret -= already_sent;
>
> /* adjust iov number and length of the last iov */
> - len = ret;
> - for (i = 0; len && i < elem_cnt; i++) {
> - struct iovec *iov = &elem[i].in_sg[0];
> -
> - if (iov->iov_len > (size_t)len)
> - iov->iov_len = len;
> -
> - len -= iov->iov_len;
> - }
> + i = iov_truncate(&iov_vu[DISCARD_IOV_NUM], elem_cnt, ret);
>
> /* adjust head count */
> while (*head_cnt > 0 && head[*head_cnt - 1] >= i)
> diff --git a/udp_vu.c b/udp_vu.c
> index 3520f89e5671..5effca777e0a 100644
> --- a/udp_vu.c
> +++ b/udp_vu.c
> @@ -71,9 +71,9 @@ static int udp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq, int s,
> bool v6, ssize_t *dlen)
> {
> const struct vu_dev *vdev = c->vdev;
> - int iov_cnt, idx, iov_used;
> - size_t off, hdrlen, l2len;
> struct msghdr msg = { 0 };
> + int iov_cnt, iov_used;
> + size_t hdrlen, l2len;
>
> ASSERT(!c->no_udp);
>
> @@ -115,13 +115,7 @@ static int udp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq, int s,
> iov_vu[0].iov_base = (char *)iov_vu[0].iov_base - hdrlen;
> iov_vu[0].iov_len += hdrlen;
>
> - /* count the numbers of buffer filled by recvmsg() */
> - idx = iov_skip_bytes(iov_vu, iov_cnt, *dlen + hdrlen, &off);
> -
> - /* adjust last iov length */
> - if (idx < iov_cnt)
> - iov_vu[idx].iov_len = off;
> - iov_used = idx + !!off;
> + iov_used = iov_truncate(iov_vu, iov_cnt, *dlen + hdrlen);
>
> /* pad frame to 60 bytes: first buffer is at least ETH_ZLEN long */
> l2len = *dlen + hdrlen - VNET_HLEN;
> --
> 2.53.0
>
--
David Gibson (he or they) | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers
2026-03-05 12:56 [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers Laurent Vivier
2026-03-06 0:05 ` David Gibson
@ 2026-03-06 7:35 ` Stefano Brivio
2026-03-06 8:17 ` Laurent Vivier
1 sibling, 1 reply; 10+ messages in thread
From: Stefano Brivio @ 2026-03-06 7:35 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev, David Gibson
On Thu, 5 Mar 2026 13:56:48 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> Add a generic iov_truncate() function that truncates an IO vector to a
> given number of bytes, returning the number of iov entries that contain
> data after truncation.
>
> Use it in udp_vu_sock_recv() and tcp_vu_sock_recv() to replace the
> open-coded truncation logic that adjusted iov entries after recvmsg().
> Also convert the direct iov_len assignment in tcp_vu_send_flag() to use
> iov_truncate() for consistency.
>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>
> Notes:
> v3: use in tcp_vu_send_flag() too
> v2: use iov_truncate() in udp_vu_sock_recv() too
>
> iov.c | 22 ++++++++++++++++++++++
> iov.h | 1 +
> tcp_vu.c | 14 +++-----------
> udp_vu.c | 12 +++---------
> 4 files changed, 29 insertions(+), 20 deletions(-)
>
> diff --git a/iov.c b/iov.c
> index ad726daa4cd8..31a3f5bc29e5 100644
> --- a/iov.c
> +++ b/iov.c
> @@ -147,6 +147,28 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
> return len;
> }
>
> +/**
> + * iov_truncate() - Truncate an IO vector to a given number of bytes
> + * @iov: IO vector (modified)
> + * @iov_cnt: Number of entries in @iov
> + * @size: Total number of bytes to keep
> + *
> + * Return: number of iov entries that contain data after truncation
> + */
> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size)
> +{
> + size_t i, offset;
> +
> + i = iov_skip_bytes(iov, iov_cnt, size, &offset);
> +
> + if (i < iov_cnt) {
> + iov[i].iov_len = offset;
> + i += !!offset;
> + }
> +
> + return i;
> +}
> +
> /**
> * iov_tail_prune() - Remove any unneeded buffers from an IOV tail
> * @tail: IO vector tail (modified)
> diff --git a/iov.h b/iov.h
> index d1ab91a94e22..b4e50b0fca5a 100644
> --- a/iov.h
> +++ b/iov.h
> @@ -29,6 +29,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
> size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
> size_t offset, void *buf, size_t bytes);
> size_t iov_size(const struct iovec *iov, size_t iov_cnt);
> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size);
>
> /*
> * DOC: Theory of Operation, struct iov_tail
> diff --git a/tcp_vu.c b/tcp_vu.c
> index 88be232dca66..8ca4170f13f6 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -131,7 +131,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
> return ret;
> }
>
> - flags_elem[0].in_sg[0].iov_len = hdrlen + optlen;
> + iov_truncate(&flags_iov[0], 1, hdrlen + optlen);
> payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
>
> if (flags & KEEPALIVE)
> @@ -192,9 +192,9 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
> struct msghdr mh_sock = { 0 };
> uint16_t mss = MSS_GET(conn);
> int s = conn->sock;
> - ssize_t ret, len;
> size_t hdrlen;
> int elem_cnt;
> + ssize_t ret;
> int i;
>
> *iov_cnt = 0;
> @@ -247,15 +247,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
> ret -= already_sent;
>
> /* adjust iov number and length of the last iov */
> - len = ret;
> - for (i = 0; len && i < elem_cnt; i++) {
> - struct iovec *iov = &elem[i].in_sg[0];
> -
> - if (iov->iov_len > (size_t)len)
> - iov->iov_len = len;
> -
> - len -= iov->iov_len;
> - }
> + i = iov_truncate(&iov_vu[DISCARD_IOV_NUM], elem_cnt, ret);
I had a quick look, but I couldn't figure this out. This causes
Coverity Scan to report:
/home/sbrivio/passt/tcp_vu.c:457:3:
Type: Overflowed constant (INTEGER_OVERFLOW)
/home/sbrivio/passt/tcp_vu.c:355:2:
1. path: Condition "!vu_queue_enabled(vq)", taking false branch.
/home/sbrivio/passt/tcp_vu.c:355:2:
2. path: Condition "!vu_queue_started(vq)", taking false branch.
/home/sbrivio/passt/tcp_vu.c:362:2:
3. path: Condition "0U /* (uint32_t)0 */ - (uint32_t)already_sent - 1 < (16777216U /* 1 << 16 + 8 */)", taking false branch.
/home/sbrivio/passt/tcp_vu.c:374:2:
4. path: Condition "!wnd_scaled", taking false branch.
/home/sbrivio/passt/tcp_vu.c:374:2:
5. path: Condition "already_sent >= wnd_scaled", taking false branch.
/home/sbrivio/passt/tcp_vu.c:388:2:
6. path: Condition "v6", taking true branch.
/home/sbrivio/passt/tcp_vu.c:390:2:
7. path: Condition "len < 0", taking false branch.
/home/sbrivio/passt/tcp_vu.c:402:2:
8. path: Condition "!len", taking false branch.
/home/sbrivio/passt/tcp_vu.c:425:2:
9. path: Condition "log_trace", taking true branch.
/home/sbrivio/passt/tcp_vu.c:426:2:
10. path: Condition "log_trace", taking true branch.
/home/sbrivio/passt/tcp_vu.c:439:2:
11. path: Condition "v6", taking true branch.
/home/sbrivio/passt/tcp_vu.c:439:2:
12. function_return: Function "tcp_vu_hdrlen(v6)" returns 86.
/home/sbrivio/passt/tcp_vu.c:439:2:
13. known_value_assign: "hdrlen" = "tcp_vu_hdrlen(v6)", its value is now 86.
/home/sbrivio/passt/tcp_vu.c:440:2:
14. path: Condition "i < head_cnt", taking true branch.
/home/sbrivio/passt/tcp_vu.c:443:3:
15. function_return: Function "iov_size(iov, buf_cnt)" returns 0.
/home/sbrivio/passt/tcp_vu.c:443:3:
16. known_value_assign: "dlen" = "iov_size(iov, buf_cnt) - hdrlen", its value is now 18446744073709551530.
/home/sbrivio/passt/tcp_vu.c:450:3:
17. path: Condition "previous_dlen != dlen", taking true branch.
/home/sbrivio/passt/tcp_vu.c:454:3:
18. path: Condition "!*c->pcap", taking false branch.
/home/sbrivio/passt/tcp_vu.c:457:3:
19. overflow_const: Expression "dlen + hdrlen", where "dlen" is known to be equal to -86, and "hdrlen" is known to be equal to 86, underflows the type of "dlen + hdrlen", which is type "unsigned long".
--
Stefano
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers
2026-03-06 7:35 ` Stefano Brivio
@ 2026-03-06 8:17 ` Laurent Vivier
2026-03-06 8:25 ` Stefano Brivio
0 siblings, 1 reply; 10+ messages in thread
From: Laurent Vivier @ 2026-03-06 8:17 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev, David Gibson
On 3/6/26 08:35, Stefano Brivio wrote:
> On Thu, 5 Mar 2026 13:56:48 +0100
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> Add a generic iov_truncate() function that truncates an IO vector to a
>> given number of bytes, returning the number of iov entries that contain
>> data after truncation.
>>
>> Use it in udp_vu_sock_recv() and tcp_vu_sock_recv() to replace the
>> open-coded truncation logic that adjusted iov entries after recvmsg().
>> Also convert the direct iov_len assignment in tcp_vu_send_flag() to use
>> iov_truncate() for consistency.
>>
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>> ---
>>
>> Notes:
>> v3: use in tcp_vu_send_flag() too
>> v2: use iov_truncate() in udp_vu_sock_recv() too
>>
>> iov.c | 22 ++++++++++++++++++++++
>> iov.h | 1 +
>> tcp_vu.c | 14 +++-----------
>> udp_vu.c | 12 +++---------
>> 4 files changed, 29 insertions(+), 20 deletions(-)
>>
>> diff --git a/iov.c b/iov.c
>> index ad726daa4cd8..31a3f5bc29e5 100644
>> --- a/iov.c
>> +++ b/iov.c
>> @@ -147,6 +147,28 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
>> return len;
>> }
>>
>> +/**
>> + * iov_truncate() - Truncate an IO vector to a given number of bytes
>> + * @iov: IO vector (modified)
>> + * @iov_cnt: Number of entries in @iov
>> + * @size: Total number of bytes to keep
>> + *
>> + * Return: number of iov entries that contain data after truncation
>> + */
>> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size)
>> +{
>> + size_t i, offset;
>> +
>> + i = iov_skip_bytes(iov, iov_cnt, size, &offset);
>> +
>> + if (i < iov_cnt) {
>> + iov[i].iov_len = offset;
>> + i += !!offset;
>> + }
>> +
>> + return i;
>> +}
>> +
>> /**
>> * iov_tail_prune() - Remove any unneeded buffers from an IOV tail
>> * @tail: IO vector tail (modified)
>> diff --git a/iov.h b/iov.h
>> index d1ab91a94e22..b4e50b0fca5a 100644
>> --- a/iov.h
>> +++ b/iov.h
>> @@ -29,6 +29,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
>> size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
>> size_t offset, void *buf, size_t bytes);
>> size_t iov_size(const struct iovec *iov, size_t iov_cnt);
>> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size);
>>
>> /*
>> * DOC: Theory of Operation, struct iov_tail
>> diff --git a/tcp_vu.c b/tcp_vu.c
>> index 88be232dca66..8ca4170f13f6 100644
>> --- a/tcp_vu.c
>> +++ b/tcp_vu.c
>> @@ -131,7 +131,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
>> return ret;
>> }
>>
>> - flags_elem[0].in_sg[0].iov_len = hdrlen + optlen;
>> + iov_truncate(&flags_iov[0], 1, hdrlen + optlen);
>> payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
>>
>> if (flags & KEEPALIVE)
>> @@ -192,9 +192,9 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
>> struct msghdr mh_sock = { 0 };
>> uint16_t mss = MSS_GET(conn);
>> int s = conn->sock;
>> - ssize_t ret, len;
>> size_t hdrlen;
>> int elem_cnt;
>> + ssize_t ret;
>> int i;
>>
>> *iov_cnt = 0;
>> @@ -247,15 +247,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
>> ret -= already_sent;
>>
>> /* adjust iov number and length of the last iov */
>> - len = ret;
>> - for (i = 0; len && i < elem_cnt; i++) {
>> - struct iovec *iov = &elem[i].in_sg[0];
>> -
>> - if (iov->iov_len > (size_t)len)
>> - iov->iov_len = len;
>> -
>> - len -= iov->iov_len;
>> - }
>> + i = iov_truncate(&iov_vu[DISCARD_IOV_NUM], elem_cnt, ret);
>
> I had a quick look, but I couldn't figure this out. This causes
> Coverity Scan to report:
>
> /home/sbrivio/passt/tcp_vu.c:457:3:
> Type: Overflowed constant (INTEGER_OVERFLOW)
>
> /home/sbrivio/passt/tcp_vu.c:355:2:
> 1. path: Condition "!vu_queue_enabled(vq)", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:355:2:
> 2. path: Condition "!vu_queue_started(vq)", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:362:2:
> 3. path: Condition "0U /* (uint32_t)0 */ - (uint32_t)already_sent - 1 < (16777216U /* 1 << 16 + 8 */)", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:374:2:
> 4. path: Condition "!wnd_scaled", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:374:2:
> 5. path: Condition "already_sent >= wnd_scaled", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:388:2:
> 6. path: Condition "v6", taking true branch.
> /home/sbrivio/passt/tcp_vu.c:390:2:
> 7. path: Condition "len < 0", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:402:2:
> 8. path: Condition "!len", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:425:2:
> 9. path: Condition "log_trace", taking true branch.
> /home/sbrivio/passt/tcp_vu.c:426:2:
> 10. path: Condition "log_trace", taking true branch.
> /home/sbrivio/passt/tcp_vu.c:439:2:
> 11. path: Condition "v6", taking true branch.
> /home/sbrivio/passt/tcp_vu.c:439:2:
> 12. function_return: Function "tcp_vu_hdrlen(v6)" returns 86.
> /home/sbrivio/passt/tcp_vu.c:439:2:
> 13. known_value_assign: "hdrlen" = "tcp_vu_hdrlen(v6)", its value is now 86.
> /home/sbrivio/passt/tcp_vu.c:440:2:
> 14. path: Condition "i < head_cnt", taking true branch.
> /home/sbrivio/passt/tcp_vu.c:443:3:
> 15. function_return: Function "iov_size(iov, buf_cnt)" returns 0.
> /home/sbrivio/passt/tcp_vu.c:443:3:
> 16. known_value_assign: "dlen" = "iov_size(iov, buf_cnt) - hdrlen", its value is now 18446744073709551530.
> /home/sbrivio/passt/tcp_vu.c:450:3:
> 17. path: Condition "previous_dlen != dlen", taking true branch.
> /home/sbrivio/passt/tcp_vu.c:454:3:
> 18. path: Condition "!*c->pcap", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:457:3:
> 19. overflow_const: Expression "dlen + hdrlen", where "dlen" is known to be equal to -86, and "hdrlen" is known to be equal to 86, underflows the type of "dlen + hdrlen", which is type "unsigned long".
>
if iov_size(iov, buf_cnt) = 0 and hdrlen = 86 (all unsigned)
"dlen" = "iov_size(iov, buf_cnt) - hdrlen", its value is now 18446744073709551530 (see
16.) (i.e. -hdrlen, unsigned -86)
so "dlen + hdrlen" overflows (I guess).
Try:
diff --git a/tcp_vu.c b/tcp_vu.c
index 8ca4170f13f6..787ee004a66a 100644
--- a/tcp_vu.c
+++ b/tcp_vu.c
@@ -440,7 +440,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
for (i = 0, previous_dlen = -1, check = NULL; i < head_cnt; i++) {
struct iovec *iov = &elem[head[i]].in_sg[0];
int buf_cnt = head[i + 1] - head[i];
- ssize_t dlen = iov_size(iov, buf_cnt) - hdrlen;
+ ssize_t dlen = (ssize_t)iov_size(iov, buf_cnt) - hdrlen;
bool push = i == head_cnt - 1;
size_t l2len;
Thanks,
Laurent
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers
2026-03-06 8:17 ` Laurent Vivier
@ 2026-03-06 8:25 ` Stefano Brivio
2026-03-06 8:51 ` Laurent Vivier
0 siblings, 1 reply; 10+ messages in thread
From: Stefano Brivio @ 2026-03-06 8:25 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev, David Gibson
On Fri, 6 Mar 2026 09:17:32 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> On 3/6/26 08:35, Stefano Brivio wrote:
> > On Thu, 5 Mar 2026 13:56:48 +0100
> > Laurent Vivier <lvivier@redhat.com> wrote:
> >
> >> Add a generic iov_truncate() function that truncates an IO vector to a
> >> given number of bytes, returning the number of iov entries that contain
> >> data after truncation.
> >>
> >> Use it in udp_vu_sock_recv() and tcp_vu_sock_recv() to replace the
> >> open-coded truncation logic that adjusted iov entries after recvmsg().
> >> Also convert the direct iov_len assignment in tcp_vu_send_flag() to use
> >> iov_truncate() for consistency.
> >>
> >> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >> ---
> >>
> >> Notes:
> >> v3: use in tcp_vu_send_flag() too
> >> v2: use iov_truncate() in udp_vu_sock_recv() too
> >>
> >> iov.c | 22 ++++++++++++++++++++++
> >> iov.h | 1 +
> >> tcp_vu.c | 14 +++-----------
> >> udp_vu.c | 12 +++---------
> >> 4 files changed, 29 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/iov.c b/iov.c
> >> index ad726daa4cd8..31a3f5bc29e5 100644
> >> --- a/iov.c
> >> +++ b/iov.c
> >> @@ -147,6 +147,28 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
> >> return len;
> >> }
> >>
> >> +/**
> >> + * iov_truncate() - Truncate an IO vector to a given number of bytes
> >> + * @iov: IO vector (modified)
> >> + * @iov_cnt: Number of entries in @iov
> >> + * @size: Total number of bytes to keep
> >> + *
> >> + * Return: number of iov entries that contain data after truncation
> >> + */
> >> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size)
> >> +{
> >> + size_t i, offset;
> >> +
> >> + i = iov_skip_bytes(iov, iov_cnt, size, &offset);
> >> +
> >> + if (i < iov_cnt) {
> >> + iov[i].iov_len = offset;
> >> + i += !!offset;
> >> + }
> >> +
> >> + return i;
> >> +}
> >> +
> >> /**
> >> * iov_tail_prune() - Remove any unneeded buffers from an IOV tail
> >> * @tail: IO vector tail (modified)
> >> diff --git a/iov.h b/iov.h
> >> index d1ab91a94e22..b4e50b0fca5a 100644
> >> --- a/iov.h
> >> +++ b/iov.h
> >> @@ -29,6 +29,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
> >> size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
> >> size_t offset, void *buf, size_t bytes);
> >> size_t iov_size(const struct iovec *iov, size_t iov_cnt);
> >> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size);
> >>
> >> /*
> >> * DOC: Theory of Operation, struct iov_tail
> >> diff --git a/tcp_vu.c b/tcp_vu.c
> >> index 88be232dca66..8ca4170f13f6 100644
> >> --- a/tcp_vu.c
> >> +++ b/tcp_vu.c
> >> @@ -131,7 +131,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
> >> return ret;
> >> }
> >>
> >> - flags_elem[0].in_sg[0].iov_len = hdrlen + optlen;
> >> + iov_truncate(&flags_iov[0], 1, hdrlen + optlen);
> >> payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
> >>
> >> if (flags & KEEPALIVE)
> >> @@ -192,9 +192,9 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
> >> struct msghdr mh_sock = { 0 };
> >> uint16_t mss = MSS_GET(conn);
> >> int s = conn->sock;
> >> - ssize_t ret, len;
> >> size_t hdrlen;
> >> int elem_cnt;
> >> + ssize_t ret;
> >> int i;
> >>
> >> *iov_cnt = 0;
> >> @@ -247,15 +247,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
> >> ret -= already_sent;
> >>
> >> /* adjust iov number and length of the last iov */
> >> - len = ret;
> >> - for (i = 0; len && i < elem_cnt; i++) {
> >> - struct iovec *iov = &elem[i].in_sg[0];
> >> -
> >> - if (iov->iov_len > (size_t)len)
> >> - iov->iov_len = len;
> >> -
> >> - len -= iov->iov_len;
> >> - }
> >> + i = iov_truncate(&iov_vu[DISCARD_IOV_NUM], elem_cnt, ret);
> >
> > I had a quick look, but I couldn't figure this out. This causes
> > Coverity Scan to report:
> >
> > /home/sbrivio/passt/tcp_vu.c:457:3:
> > Type: Overflowed constant (INTEGER_OVERFLOW)
> >
> > /home/sbrivio/passt/tcp_vu.c:355:2:
> > 1. path: Condition "!vu_queue_enabled(vq)", taking false branch.
> > /home/sbrivio/passt/tcp_vu.c:355:2:
> > 2. path: Condition "!vu_queue_started(vq)", taking false branch.
> > /home/sbrivio/passt/tcp_vu.c:362:2:
> > 3. path: Condition "0U /* (uint32_t)0 */ - (uint32_t)already_sent - 1 < (16777216U /* 1 << 16 + 8 */)", taking false branch.
> > /home/sbrivio/passt/tcp_vu.c:374:2:
> > 4. path: Condition "!wnd_scaled", taking false branch.
> > /home/sbrivio/passt/tcp_vu.c:374:2:
> > 5. path: Condition "already_sent >= wnd_scaled", taking false branch.
> > /home/sbrivio/passt/tcp_vu.c:388:2:
> > 6. path: Condition "v6", taking true branch.
> > /home/sbrivio/passt/tcp_vu.c:390:2:
> > 7. path: Condition "len < 0", taking false branch.
> > /home/sbrivio/passt/tcp_vu.c:402:2:
> > 8. path: Condition "!len", taking false branch.
> > /home/sbrivio/passt/tcp_vu.c:425:2:
> > 9. path: Condition "log_trace", taking true branch.
> > /home/sbrivio/passt/tcp_vu.c:426:2:
> > 10. path: Condition "log_trace", taking true branch.
> > /home/sbrivio/passt/tcp_vu.c:439:2:
> > 11. path: Condition "v6", taking true branch.
> > /home/sbrivio/passt/tcp_vu.c:439:2:
> > 12. function_return: Function "tcp_vu_hdrlen(v6)" returns 86.
> > /home/sbrivio/passt/tcp_vu.c:439:2:
> > 13. known_value_assign: "hdrlen" = "tcp_vu_hdrlen(v6)", its value is now 86.
> > /home/sbrivio/passt/tcp_vu.c:440:2:
> > 14. path: Condition "i < head_cnt", taking true branch.
> > /home/sbrivio/passt/tcp_vu.c:443:3:
> > 15. function_return: Function "iov_size(iov, buf_cnt)" returns 0.
> > /home/sbrivio/passt/tcp_vu.c:443:3:
> > 16. known_value_assign: "dlen" = "iov_size(iov, buf_cnt) - hdrlen", its value is now 18446744073709551530.
> > /home/sbrivio/passt/tcp_vu.c:450:3:
> > 17. path: Condition "previous_dlen != dlen", taking true branch.
> > /home/sbrivio/passt/tcp_vu.c:454:3:
> > 18. path: Condition "!*c->pcap", taking false branch.
> > /home/sbrivio/passt/tcp_vu.c:457:3:
> > 19. overflow_const: Expression "dlen + hdrlen", where "dlen" is known to be equal to -86, and "hdrlen" is known to be equal to 86, underflows the type of "dlen + hdrlen", which is type "unsigned long".
> >
>
> if iov_size(iov, buf_cnt) = 0 and hdrlen = 86 (all unsigned)
> "dlen" = "iov_size(iov, buf_cnt) - hdrlen", its value is now 18446744073709551530 (see
> 16.) (i.e. -hdrlen, unsigned -86)
> so "dlen + hdrlen" overflows (I guess).
I was also thinking it was something like that but:
>
> Try:
> diff --git a/tcp_vu.c b/tcp_vu.c
> index 8ca4170f13f6..787ee004a66a 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -440,7 +440,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
> for (i = 0, previous_dlen = -1, check = NULL; i < head_cnt; i++) {
> struct iovec *iov = &elem[head[i]].in_sg[0];
> int buf_cnt = head[i + 1] - head[i];
> - ssize_t dlen = iov_size(iov, buf_cnt) - hdrlen;
> + ssize_t dlen = (ssize_t)iov_size(iov, buf_cnt) - hdrlen;
> bool push = i == head_cnt - 1;
> size_t l2len;
...still not happy because of how we use dlen later (I think?):
/home/sbrivio/passt/tcp_vu.c:457:3:
Type: Overflowed constant (INTEGER_OVERFLOW)
/home/sbrivio/passt/tcp_vu.c:355:2:
1. path: Condition "!vu_queue_enabled(vq)", taking false branch.
/home/sbrivio/passt/tcp_vu.c:355:2:
2. path: Condition "!vu_queue_started(vq)", taking false branch.
/home/sbrivio/passt/tcp_vu.c:362:2:
3. path: Condition "0U /* (uint32_t)0 */ - (uint32_t)already_sent - 1 < (16777216U /* 1 << 16 + 8 */)", taking false branch.
/home/sbrivio/passt/tcp_vu.c:374:2:
4. path: Condition "!wnd_scaled", taking false branch.
/home/sbrivio/passt/tcp_vu.c:374:2:
5. path: Condition "already_sent >= wnd_scaled", taking false branch.
/home/sbrivio/passt/tcp_vu.c:388:2:
6. path: Condition "v6", taking true branch.
/home/sbrivio/passt/tcp_vu.c:390:2:
7. path: Condition "len < 0", taking false branch.
/home/sbrivio/passt/tcp_vu.c:402:2:
8. path: Condition "!len", taking false branch.
/home/sbrivio/passt/tcp_vu.c:425:2:
9. path: Condition "log_trace", taking true branch.
/home/sbrivio/passt/tcp_vu.c:426:2:
10. path: Condition "log_trace", taking true branch.
/home/sbrivio/passt/tcp_vu.c:439:2:
11. path: Condition "v6", taking true branch.
/home/sbrivio/passt/tcp_vu.c:440:2:
12. path: Condition "i < head_cnt", taking true branch.
/home/sbrivio/passt/tcp_vu.c:443:3:
13. known_value_assign: "dlen" = "(ssize_t)iov_size(iov, buf_cnt) - hdrlen", its value is now 18446744073709551530.
/home/sbrivio/passt/tcp_vu.c:450:3:
14. path: Condition "previous_dlen != dlen", taking true branch.
/home/sbrivio/passt/tcp_vu.c:454:3:
15. path: Condition "!*c->pcap", taking false branch.
/home/sbrivio/passt/tcp_vu.c:457:3:
16. overflow_const: Expression "dlen + hdrlen", where "dlen" is known to be equal to -86, and "hdrlen" is known to be equal to 86, underflows the type of "dlen + hdrlen", which is type "unsigned long".
/home/sbrivio/passt/conf.c:2373:4:
Type: Untrusted value as argument (TAINTED_SCALAR)
--
Stefano
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers
2026-03-06 8:25 ` Stefano Brivio
@ 2026-03-06 8:51 ` Laurent Vivier
2026-03-06 10:41 ` Laurent Vivier
0 siblings, 1 reply; 10+ messages in thread
From: Laurent Vivier @ 2026-03-06 8:51 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev, David Gibson
On 3/6/26 09:25, Stefano Brivio wrote:
> On Fri, 6 Mar 2026 09:17:32 +0100
> Laurent Vivier <lvivier@redhat.com> wrote:
>
>> On 3/6/26 08:35, Stefano Brivio wrote:
>>> On Thu, 5 Mar 2026 13:56:48 +0100
>>> Laurent Vivier <lvivier@redhat.com> wrote:
>>>
>>>> Add a generic iov_truncate() function that truncates an IO vector to a
>>>> given number of bytes, returning the number of iov entries that contain
>>>> data after truncation.
>>>>
>>>> Use it in udp_vu_sock_recv() and tcp_vu_sock_recv() to replace the
>>>> open-coded truncation logic that adjusted iov entries after recvmsg().
>>>> Also convert the direct iov_len assignment in tcp_vu_send_flag() to use
>>>> iov_truncate() for consistency.
>>>>
>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>> ---
>>>>
>>>> Notes:
>>>> v3: use in tcp_vu_send_flag() too
>>>> v2: use iov_truncate() in udp_vu_sock_recv() too
>>>>
>>>> iov.c | 22 ++++++++++++++++++++++
>>>> iov.h | 1 +
>>>> tcp_vu.c | 14 +++-----------
>>>> udp_vu.c | 12 +++---------
>>>> 4 files changed, 29 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/iov.c b/iov.c
>>>> index ad726daa4cd8..31a3f5bc29e5 100644
>>>> --- a/iov.c
>>>> +++ b/iov.c
>>>> @@ -147,6 +147,28 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
>>>> return len;
>>>> }
>>>>
>>>> +/**
>>>> + * iov_truncate() - Truncate an IO vector to a given number of bytes
>>>> + * @iov: IO vector (modified)
>>>> + * @iov_cnt: Number of entries in @iov
>>>> + * @size: Total number of bytes to keep
>>>> + *
>>>> + * Return: number of iov entries that contain data after truncation
>>>> + */
>>>> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size)
>>>> +{
>>>> + size_t i, offset;
>>>> +
>>>> + i = iov_skip_bytes(iov, iov_cnt, size, &offset);
>>>> +
>>>> + if (i < iov_cnt) {
>>>> + iov[i].iov_len = offset;
>>>> + i += !!offset;
>>>> + }
>>>> +
>>>> + return i;
>>>> +}
>>>> +
>>>> /**
>>>> * iov_tail_prune() - Remove any unneeded buffers from an IOV tail
>>>> * @tail: IO vector tail (modified)
>>>> diff --git a/iov.h b/iov.h
>>>> index d1ab91a94e22..b4e50b0fca5a 100644
>>>> --- a/iov.h
>>>> +++ b/iov.h
>>>> @@ -29,6 +29,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
>>>> size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
>>>> size_t offset, void *buf, size_t bytes);
>>>> size_t iov_size(const struct iovec *iov, size_t iov_cnt);
>>>> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size);
>>>>
>>>> /*
>>>> * DOC: Theory of Operation, struct iov_tail
>>>> diff --git a/tcp_vu.c b/tcp_vu.c
>>>> index 88be232dca66..8ca4170f13f6 100644
>>>> --- a/tcp_vu.c
>>>> +++ b/tcp_vu.c
>>>> @@ -131,7 +131,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
>>>> return ret;
>>>> }
>>>>
>>>> - flags_elem[0].in_sg[0].iov_len = hdrlen + optlen;
>>>> + iov_truncate(&flags_iov[0], 1, hdrlen + optlen);
>>>> payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
>>>>
>>>> if (flags & KEEPALIVE)
>>>> @@ -192,9 +192,9 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
>>>> struct msghdr mh_sock = { 0 };
>>>> uint16_t mss = MSS_GET(conn);
>>>> int s = conn->sock;
>>>> - ssize_t ret, len;
>>>> size_t hdrlen;
>>>> int elem_cnt;
>>>> + ssize_t ret;
>>>> int i;
>>>>
>>>> *iov_cnt = 0;
>>>> @@ -247,15 +247,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct vu_virtq *vq,
>>>> ret -= already_sent;
>>>>
>>>> /* adjust iov number and length of the last iov */
>>>> - len = ret;
>>>> - for (i = 0; len && i < elem_cnt; i++) {
>>>> - struct iovec *iov = &elem[i].in_sg[0];
>>>> -
>>>> - if (iov->iov_len > (size_t)len)
>>>> - iov->iov_len = len;
>>>> -
>>>> - len -= iov->iov_len;
>>>> - }
>>>> + i = iov_truncate(&iov_vu[DISCARD_IOV_NUM], elem_cnt, ret);
>>>
>>> I had a quick look, but I couldn't figure this out. This causes
>>> Coverity Scan to report:
>>>
>>> /home/sbrivio/passt/tcp_vu.c:457:3:
>>> Type: Overflowed constant (INTEGER_OVERFLOW)
>>>
>>> /home/sbrivio/passt/tcp_vu.c:355:2:
>>> 1. path: Condition "!vu_queue_enabled(vq)", taking false branch.
>>> /home/sbrivio/passt/tcp_vu.c:355:2:
>>> 2. path: Condition "!vu_queue_started(vq)", taking false branch.
>>> /home/sbrivio/passt/tcp_vu.c:362:2:
>>> 3. path: Condition "0U /* (uint32_t)0 */ - (uint32_t)already_sent - 1 < (16777216U /* 1 << 16 + 8 */)", taking false branch.
>>> /home/sbrivio/passt/tcp_vu.c:374:2:
>>> 4. path: Condition "!wnd_scaled", taking false branch.
>>> /home/sbrivio/passt/tcp_vu.c:374:2:
>>> 5. path: Condition "already_sent >= wnd_scaled", taking false branch.
>>> /home/sbrivio/passt/tcp_vu.c:388:2:
>>> 6. path: Condition "v6", taking true branch.
>>> /home/sbrivio/passt/tcp_vu.c:390:2:
>>> 7. path: Condition "len < 0", taking false branch.
>>> /home/sbrivio/passt/tcp_vu.c:402:2:
>>> 8. path: Condition "!len", taking false branch.
>>> /home/sbrivio/passt/tcp_vu.c:425:2:
>>> 9. path: Condition "log_trace", taking true branch.
>>> /home/sbrivio/passt/tcp_vu.c:426:2:
>>> 10. path: Condition "log_trace", taking true branch.
>>> /home/sbrivio/passt/tcp_vu.c:439:2:
>>> 11. path: Condition "v6", taking true branch.
>>> /home/sbrivio/passt/tcp_vu.c:439:2:
>>> 12. function_return: Function "tcp_vu_hdrlen(v6)" returns 86.
>>> /home/sbrivio/passt/tcp_vu.c:439:2:
>>> 13. known_value_assign: "hdrlen" = "tcp_vu_hdrlen(v6)", its value is now 86.
>>> /home/sbrivio/passt/tcp_vu.c:440:2:
>>> 14. path: Condition "i < head_cnt", taking true branch.
>>> /home/sbrivio/passt/tcp_vu.c:443:3:
>>> 15. function_return: Function "iov_size(iov, buf_cnt)" returns 0.
>>> /home/sbrivio/passt/tcp_vu.c:443:3:
>>> 16. known_value_assign: "dlen" = "iov_size(iov, buf_cnt) - hdrlen", its value is now 18446744073709551530.
>>> /home/sbrivio/passt/tcp_vu.c:450:3:
>>> 17. path: Condition "previous_dlen != dlen", taking true branch.
>>> /home/sbrivio/passt/tcp_vu.c:454:3:
>>> 18. path: Condition "!*c->pcap", taking false branch.
>>> /home/sbrivio/passt/tcp_vu.c:457:3:
>>> 19. overflow_const: Expression "dlen + hdrlen", where "dlen" is known to be equal to -86, and "hdrlen" is known to be equal to 86, underflows the type of "dlen + hdrlen", which is type "unsigned long".
>>>
>>
>> if iov_size(iov, buf_cnt) = 0 and hdrlen = 86 (all unsigned)
>> "dlen" = "iov_size(iov, buf_cnt) - hdrlen", its value is now 18446744073709551530 (see
>> 16.) (i.e. -hdrlen, unsigned -86)
>> so "dlen + hdrlen" overflows (I guess).
>
> I was also thinking it was something like that but:
>
>>
>> Try:
>> diff --git a/tcp_vu.c b/tcp_vu.c
>> index 8ca4170f13f6..787ee004a66a 100644
>> --- a/tcp_vu.c
>> +++ b/tcp_vu.c
>> @@ -440,7 +440,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn *conn)
>> for (i = 0, previous_dlen = -1, check = NULL; i < head_cnt; i++) {
>> struct iovec *iov = &elem[head[i]].in_sg[0];
>> int buf_cnt = head[i + 1] - head[i];
>> - ssize_t dlen = iov_size(iov, buf_cnt) - hdrlen;
>> + ssize_t dlen = (ssize_t)iov_size(iov, buf_cnt) - hdrlen;
>> bool push = i == head_cnt - 1;
>> size_t l2len;
>
> ...still not happy because of how we use dlen later (I think?):
>
> /home/sbrivio/passt/tcp_vu.c:457:3:
> Type: Overflowed constant (INTEGER_OVERFLOW)
>
> /home/sbrivio/passt/tcp_vu.c:355:2:
> 1. path: Condition "!vu_queue_enabled(vq)", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:355:2:
> 2. path: Condition "!vu_queue_started(vq)", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:362:2:
> 3. path: Condition "0U /* (uint32_t)0 */ - (uint32_t)already_sent - 1 < (16777216U /* 1 << 16 + 8 */)", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:374:2:
> 4. path: Condition "!wnd_scaled", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:374:2:
> 5. path: Condition "already_sent >= wnd_scaled", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:388:2:
> 6. path: Condition "v6", taking true branch.
> /home/sbrivio/passt/tcp_vu.c:390:2:
> 7. path: Condition "len < 0", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:402:2:
> 8. path: Condition "!len", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:425:2:
> 9. path: Condition "log_trace", taking true branch.
> /home/sbrivio/passt/tcp_vu.c:426:2:
> 10. path: Condition "log_trace", taking true branch.
> /home/sbrivio/passt/tcp_vu.c:439:2:
> 11. path: Condition "v6", taking true branch.
> /home/sbrivio/passt/tcp_vu.c:440:2:
> 12. path: Condition "i < head_cnt", taking true branch.
> /home/sbrivio/passt/tcp_vu.c:443:3:
> 13. known_value_assign: "dlen" = "(ssize_t)iov_size(iov, buf_cnt) - hdrlen", its value is now 18446744073709551530.
> /home/sbrivio/passt/tcp_vu.c:450:3:
> 14. path: Condition "previous_dlen != dlen", taking true branch.
> /home/sbrivio/passt/tcp_vu.c:454:3:
> 15. path: Condition "!*c->pcap", taking false branch.
> /home/sbrivio/passt/tcp_vu.c:457:3:
> 16. overflow_const: Expression "dlen + hdrlen", where "dlen" is known to be equal to -86, and "hdrlen" is known to be equal to 86, underflows the type of "dlen + hdrlen", which is type "unsigned long".
>
> /home/sbrivio/passt/conf.c:2373:4:
> Type: Untrusted value as argument (TAINTED_SCALAR)
>
Could you try:
diff --git a/tcp_vu.c b/tcp_vu.c
index 8ca4170f13f6..fd734e857b3b 100644
--- a/tcp_vu.c
+++ b/tcp_vu.c
@@ -440,10 +440,14 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn
*conn)
for (i = 0, previous_dlen = -1, check = NULL; i < head_cnt; i++) {
struct iovec *iov = &elem[head[i]].in_sg[0];
int buf_cnt = head[i + 1] - head[i];
- ssize_t dlen = iov_size(iov, buf_cnt) - hdrlen;
+ size_t frame_size = iov_size(iov, buf_cnt);
bool push = i == head_cnt - 1;
+ ssize_t dlen;
size_t l2len;
+ ASSERT(frame_size >= hdrlen);
+
+ dlen = frame_size - hdrlen;
vu_set_vnethdr(iov->iov_base, buf_cnt);
/* The IPv4 header checksum varies only with dlen */
Coverity likes "ASSERT()"...
Thanks,
Laurent
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers
2026-03-06 8:51 ` Laurent Vivier
@ 2026-03-06 10:41 ` Laurent Vivier
2026-03-06 10:52 ` Stefano Brivio
0 siblings, 1 reply; 10+ messages in thread
From: Laurent Vivier @ 2026-03-06 10:41 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev, David Gibson
On 3/6/26 09:51, Laurent Vivier wrote:
> On 3/6/26 09:25, Stefano Brivio wrote:
>> On Fri, 6 Mar 2026 09:17:32 +0100
>> Laurent Vivier <lvivier@redhat.com> wrote:
>>
>>> On 3/6/26 08:35, Stefano Brivio wrote:
>>>> On Thu, 5 Mar 2026 13:56:48 +0100
>>>> Laurent Vivier <lvivier@redhat.com> wrote:
>>>>> Add a generic iov_truncate() function that truncates an IO vector to a
>>>>> given number of bytes, returning the number of iov entries that contain
>>>>> data after truncation.
>>>>>
>>>>> Use it in udp_vu_sock_recv() and tcp_vu_sock_recv() to replace the
>>>>> open-coded truncation logic that adjusted iov entries after recvmsg().
>>>>> Also convert the direct iov_len assignment in tcp_vu_send_flag() to use
>>>>> iov_truncate() for consistency.
>>>>>
>>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>>>> ---
>>>>>
>>>>> Notes:
>>>>> v3: use in tcp_vu_send_flag() too
>>>>> v2: use iov_truncate() in udp_vu_sock_recv() too
>>>>>
>>>>> iov.c | 22 ++++++++++++++++++++++
>>>>> iov.h | 1 +
>>>>> tcp_vu.c | 14 +++-----------
>>>>> udp_vu.c | 12 +++---------
>>>>> 4 files changed, 29 insertions(+), 20 deletions(-)
>>>>>
>>>>> diff --git a/iov.c b/iov.c
>>>>> index ad726daa4cd8..31a3f5bc29e5 100644
>>>>> --- a/iov.c
>>>>> +++ b/iov.c
>>>>> @@ -147,6 +147,28 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
>>>>> return len;
>>>>> }
>>>>> +/**
>>>>> + * iov_truncate() - Truncate an IO vector to a given number of bytes
>>>>> + * @iov: IO vector (modified)
>>>>> + * @iov_cnt: Number of entries in @iov
>>>>> + * @size: Total number of bytes to keep
>>>>> + *
>>>>> + * Return: number of iov entries that contain data after truncation
>>>>> + */
>>>>> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size)
>>>>> +{
>>>>> + size_t i, offset;
>>>>> +
>>>>> + i = iov_skip_bytes(iov, iov_cnt, size, &offset);
>>>>> +
>>>>> + if (i < iov_cnt) {
>>>>> + iov[i].iov_len = offset;
>>>>> + i += !!offset;
>>>>> + }
>>>>> +
>>>>> + return i;
>>>>> +}
>>>>> +
>>>>> /**
>>>>> * iov_tail_prune() - Remove any unneeded buffers from an IOV tail
>>>>> * @tail: IO vector tail (modified)
>>>>> diff --git a/iov.h b/iov.h
>>>>> index d1ab91a94e22..b4e50b0fca5a 100644
>>>>> --- a/iov.h
>>>>> +++ b/iov.h
>>>>> @@ -29,6 +29,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
>>>>> size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
>>>>> size_t offset, void *buf, size_t bytes);
>>>>> size_t iov_size(const struct iovec *iov, size_t iov_cnt);
>>>>> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size);
>>>>> /*
>>>>> * DOC: Theory of Operation, struct iov_tail
>>>>> diff --git a/tcp_vu.c b/tcp_vu.c
>>>>> index 88be232dca66..8ca4170f13f6 100644
>>>>> --- a/tcp_vu.c
>>>>> +++ b/tcp_vu.c
>>>>> @@ -131,7 +131,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn
>>>>> *conn, int flags)
>>>>> return ret;
>>>>> }
>>>>> - flags_elem[0].in_sg[0].iov_len = hdrlen + optlen;
>>>>> + iov_truncate(&flags_iov[0], 1, hdrlen + optlen);
>>>>> payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
>>>>> if (flags & KEEPALIVE)
>>>>> @@ -192,9 +192,9 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct
>>>>> vu_virtq *vq,
>>>>> struct msghdr mh_sock = { 0 };
>>>>> uint16_t mss = MSS_GET(conn);
>>>>> int s = conn->sock;
>>>>> - ssize_t ret, len;
>>>>> size_t hdrlen;
>>>>> int elem_cnt;
>>>>> + ssize_t ret;
>>>>> int i;
>>>>> *iov_cnt = 0;
>>>>> @@ -247,15 +247,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct
>>>>> vu_virtq *vq,
>>>>> ret -= already_sent;
>>>>> /* adjust iov number and length of the last iov */
>>>>> - len = ret;
>>>>> - for (i = 0; len && i < elem_cnt; i++) {
>>>>> - struct iovec *iov = &elem[i].in_sg[0];
>>>>> -
>>>>> - if (iov->iov_len > (size_t)len)
>>>>> - iov->iov_len = len;
>>>>> -
>>>>> - len -= iov->iov_len;
>>>>> - }
>>>>> + i = iov_truncate(&iov_vu[DISCARD_IOV_NUM], elem_cnt, ret);
>>>>
>>>> I had a quick look, but I couldn't figure this out. This causes
>>>> Coverity Scan to report:
>>>>
>>>> /home/sbrivio/passt/tcp_vu.c:457:3:
>>>> Type: Overflowed constant (INTEGER_OVERFLOW)
>>>>
>>>> /home/sbrivio/passt/tcp_vu.c:355:2:
>>>> 1. path: Condition "!vu_queue_enabled(vq)", taking false branch.
>>>> /home/sbrivio/passt/tcp_vu.c:355:2:
>>>> 2. path: Condition "!vu_queue_started(vq)", taking false branch.
>>>> /home/sbrivio/passt/tcp_vu.c:362:2:
>>>> 3. path: Condition "0U /* (uint32_t)0 */ - (uint32_t)already_sent - 1 <
>>>> (16777216U /* 1 << 16 + 8 */)", taking false branch.
>>>> /home/sbrivio/passt/tcp_vu.c:374:2:
>>>> 4. path: Condition "!wnd_scaled", taking false branch.
>>>> /home/sbrivio/passt/tcp_vu.c:374:2:
>>>> 5. path: Condition "already_sent >= wnd_scaled", taking false branch.
>>>> /home/sbrivio/passt/tcp_vu.c:388:2:
>>>> 6. path: Condition "v6", taking true branch.
>>>> /home/sbrivio/passt/tcp_vu.c:390:2:
>>>> 7. path: Condition "len < 0", taking false branch.
>>>> /home/sbrivio/passt/tcp_vu.c:402:2:
>>>> 8. path: Condition "!len", taking false branch.
>>>> /home/sbrivio/passt/tcp_vu.c:425:2:
>>>> 9. path: Condition "log_trace", taking true branch.
>>>> /home/sbrivio/passt/tcp_vu.c:426:2:
>>>> 10. path: Condition "log_trace", taking true branch.
>>>> /home/sbrivio/passt/tcp_vu.c:439:2:
>>>> 11. path: Condition "v6", taking true branch.
>>>> /home/sbrivio/passt/tcp_vu.c:439:2:
>>>> 12. function_return: Function "tcp_vu_hdrlen(v6)" returns 86.
>>>> /home/sbrivio/passt/tcp_vu.c:439:2:
>>>> 13. known_value_assign: "hdrlen" = "tcp_vu_hdrlen(v6)", its value is now 86.
>>>> /home/sbrivio/passt/tcp_vu.c:440:2:
>>>> 14. path: Condition "i < head_cnt", taking true branch.
>>>> /home/sbrivio/passt/tcp_vu.c:443:3:
>>>> 15. function_return: Function "iov_size(iov, buf_cnt)" returns 0.
>>>> /home/sbrivio/passt/tcp_vu.c:443:3:
>>>> 16. known_value_assign: "dlen" = "iov_size(iov, buf_cnt) - hdrlen", its value is
>>>> now 18446744073709551530.
>>>> /home/sbrivio/passt/tcp_vu.c:450:3:
>>>> 17. path: Condition "previous_dlen != dlen", taking true branch.
>>>> /home/sbrivio/passt/tcp_vu.c:454:3:
>>>> 18. path: Condition "!*c->pcap", taking false branch.
>>>> /home/sbrivio/passt/tcp_vu.c:457:3:
>>>> 19. overflow_const: Expression "dlen + hdrlen", where "dlen" is known to be equal
>>>> to -86, and "hdrlen" is known to be equal to 86, underflows the type of "dlen +
>>>> hdrlen", which is type "unsigned long".
>>>
>>> if iov_size(iov, buf_cnt) = 0 and hdrlen = 86 (all unsigned)
>>> "dlen" = "iov_size(iov, buf_cnt) - hdrlen", its value is now 18446744073709551530 (see
>>> 16.) (i.e. -hdrlen, unsigned -86)
>>> so "dlen + hdrlen" overflows (I guess).
>>
>> I was also thinking it was something like that but:
>>
>>>
>>> Try:
>>> diff --git a/tcp_vu.c b/tcp_vu.c
>>> index 8ca4170f13f6..787ee004a66a 100644
>>> --- a/tcp_vu.c
>>> +++ b/tcp_vu.c
>>> @@ -440,7 +440,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn
>>> *conn)
>>> for (i = 0, previous_dlen = -1, check = NULL; i < head_cnt; i++) {
>>> struct iovec *iov = &elem[head[i]].in_sg[0];
>>> int buf_cnt = head[i + 1] - head[i];
>>> - ssize_t dlen = iov_size(iov, buf_cnt) - hdrlen;
>>> + ssize_t dlen = (ssize_t)iov_size(iov, buf_cnt) - hdrlen;
>>> bool push = i == head_cnt - 1;
>>> size_t l2len;
>>
>> ...still not happy because of how we use dlen later (I think?):
>>
>> /home/sbrivio/passt/tcp_vu.c:457:3:
>> Type: Overflowed constant (INTEGER_OVERFLOW)
>>
>> /home/sbrivio/passt/tcp_vu.c:355:2:
>> 1. path: Condition "!vu_queue_enabled(vq)", taking false branch.
>> /home/sbrivio/passt/tcp_vu.c:355:2:
>> 2. path: Condition "!vu_queue_started(vq)", taking false branch.
>> /home/sbrivio/passt/tcp_vu.c:362:2:
>> 3. path: Condition "0U /* (uint32_t)0 */ - (uint32_t)already_sent - 1 < (16777216U /*
>> 1 << 16 + 8 */)", taking false branch.
>> /home/sbrivio/passt/tcp_vu.c:374:2:
>> 4. path: Condition "!wnd_scaled", taking false branch.
>> /home/sbrivio/passt/tcp_vu.c:374:2:
>> 5. path: Condition "already_sent >= wnd_scaled", taking false branch.
>> /home/sbrivio/passt/tcp_vu.c:388:2:
>> 6. path: Condition "v6", taking true branch.
>> /home/sbrivio/passt/tcp_vu.c:390:2:
>> 7. path: Condition "len < 0", taking false branch.
>> /home/sbrivio/passt/tcp_vu.c:402:2:
>> 8. path: Condition "!len", taking false branch.
>> /home/sbrivio/passt/tcp_vu.c:425:2:
>> 9. path: Condition "log_trace", taking true branch.
>> /home/sbrivio/passt/tcp_vu.c:426:2:
>> 10. path: Condition "log_trace", taking true branch.
>> /home/sbrivio/passt/tcp_vu.c:439:2:
>> 11. path: Condition "v6", taking true branch.
>> /home/sbrivio/passt/tcp_vu.c:440:2:
>> 12. path: Condition "i < head_cnt", taking true branch.
>> /home/sbrivio/passt/tcp_vu.c:443:3:
>> 13. known_value_assign: "dlen" = "(ssize_t)iov_size(iov, buf_cnt) - hdrlen", its
>> value is now 18446744073709551530.
>> /home/sbrivio/passt/tcp_vu.c:450:3:
>> 14. path: Condition "previous_dlen != dlen", taking true branch.
>> /home/sbrivio/passt/tcp_vu.c:454:3:
>> 15. path: Condition "!*c->pcap", taking false branch.
>> /home/sbrivio/passt/tcp_vu.c:457:3:
>> 16. overflow_const: Expression "dlen + hdrlen", where "dlen" is known to be equal to
>> -86, and "hdrlen" is known to be equal to 86, underflows the type of "dlen + hdrlen",
>> which is type "unsigned long".
>>
>> /home/sbrivio/passt/conf.c:2373:4:
>> Type: Untrusted value as argument (TAINTED_SCALAR)
>>
>
> Could you try:
> diff --git a/tcp_vu.c b/tcp_vu.c
> index 8ca4170f13f6..fd734e857b3b 100644
> --- a/tcp_vu.c
> +++ b/tcp_vu.c
> @@ -440,10 +440,14 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn
> *conn)
> for (i = 0, previous_dlen = -1, check = NULL; i < head_cnt; i++) {
> struct iovec *iov = &elem[head[i]].in_sg[0];
> int buf_cnt = head[i + 1] - head[i];
> - ssize_t dlen = iov_size(iov, buf_cnt) - hdrlen;
> + size_t frame_size = iov_size(iov, buf_cnt);
> bool push = i == head_cnt - 1;
> + ssize_t dlen;
> size_t l2len;
>
> + ASSERT(frame_size >= hdrlen);
> +
> + dlen = frame_size - hdrlen;
> vu_set_vnethdr(iov->iov_base, buf_cnt);
>
> /* The IPv4 header checksum varies only with dlen */
>
> Coverity likes "ASSERT()"...
This seems to fix the problem for me.
Do you want a separate patch or to merge with this one?
Thanks,
Laurent
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers
2026-03-06 10:41 ` Laurent Vivier
@ 2026-03-06 10:52 ` Stefano Brivio
2026-03-06 10:56 ` Laurent Vivier
0 siblings, 1 reply; 10+ messages in thread
From: Stefano Brivio @ 2026-03-06 10:52 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev, David Gibson
On Fri, 6 Mar 2026 11:41:17 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> On 3/6/26 09:51, Laurent Vivier wrote:
> > On 3/6/26 09:25, Stefano Brivio wrote:
> >> On Fri, 6 Mar 2026 09:17:32 +0100
> >> Laurent Vivier <lvivier@redhat.com> wrote:
> >>
> >>> On 3/6/26 08:35, Stefano Brivio wrote:
> >>>> On Thu, 5 Mar 2026 13:56:48 +0100
> >>>> Laurent Vivier <lvivier@redhat.com> wrote:
> >>>>> Add a generic iov_truncate() function that truncates an IO vector to a
> >>>>> given number of bytes, returning the number of iov entries that contain
> >>>>> data after truncation.
> >>>>>
> >>>>> Use it in udp_vu_sock_recv() and tcp_vu_sock_recv() to replace the
> >>>>> open-coded truncation logic that adjusted iov entries after recvmsg().
> >>>>> Also convert the direct iov_len assignment in tcp_vu_send_flag() to use
> >>>>> iov_truncate() for consistency.
> >>>>>
> >>>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> >>>>> ---
> >>>>>
> >>>>> Notes:
> >>>>> v3: use in tcp_vu_send_flag() too
> >>>>> v2: use iov_truncate() in udp_vu_sock_recv() too
> >>>>>
> >>>>> iov.c | 22 ++++++++++++++++++++++
> >>>>> iov.h | 1 +
> >>>>> tcp_vu.c | 14 +++-----------
> >>>>> udp_vu.c | 12 +++---------
> >>>>> 4 files changed, 29 insertions(+), 20 deletions(-)
> >>>>>
> >>>>> diff --git a/iov.c b/iov.c
> >>>>> index ad726daa4cd8..31a3f5bc29e5 100644
> >>>>> --- a/iov.c
> >>>>> +++ b/iov.c
> >>>>> @@ -147,6 +147,28 @@ size_t iov_size(const struct iovec *iov, size_t iov_cnt)
> >>>>> return len;
> >>>>> }
> >>>>> +/**
> >>>>> + * iov_truncate() - Truncate an IO vector to a given number of bytes
> >>>>> + * @iov: IO vector (modified)
> >>>>> + * @iov_cnt: Number of entries in @iov
> >>>>> + * @size: Total number of bytes to keep
> >>>>> + *
> >>>>> + * Return: number of iov entries that contain data after truncation
> >>>>> + */
> >>>>> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size)
> >>>>> +{
> >>>>> + size_t i, offset;
> >>>>> +
> >>>>> + i = iov_skip_bytes(iov, iov_cnt, size, &offset);
> >>>>> +
> >>>>> + if (i < iov_cnt) {
> >>>>> + iov[i].iov_len = offset;
> >>>>> + i += !!offset;
> >>>>> + }
> >>>>> +
> >>>>> + return i;
> >>>>> +}
> >>>>> +
> >>>>> /**
> >>>>> * iov_tail_prune() - Remove any unneeded buffers from an IOV tail
> >>>>> * @tail: IO vector tail (modified)
> >>>>> diff --git a/iov.h b/iov.h
> >>>>> index d1ab91a94e22..b4e50b0fca5a 100644
> >>>>> --- a/iov.h
> >>>>> +++ b/iov.h
> >>>>> @@ -29,6 +29,7 @@ size_t iov_from_buf(const struct iovec *iov, size_t iov_cnt,
> >>>>> size_t iov_to_buf(const struct iovec *iov, size_t iov_cnt,
> >>>>> size_t offset, void *buf, size_t bytes);
> >>>>> size_t iov_size(const struct iovec *iov, size_t iov_cnt);
> >>>>> +size_t iov_truncate(struct iovec *iov, size_t iov_cnt, size_t size);
> >>>>> /*
> >>>>> * DOC: Theory of Operation, struct iov_tail
> >>>>> diff --git a/tcp_vu.c b/tcp_vu.c
> >>>>> index 88be232dca66..8ca4170f13f6 100644
> >>>>> --- a/tcp_vu.c
> >>>>> +++ b/tcp_vu.c
> >>>>> @@ -131,7 +131,7 @@ int tcp_vu_send_flag(const struct ctx *c, struct tcp_tap_conn
> >>>>> *conn, int flags)
> >>>>> return ret;
> >>>>> }
> >>>>> - flags_elem[0].in_sg[0].iov_len = hdrlen + optlen;
> >>>>> + iov_truncate(&flags_iov[0], 1, hdrlen + optlen);
> >>>>> payload = IOV_TAIL(flags_elem[0].in_sg, 1, hdrlen);
> >>>>> if (flags & KEEPALIVE)
> >>>>> @@ -192,9 +192,9 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct
> >>>>> vu_virtq *vq,
> >>>>> struct msghdr mh_sock = { 0 };
> >>>>> uint16_t mss = MSS_GET(conn);
> >>>>> int s = conn->sock;
> >>>>> - ssize_t ret, len;
> >>>>> size_t hdrlen;
> >>>>> int elem_cnt;
> >>>>> + ssize_t ret;
> >>>>> int i;
> >>>>> *iov_cnt = 0;
> >>>>> @@ -247,15 +247,7 @@ static ssize_t tcp_vu_sock_recv(const struct ctx *c, struct
> >>>>> vu_virtq *vq,
> >>>>> ret -= already_sent;
> >>>>> /* adjust iov number and length of the last iov */
> >>>>> - len = ret;
> >>>>> - for (i = 0; len && i < elem_cnt; i++) {
> >>>>> - struct iovec *iov = &elem[i].in_sg[0];
> >>>>> -
> >>>>> - if (iov->iov_len > (size_t)len)
> >>>>> - iov->iov_len = len;
> >>>>> -
> >>>>> - len -= iov->iov_len;
> >>>>> - }
> >>>>> + i = iov_truncate(&iov_vu[DISCARD_IOV_NUM], elem_cnt, ret);
> >>>>
> >>>> I had a quick look, but I couldn't figure this out. This causes
> >>>> Coverity Scan to report:
> >>>>
> >>>> /home/sbrivio/passt/tcp_vu.c:457:3:
> >>>> Type: Overflowed constant (INTEGER_OVERFLOW)
> >>>>
> >>>> /home/sbrivio/passt/tcp_vu.c:355:2:
> >>>> 1. path: Condition "!vu_queue_enabled(vq)", taking false branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:355:2:
> >>>> 2. path: Condition "!vu_queue_started(vq)", taking false branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:362:2:
> >>>> 3. path: Condition "0U /* (uint32_t)0 */ - (uint32_t)already_sent - 1 <
> >>>> (16777216U /* 1 << 16 + 8 */)", taking false branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:374:2:
> >>>> 4. path: Condition "!wnd_scaled", taking false branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:374:2:
> >>>> 5. path: Condition "already_sent >= wnd_scaled", taking false branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:388:2:
> >>>> 6. path: Condition "v6", taking true branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:390:2:
> >>>> 7. path: Condition "len < 0", taking false branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:402:2:
> >>>> 8. path: Condition "!len", taking false branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:425:2:
> >>>> 9. path: Condition "log_trace", taking true branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:426:2:
> >>>> 10. path: Condition "log_trace", taking true branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:439:2:
> >>>> 11. path: Condition "v6", taking true branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:439:2:
> >>>> 12. function_return: Function "tcp_vu_hdrlen(v6)" returns 86.
> >>>> /home/sbrivio/passt/tcp_vu.c:439:2:
> >>>> 13. known_value_assign: "hdrlen" = "tcp_vu_hdrlen(v6)", its value is now 86.
> >>>> /home/sbrivio/passt/tcp_vu.c:440:2:
> >>>> 14. path: Condition "i < head_cnt", taking true branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:443:3:
> >>>> 15. function_return: Function "iov_size(iov, buf_cnt)" returns 0.
> >>>> /home/sbrivio/passt/tcp_vu.c:443:3:
> >>>> 16. known_value_assign: "dlen" = "iov_size(iov, buf_cnt) - hdrlen", its value is
> >>>> now 18446744073709551530.
> >>>> /home/sbrivio/passt/tcp_vu.c:450:3:
> >>>> 17. path: Condition "previous_dlen != dlen", taking true branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:454:3:
> >>>> 18. path: Condition "!*c->pcap", taking false branch.
> >>>> /home/sbrivio/passt/tcp_vu.c:457:3:
> >>>> 19. overflow_const: Expression "dlen + hdrlen", where "dlen" is known to be equal
> >>>> to -86, and "hdrlen" is known to be equal to 86, underflows the type of "dlen +
> >>>> hdrlen", which is type "unsigned long".
> >>>
> >>> if iov_size(iov, buf_cnt) = 0 and hdrlen = 86 (all unsigned)
> >>> "dlen" = "iov_size(iov, buf_cnt) - hdrlen", its value is now 18446744073709551530 (see
> >>> 16.) (i.e. -hdrlen, unsigned -86)
> >>> so "dlen + hdrlen" overflows (I guess).
> >>
> >> I was also thinking it was something like that but:
> >>
> >>>
> >>> Try:
> >>> diff --git a/tcp_vu.c b/tcp_vu.c
> >>> index 8ca4170f13f6..787ee004a66a 100644
> >>> --- a/tcp_vu.c
> >>> +++ b/tcp_vu.c
> >>> @@ -440,7 +440,7 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn
> >>> *conn)
> >>> for (i = 0, previous_dlen = -1, check = NULL; i < head_cnt; i++) {
> >>> struct iovec *iov = &elem[head[i]].in_sg[0];
> >>> int buf_cnt = head[i + 1] - head[i];
> >>> - ssize_t dlen = iov_size(iov, buf_cnt) - hdrlen;
> >>> + ssize_t dlen = (ssize_t)iov_size(iov, buf_cnt) - hdrlen;
> >>> bool push = i == head_cnt - 1;
> >>> size_t l2len;
> >>
> >> ...still not happy because of how we use dlen later (I think?):
> >>
> >> /home/sbrivio/passt/tcp_vu.c:457:3:
> >> Type: Overflowed constant (INTEGER_OVERFLOW)
> >>
> >> /home/sbrivio/passt/tcp_vu.c:355:2:
> >> 1. path: Condition "!vu_queue_enabled(vq)", taking false branch.
> >> /home/sbrivio/passt/tcp_vu.c:355:2:
> >> 2. path: Condition "!vu_queue_started(vq)", taking false branch.
> >> /home/sbrivio/passt/tcp_vu.c:362:2:
> >> 3. path: Condition "0U /* (uint32_t)0 */ - (uint32_t)already_sent - 1 < (16777216U /*
> >> 1 << 16 + 8 */)", taking false branch.
> >> /home/sbrivio/passt/tcp_vu.c:374:2:
> >> 4. path: Condition "!wnd_scaled", taking false branch.
> >> /home/sbrivio/passt/tcp_vu.c:374:2:
> >> 5. path: Condition "already_sent >= wnd_scaled", taking false branch.
> >> /home/sbrivio/passt/tcp_vu.c:388:2:
> >> 6. path: Condition "v6", taking true branch.
> >> /home/sbrivio/passt/tcp_vu.c:390:2:
> >> 7. path: Condition "len < 0", taking false branch.
> >> /home/sbrivio/passt/tcp_vu.c:402:2:
> >> 8. path: Condition "!len", taking false branch.
> >> /home/sbrivio/passt/tcp_vu.c:425:2:
> >> 9. path: Condition "log_trace", taking true branch.
> >> /home/sbrivio/passt/tcp_vu.c:426:2:
> >> 10. path: Condition "log_trace", taking true branch.
> >> /home/sbrivio/passt/tcp_vu.c:439:2:
> >> 11. path: Condition "v6", taking true branch.
> >> /home/sbrivio/passt/tcp_vu.c:440:2:
> >> 12. path: Condition "i < head_cnt", taking true branch.
> >> /home/sbrivio/passt/tcp_vu.c:443:3:
> >> 13. known_value_assign: "dlen" = "(ssize_t)iov_size(iov, buf_cnt) - hdrlen", its
> >> value is now 18446744073709551530.
> >> /home/sbrivio/passt/tcp_vu.c:450:3:
> >> 14. path: Condition "previous_dlen != dlen", taking true branch.
> >> /home/sbrivio/passt/tcp_vu.c:454:3:
> >> 15. path: Condition "!*c->pcap", taking false branch.
> >> /home/sbrivio/passt/tcp_vu.c:457:3:
> >> 16. overflow_const: Expression "dlen + hdrlen", where "dlen" is known to be equal to
> >> -86, and "hdrlen" is known to be equal to 86, underflows the type of "dlen + hdrlen",
> >> which is type "unsigned long".
> >>
> >> /home/sbrivio/passt/conf.c:2373:4:
> >> Type: Untrusted value as argument (TAINTED_SCALAR)
> >>
> >
> > Could you try:
> > diff --git a/tcp_vu.c b/tcp_vu.c
> > index 8ca4170f13f6..fd734e857b3b 100644
> > --- a/tcp_vu.c
> > +++ b/tcp_vu.c
> > @@ -440,10 +440,14 @@ int tcp_vu_data_from_sock(const struct ctx *c, struct tcp_tap_conn
> > *conn)
> > for (i = 0, previous_dlen = -1, check = NULL; i < head_cnt; i++) {
> > struct iovec *iov = &elem[head[i]].in_sg[0];
> > int buf_cnt = head[i + 1] - head[i];
> > - ssize_t dlen = iov_size(iov, buf_cnt) - hdrlen;
> > + size_t frame_size = iov_size(iov, buf_cnt);
> > bool push = i == head_cnt - 1;
> > + ssize_t dlen;
> > size_t l2len;
> >
> > + ASSERT(frame_size >= hdrlen);
> > +
> > + dlen = frame_size - hdrlen;
> > vu_set_vnethdr(iov->iov_base, buf_cnt);
> >
> > /* The IPv4 header checksum varies only with dlen */
> >
> > Coverity likes "ASSERT()"...
>
> This seems to fix the problem for me.
>
> Do you want a separate patch or to merge with this one?
A separate patch would be nice, so that I don't have to convert spaces
back to tabs myself, and I guess it might deserve a new review anyway...
--
Stefano
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers
2026-03-06 10:52 ` Stefano Brivio
@ 2026-03-06 10:56 ` Laurent Vivier
2026-03-06 11:01 ` Stefano Brivio
0 siblings, 1 reply; 10+ messages in thread
From: Laurent Vivier @ 2026-03-06 10:56 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev, David Gibson
On 3/6/26 11:52, Stefano Brivio wrote:
>> This seems to fix the problem for me.
>>
>> Do you want a separate patch or to merge with this one?
> A separate patch would be nice, so that I don't have to convert spaces
> back to tabs myself, and I guess it might deserve a new review anyway...
I'm not sure to understand...
My question was in fact: do I merge the fix inside this patch or do I send two separate
patches in a series (this one, keep v3, and another patch to fix the issue reported by
coverity).
Thanks,
Laurent
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers
2026-03-06 10:56 ` Laurent Vivier
@ 2026-03-06 11:01 ` Stefano Brivio
0 siblings, 0 replies; 10+ messages in thread
From: Stefano Brivio @ 2026-03-06 11:01 UTC (permalink / raw)
To: Laurent Vivier; +Cc: passt-dev, David Gibson
On Fri, 6 Mar 2026 11:56:30 +0100
Laurent Vivier <lvivier@redhat.com> wrote:
> On 3/6/26 11:52, Stefano Brivio wrote:
> >> This seems to fix the problem for me.
> >>
> >> Do you want a separate patch or to merge with this one?
> > A separate patch would be nice, so that I don't have to convert spaces
> > back to tabs myself, and I guess it might deserve a new review anyway...
>
> I'm not sure to understand...
>
> My question was in fact: do I merge the fix inside this patch or do I send two separate
> patches in a series (this one, keep v3, and another patch to fix the issue reported by
> coverity).
Oh gosh sorry, by separate patch I meant (not really obvious!) a single
v4 with everything, that can be reviewed stand-alone.
Even if the issue was not reported earlier but pre-existing (I'm not
sure), I think fixing it in the same patch that triggers the report
would be entirely reasonable.
I meant I wouldn't merge your subsequent changes while applying v3
myself.
--
Stefano
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-03-06 11:01 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-03-05 12:56 [PATCH v3] iov: Add iov_truncate() helper and use it in vu handlers Laurent Vivier
2026-03-06 0:05 ` David Gibson
2026-03-06 7:35 ` Stefano Brivio
2026-03-06 8:17 ` Laurent Vivier
2026-03-06 8:25 ` Stefano Brivio
2026-03-06 8:51 ` Laurent Vivier
2026-03-06 10:41 ` Laurent Vivier
2026-03-06 10:52 ` Stefano Brivio
2026-03-06 10:56 ` Laurent Vivier
2026-03-06 11:01 ` Stefano Brivio
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).