[PATCH 0/7] Fix issues and false positives in code coverge tool

[PATCH 0/7] Fix issues and false positives in code coverge tool

[Jon Maloy]

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

The core code changes addresses missing fd initialisations and
explicit invariant checks for unsigned arithmetic.

We also fix address buffer handling, type mismatches, uninitialised
variables, and resource leaks in the documentation example code.

Jon Maloy (7):
  passt: Initialise listening socket fds to -1
  udp_vu: Assert iov_tail_clone() return before assigning to msg_iovlen
  tap: Assert IPv6 tail size before subtracting header length
  doc/migration: Use strncpy() for socket path and fix argv access
  doc/migration: Fix buffer type mismatch in recv() call
  doc/platform-requirements: Initialise va_list before va_start()
  doc/platform-requirements: Close leaked sockets in test_close_dup()

 doc/migration/source.c                    |  2 +-
 doc/migration/target.c                    | 10 ++++------
 doc/platform-requirements/common.h        |  2 +-
 doc/platform-requirements/udp-close-dup.c |  3 +++
 passt.c                                   |  3 +++
 tap.c                                     |  4 +++-
 udp_vu.c                                  |  7 +++++--
 7 files changed, 20 insertions(+), 11 deletions(-)

-- 
2.52.0

[PATCH 1/7] 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>
---
 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 2/7] udp_vu: Assert iov_tail_clone() return before assigning to msg_iovlen

[Jon Maloy]

iov_tail_clone() returns ssize_t, but its return value was assigned
directly to msg.msg_iovlen which is size_t. Add an assert to document
the invariant that the clone into a VIRTQUEUE_MAX_SIZE array cannot
fail.

Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
 udp_vu.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/udp_vu.c b/udp_vu.c
index e4fb1057..5d6f45ec 100644
--- a/udp_vu.c
+++ b/udp_vu.c
@@ -67,11 +67,14 @@ 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);
+	assert(iovlen >= 0);
+
 	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 3/7] tap: Assert IPv6 tail size before subtracting header length

[Jon Maloy]

In tap6_handler(), iov_tail_size(&data) - sizeof(*ip6h) is used to
compute the expected payload length. The subtraction is safe because
IOV_PEEK_HEADER() already verified that the tail contains at least
sizeof(*ip6h) bytes, but add an assert to make the invariant explicit.

Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
 tap.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

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

[PATCH 4/7] doc/migration: Use strncpy() for socket path and fix argv access

[Jon Maloy]

Replace strcpy() with strncpy() when copying the helper socket path
into sun_path, preventing a potential buffer overflow from a long
argument.

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>
---
 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..77ff8fda 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]);
+	strncpy(a_helper.sun_path, argv[4], sizeof(a_helper.sun_path) - 1);
 	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..0eb1f6ac 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;
 	}
 
+	strncpy(a_helper.sun_path, argv[4], sizeof(a_helper.sun_path) - 1);
+	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 5/7] 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>
---
 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 0eb1f6ac..78cfd564 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 6/7] doc/platform-requirements: Initialise va_list before va_start()

[Jon Maloy]

va_start() initialises the va_list, but zero-initialise it at
declaration as well to make it clear the variable is not used
uninitialised.

Signed-off-by: Jon Maloy <jmaloy@redhat.com>
---
 doc/platform-requirements/common.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/platform-requirements/common.h b/doc/platform-requirements/common.h
index e85fc2b5..815e8271 100644
--- a/doc/platform-requirements/common.h
+++ b/doc/platform-requirements/common.h
@@ -18,7 +18,7 @@
 __attribute__((format(printf, 1, 2), noreturn))
 static inline void die(const char *fmt, ...)
 {
-	va_list ap;
+	va_list ap = { 0 };
 
 	va_start(ap, fmt);
 	(void)vfprintf(stderr, fmt, ap);
-- 
2.52.0

[PATCH 7/7] 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>
---
 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