[PATCH v2 0/6] Fix issues and false positives from static analysis

[PATCH v2 0/6] Fix issues and false positives from static analysis

[Jon Maloy]

This series fixes minor issues found by static analysis tools in the
main passt code and in documentation examples.

The core code changes add missing fd initialisations and guard unsigned
arithmetic against potential underflow.
                                       
We also fix buffer handling, type mismatches, and resource leaks in
the documentation example code.

v2: - udp_vu, tap: Use if-guards instead of assert(), to handle the
      error paths properly and let the static checker follow the logic
    - doc/migration: Use snprintf() instead of strncpy() for clarity
      and guaranteed null-termination (David Gibson)
    - Drop va_list initialisation patch (Stefano Brivio, David Gibson)   

Jon Maloy (6):
  passt: Initialise listening socket fds to -1
  udp_vu: Check iov_tail_clone() return before assigning to msg_iovlen
  tap: Guard IPv6 tail size subtraction against underflow
  doc/migration: Use snprintf() for socket path and fix argv access
  doc/migration: Fix buffer type mismatch in recv() call
  doc/platform-requirements: Close leaked sockets in test_close_dup()

 doc/migration/source.c                    |  2 +-
 doc/migration/target.c                    | 10 ++++------
 doc/platform-requirements/udp-close-dup.c |  3 +++
 passt.c                                   |  3 +++
 tap.c                                     |  5 ++++-
 udp_vu.c                                  |  8 ++++++--
 6 files changed, 21 insertions(+), 10 deletions(-)

-- 
2.52.0

[PATCH v2 1/6] passt: Initialise listening socket fds to -1

[Jon Maloy]

fd_tap_listen, fd_control_listen, and fd_repair_listen are file
descriptors and should be initialised to -1 rather than the implicit
0, consistent with the other fd fields in passt_ctx.

Signed-off-by: Jon Maloy <jmaloy@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

---
v2: No changes.
---
 passt.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/passt.c b/passt.c
index 65a07d72..4e5b9289 100644
--- a/passt.c
+++ b/passt.c
@@ -65,6 +65,9 @@ char pkt_buf[PKT_BUF_BYTES]	__attribute__ ((aligned(PAGE_SIZE)));
 struct ctx passt_ctx = {
 	.pidfile_fd		= -1,
 	.fd_tap			= -1,
+	.fd_tap_listen		= -1,
+	.fd_control_listen	= -1,
+	.fd_repair_listen	= -1,
 	.pasta_netns_fd		= -1,
 	.device_state_fd	= -1,
 };
-- 
2.52.0

[PATCH v2 2/6] udp_vu: Check iov_tail_clone() return before assigning to msg_iovlen

[Jon Maloy]

iov_tail_clone() returns ssize_t and can return -1 if the destination
iov array is too small. Its return value was assigned directly to
msg.msg_iovlen which is size_t, wrapping a negative value to a large
unsigned number passed to recvmsg().

Check for failure and return early, letting the caller rewind the
virtqueue.

Signed-off-by: Jon Maloy <jmaloy@redhat.com>

---
v2: Use if-guard instead of assert(), since the error path is
    a legitimate (if unlikely) condition handled by the caller.
---
 udp_vu.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/udp_vu.c b/udp_vu.c
index e4fb1057..edd9cae7 100644
--- a/udp_vu.c
+++ b/udp_vu.c
@@ -67,11 +67,15 @@ static ssize_t udp_vu_sock_recv(struct iov_tail *payload, size_t *cnt, int s)
 	struct iovec msg_iov[VIRTQUEUE_MAX_SIZE];
 	struct msghdr msg  = { 0 };
 	size_t iov_used;
+	ssize_t iovlen;
 	ssize_t dlen;
 
+	iovlen = iov_tail_clone(msg_iov, ARRAY_SIZE(msg_iov), payload);
+	if (iovlen < 0)
+		return -1;
+
 	msg.msg_iov = msg_iov;
