public inbox for passt-dev@passt.top
 help / color / mirror / code / Atom feed
* [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).