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