-	msg.msg_iovlen = iov_tail_clone(msg.msg_iov, ARRAY_SIZE(msg_iov),
-					payload);
+	msg.msg_iovlen = iovlen;
 
 	/* read data from the socket */
 	dlen = recvmsg(s, &msg, 0);
-- 
2.52.0

[PATCH v2 3/6] tap: Guard IPv6 tail size subtraction against underflow

[Jon Maloy]

In tap6_handler(), iov_tail_size(&data) - sizeof(*ip6h) computes
the expected payload length. IOV_PEEK_HEADER() guarantees at least
sizeof(*ip6h) bytes, but add an explicit check to guard the unsigned
subtraction. A too-small tail would indicate a malformed packet, so
skip it.

Signed-off-by: Jon Maloy <jmaloy@redhat.com>

---
v2: Use if-guard instead of assert(), to avoid runtime cost in
    per-packet path and to let the static checker follow the logic.
---
 tap.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tap.c b/tap.c
index 6d93c7ce..6fd5f595 100644
--- a/tap.c
+++ b/tap.c
@@ -986,7 +986,10 @@ resume:
 		if (!ip6h)
 			continue;
 
-		check = iov_tail_size(&data) - sizeof(*ip6h);
+		check = iov_tail_size(&data);
+		if (check < sizeof(*ip6h))
+			continue;
+		check -= sizeof(*ip6h);
 
 		saddr = &ip6h->saddr;
 		daddr = &ip6h->daddr;
-- 
2.52.0

[PATCH v2 4/6] doc/migration: Use snprintf() for socket path and fix argv access

[Jon Maloy]

Replace strcpy() with snprintf() when copying the helper socket path
into sun_path, preventing a potential buffer overflow from a long
argument and guaranteeing null-termination.

In target.c, move the argc check before accessing argv[4], so we
don't dereference past the end of argv.

Signed-off-by: Jon Maloy <jmaloy@redhat.com>

---
v2: Use snprintf() instead of strncpy() for clarity and
    guaranteed null-termination, as suggested by David Gibson
---
 doc/migration/source.c | 2 +-
 doc/migration/target.c | 8 +++-----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/doc/migration/source.c b/doc/migration/source.c
index d44ebf1f..8aad0259 100644
--- a/doc/migration/source.c
+++ b/doc/migration/source.c
@@ -47,7 +47,7 @@ int main(int argc, char **argv)
 		return -1;
 	}
 
-	strcpy(a_helper.sun_path, argv[4]);
+	snprintf(a_helper.sun_path, sizeof(a_helper.sun_path), "%s", argv[4]);
 	getaddrinfo(argv[1], argv[2], &hints, &r);
 
 	/* Connect socket to server and send some data */
diff --git a/doc/migration/target.c b/doc/migration/target.c
index f7d31083..e2469837 100644
--- a/doc/migration/target.c
+++ b/doc/migration/target.c
@@ -38,11 +38,6 @@ int main(int argc, char **argv)
 	struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg);
 	struct addrinfo *r;
 
-	(void)argc;
-
-	strcpy(a_helper.sun_path, argv[4]);
-	getaddrinfo(argv[1], argv[2], &hints, &r);
-
 	if (argc != 7) {
 		fprintf(stderr,
 			"%s DST_ADDR DST_PORT SRC_PORT HELPER_PATH SSEQ RSEQ\n",
@@ -50,6 +45,9 @@ int main(int argc, char **argv)
 		return -1;
 	}
 
+	snprintf(a_helper.sun_path, sizeof(a_helper.sun_path), "%s", argv[4]);
+	getaddrinfo(argv[1], argv[2], &hints, &r);
+
 	/* Prepare socket, bind to source port */
 	s = socket(r->ai_family, SOCK_STREAM, IPPROTO_TCP);
 	setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &((int){ 1 }), sizeof(int));
-- 
2.52.0

[PATCH v2 5/6] doc/migration: Fix buffer type mismatch in recv() call

[Jon Maloy]

The compound literal passed to recv() was int (4 bytes) but only
1 byte is received. Use int8_t to match the length, consistent with
the other recv() calls in this file and in source.c.

Signed-off-by: Jon Maloy <jmaloy@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

---
v2: No changes.
---
 doc/migration/target.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/migration/target.c b/doc/migration/target.c
index e2469837..3e664659 100644
--- a/doc/migration/target.c
+++ b/doc/migration/target.c
@@ -69,7 +69,7 @@ int main(int argc, char **argv)
 	/* Send command to helper: turn repair mode on, wait for reply */
 	cmd = TCP_REPAIR_ON;
 	sendmsg(s_helper, &msg, 0);
-	recv(s_helper, &((int){ 0 }), 1, 0);
+	recv(s_helper, &((int8_t){ 0 }), 1, 0);
 
 	/* Set sending sequence */
 	seq = TCP_SEND_QUEUE;
-- 
2.52.0

[PATCH v2 6/6] doc/platform-requirements: Close leaked sockets in test_close_dup()

[Jon Maloy]

s1 and send_s were not closed before returning, leaking file
descriptors across the loop iterations in main().

Signed-off-by: Jon Maloy <jmaloy@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

---
v2: No changes.
---
 doc/platform-requirements/udp-close-dup.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/doc/platform-requirements/udp-close-dup.c b/doc/platform-requirements/udp-close-dup.c
index 99060fcb..abb719a5 100644
--- a/doc/platform-requirements/udp-close-dup.c
+++ b/doc/platform-requirements/udp-close-dup.c
@@ -87,6 +87,9 @@ static void test_close_dup(enum dup_method method)
 	token = random();
 	send_token(send_s, token);
 	recv_token(s1, token);
+
+	close(s1);
+	close(send_s);
 }
 
 int main(int argc, char *argv[])
-- 
2.52.0

Re: [PATCH v2 2/6] udp_vu: Check iov_tail_clone() return before assigning to msg_iovlen

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]

On Thu, Jul 09, 2026 at 05:56:52PM -0400, Jon Maloy wrote:
> iov_tail_clone() returns ssize_t and can return -1 if the destination
> iov array is too small. Its return value was assigned directly to
> msg.msg_iovlen which is size_t, wrapping a negative value to a large
> unsigned number passed to recvmsg().
> 
> Check for failure and return early, letting the caller rewind the
> virtqueue.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> 
> ---
> v2: Use if-guard instead of assert(), since the error path is
>     a legitimate (if unlikely) condition handled by the caller.
> ---
>  udp_vu.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/udp_vu.c b/udp_vu.c
> index e4fb1057..edd9cae7 100644
> --- a/udp_vu.c
> +++ b/udp_vu.c
> @@ -67,11 +67,15 @@ static ssize_t udp_vu_sock_recv(struct iov_tail *payload, size_t *cnt, int s)
>  	struct iovec msg_iov[VIRTQUEUE_MAX_SIZE];
>  	struct msghdr msg  = { 0 };
>  	size_t iov_used;
> +	ssize_t iovlen;
>  	ssize_t dlen;
>  
> +	iovlen = iov_tail_clone(msg_iov, ARRAY_SIZE(msg_iov), payload);
> +	if (iovlen < 0)
> +		return -1;
> +
>  	msg.msg_iov = msg_iov;
> -	msg.msg_iovlen = iov_tail_clone(msg.msg_iov, ARRAY_SIZE(msg_iov),
> -					payload);
> +	msg.msg_iovlen = iovlen;
>  
>  	/* read data from the socket */
>  	dlen = recvmsg(s, &msg, 0);
> -- 
> 2.52.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 --]

Re: [PATCH v2 3/6] tap: Guard IPv6 tail size subtraction against underflow

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1443 bytes --]

On Thu, Jul 09, 2026 at 05:56:53PM -0400, Jon Maloy wrote:
> In tap6_handler(), iov_tail_size(&data) - sizeof(*ip6h) computes
> the expected payload length. IOV_PEEK_HEADER() guarantees at least
> sizeof(*ip6h) bytes, but add an explicit check to guard the unsigned
> subtraction. A too-small tail would indicate a malformed packet, so
> skip it.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>
> 
> ---
> v2: Use if-guard instead of assert(), to avoid runtime cost in
>     per-packet path and to let the static checker follow the logic.

Why would the if be cheaper than an assert()?  The IOV_PEEK_HEADER()
already checks the length, so this check is definitely redundant - it
exists only for the benefit of static checkers.

> ---
>  tap.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/tap.c b/tap.c
> index 6d93c7ce..6fd5f595 100644
> --- a/tap.c
> +++ b/tap.c
> @@ -986,7 +986,10 @@ resume:
>  		if (!ip6h)
>  			continue;
>  
> -		check = iov_tail_size(&data) - sizeof(*ip6h);
> +		check = iov_tail_size(&data);
> +		if (check < sizeof(*ip6h))
> +			continue;
> +		check -= sizeof(*ip6h);
>  
>  		saddr = &ip6h->saddr;
>  		daddr = &ip6h->daddr;
> -- 
> 2.52.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 --]

Re: [PATCH v2 4/6] doc/migration: Use snprintf() for socket path and fix argv access

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2476 bytes --]

On Thu, Jul 09, 2026 at 05:56:54PM -0400, Jon Maloy wrote:
> Replace strcpy() with snprintf() when copying the helper socket path
> into sun_path, preventing a potential buffer overflow from a long
> argument and guaranteeing null-termination.
> 
> In target.c, move the argc check before accessing argv[4], so we
> don't dereference past the end of argv.
> 
> Signed-off-by: Jon Maloy <jmaloy@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

Although... it wouldn't surprise me if some static checker now
complains about not testing the return value from snprintf().

> 
> ---
> v2: Use snprintf() instead of strncpy() for clarity and
>     guaranteed null-termination, as suggested by David Gibson
> ---
>  doc/migration/source.c | 2 +-
>  doc/migration/target.c | 8 +++-----
>  2 files changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/doc/migration/source.c b/doc/migration/source.c
> index d44ebf1f..8aad0259 100644
> --- a/doc/migration/source.c
> +++ b/doc/migration/source.c
> @@ -47,7 +47,7 @@ int main(int argc, char **argv)
>  		return -1;
>  	}
>  
> -	strcpy(a_helper.sun_path, argv[4]);
> +	snprintf(a_helper.sun_path, sizeof(a_helper.sun_path), "%s", argv[4]);
>  	getaddrinfo(argv[1], argv[2], &hints, &r);
>  
>  	/* Connect socket to server and send some data */
> diff --git a/doc/migration/target.c b/doc/migration/target.c
> index f7d31083..e2469837 100644
> --- a/doc/migration/target.c
> +++ b/doc/migration/target.c
> @@ -38,11 +38,6 @@ int main(int argc, char **argv)
>  	struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg);
>  	struct addrinfo *r;
>  
> -	(void)argc;
> -
> -	strcpy(a_helper.sun_path, argv[4]);
> -	getaddrinfo(argv[1], argv[2], &hints, &r);
> -
>  	if (argc != 7) {
>  		fprintf(stderr,
>  			"%s DST_ADDR DST_PORT SRC_PORT HELPER_PATH SSEQ RSEQ\n",
> @@ -50,6 +45,9 @@ int main(int argc, char **argv)
>  		return -1;
>  	}
>  
> +	snprintf(a_helper.sun_path, sizeof(a_helper.sun_path), "%s", argv[4]);
> +	getaddrinfo(argv[1], argv[2], &hints, &r);
> +
>  	/* Prepare socket, bind to source port */
>  	s = socket(r->ai_family, SOCK_STREAM, IPPROTO_TCP);
>  	setsockopt(s, SOL_SOCKET, SO_REUSEADDR, &((int){ 1 }), sizeof(int));
> -- 
> 2.52.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 --]