[PATCH 0/5] Fixes for Fedora 43 (or other bitrot)

[PATCH 0/5] Fixes for Fedora 43 (or other bitrot)

[David Gibson]

While away, I updated my laptop to Fedora 43.  Naturally a bunch of
things in the passt tests broke.  First there were the usual breakages
due to a new static checker version (also one real bug).  Then several
other problems.

David Gibson (5):
  util: Be more defensive about buffer overruns in read_file()
  migrate: Don't use terminator element for versions[] array
  treewide: Don't rely on terminator records in ip[46].dns arrays
  test: Handle Operating System Command escapes in terminal output
  test: Include sshd-auth in mbuto guest image

 conf.c           | 8 ++++++--
 dhcp.c           | 4 +++-
 dhcpv6.c         | 4 +++-
 migrate.c        | 3 +--
 ndp.c            | 4 +++-
 passt.h          | 4 ++--
 test/lib/term    | 4 ++--
 test/passt.mbuto | 6 ++++++
 util.c           | 2 +-
 9 files changed, 27 insertions(+), 12 deletions(-)

-- 
2.52.0

[PATCH 1/5] util: Be more defensive about buffer overruns in read_file()

[David Gibson]

clang-21.1.7 complains about read_file(), thinking that total_read might
come to exceed buf_size, leading to an out of bounds access at the end of
the function.  In fact, the semantics of read()'s return mean this can't
ever happen.  But we already have to check for the total_read == buf_size
case, so it's basically free to change it to >= and suppress the error.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 util.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/util.c b/util.c
index 27303950..a48f727c 100644
--- a/util.c
+++ b/util.c
@@ -715,7 +715,7 @@ static ssize_t read_file(const char *path, char *buf, size_t buf_size)
 
 	close(fd);
 
-	if (total_read == buf_size) {
+	if (total_read >= buf_size) {
 		buf[buf_size - 1] = '\0';
 		return -ENOBUFS;
 	}
-- 
2.52.0

[PATCH 2/5] migrate: Don't use terminator element for versions[] array

[David Gibson]

When scanning the versions[] array we use a dummy entry to detect when
we're finished.  Use ARRAY_SIZE() instead, which is almost as easy, a
little safer, and doesn't cause clang-tidy 21.1.7 to complain.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 migrate.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/migrate.c b/migrate.c
index 48d63a07..18ca18a8 100644
--- a/migrate.c
+++ b/migrate.c
@@ -123,7 +123,6 @@ static const struct migrate_version versions[] = {
 	 * MSS and omitted timestamps, which meant it usually wouldn't work.
 	 * Therefore we don't attempt to support compatibility with it.
 	 */
-	{ 0 },
 };
 
 /* Current encoding version */
@@ -196,7 +195,7 @@ static const struct migrate_version *migrate_target_read_header(int fd)
 		return NULL;
 	}
 
-	for (v = versions; v->id; v++)
+	for (v = versions; (v - versions) < ARRAY_SIZE(versions); v++)
 		if (v->id <= id && v->id >= compat_id)
 			return v;
 
-- 
2.52.0

[PATCH 3/5] treewide: Don't rely on terminator records in ip[46].dns arrays

[David Gibson]

In our arrays of DNS resolvers to pass to the guest we use a blank entry
to indicate the end of the list.  We rely on this when scanning the array,
not having separate bounds checking.  clang-tidy 21.1.7 has fancier
checking for array overruns in loops, but it's not able to reason that
there's always a terminating entry, so complains.

Indeed, it's correct to do so in this case.  Although we allow space in the
arrays for the terminator (size MAXNS + 1), add_dns[46]() check only for
    idx >= ARRAY_SIZE()
before adding an entry.  This allows it to consume the last slot with a
"real" entry, meaning the places where we scan really could overrun.

Fix the bug, and make it easier to reason about (for both clang-tidy and
people) by using ARRAY_SIZE() base bounds checking.  Treat the terminator
explicitly as an early exit case using 'break'.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 conf.c   | 8 ++++++--
 dhcp.c   | 4 +++-
 dhcpv6.c | 4 +++-
 ndp.c    | 4 +++-
 passt.h  | 4 ++--
 5 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/conf.c b/conf.c
index 84ae12b2..ceb9aa55 100644
--- a/conf.c
+++ b/conf.c
@@ -1159,7 +1159,9 @@ static void conf_print(const struct ctx *c)
 				       buf4, sizeof(buf4)));
 		}
 
-		for (i = 0; !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]); i++) {
+		for (i = 0; i < ARRAY_SIZE(c->ip4.dns); i++) {
+			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]))
+			    break;
 			if (!i)
 				info("DNS:");
 			inet_ntop(AF_INET, &c->ip4.dns[i], buf4, sizeof(buf4));
@@ -1197,7 +1199,9 @@ static void conf_print(const struct ctx *c)
 			       buf6, sizeof(buf6)));
 
 dns6:
-		for (i = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]); i++) {
+		for (i = 0; i < ARRAY_SIZE(c->ip6.dns); i++) {
+			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]))
+			    break;
 			if (!i)
 				info("DNS:");
 			inet_ntop(AF_INET6, &c->ip6.dns[i], buf6, sizeof(buf6));
diff --git a/dhcp.c b/dhcp.c
index 6b9c2e3b..1ff8cba9 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -430,7 +430,9 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
 	}
 
 	for (i = 0, opts[6].slen = 0;
-	     !c->no_dhcp_dns && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]); i++) {
+	     !c->no_dhcp_dns && i < ARRAY_SIZE(c->ip4.dns); i++) {
+		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]))
+			break;
 		((struct in_addr *)opts[6].s)[i] = c->ip4.dns[i];
 		opts[6].slen += sizeof(uint32_t);
 	}
diff --git a/dhcpv6.c b/dhcpv6.c
index e4df0db5..d94be23a 100644
--- a/dhcpv6.c
+++ b/dhcpv6.c
@@ -425,7 +425,9 @@ static size_t dhcpv6_dns_fill(const struct ctx *c, char *buf, int offset)
 	if (c->no_dhcp_dns)
 		goto search;
 
-	for (i = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]); i++) {
+	for (i = 0; i < ARRAY_SIZE(c->ip6.dns); i++) {
+		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]))
+			break;
 		if (!i) {
 			srv = (struct opt_dns_servers *)(buf + offset);
 			offset += sizeof(struct opt_hdr);
diff --git a/ndp.c b/ndp.c
index eb9e3139..1f2bcb0c 100644
--- a/ndp.c
+++ b/ndp.c
@@ -285,7 +285,9 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
 		size_t dns_s_len = 0;
 		int i, n;
 
-		for (n = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[n]); n++);
+		for (n = 0; n < ARRAY_SIZE(c->ip6.dns); n++)
+			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[n]))
+				break;
 		if (n) {
 			struct opt_rdnss *rdnss = (struct opt_rdnss *)ptr;
 			*rdnss = (struct opt_rdnss) {
diff --git a/passt.h b/passt.h
index 79d01ddb..87da76d3 100644
--- a/passt.h
+++ b/passt.h
@@ -91,7 +91,7 @@ struct ip4_ctx {
 	struct in_addr guest_gw;
 	struct in_addr map_host_loopback;
 	struct in_addr map_guest_addr;
-	struct in_addr dns[MAXNS + 1];
+	struct in_addr dns[MAXNS];
 	struct in_addr dns_match;
 	struct in_addr our_tap_addr;
 
@@ -132,7 +132,7 @@ struct ip6_ctx {
 	struct in6_addr guest_gw;
 	struct in6_addr map_host_loopback;
 	struct in6_addr map_guest_addr;
-	struct in6_addr dns[MAXNS + 1];
+	struct in6_addr dns[MAXNS];
 	struct in6_addr dns_match;
 	struct in6_addr our_tap_ll;
 
-- 
2.52.0

[PATCH 4/5] test: Handle Operating System Command escapes in terminal output

[David Gibson]

Our "old style" test script stuff uses raw terminal output scraped from
tmux.  This might include terminal escape sequences from the shell, or
whatever we run in it: they think they're talking to a terminal emulator
not a script, and they're not entirely incorrect.

In several places we filter out ANSI ESC-[ sequences to make this work.
However, this doesn't include ESC-] "Operating System Command" sequences.
Something I've updated in Fedora 43 generates heaps of these, which break
everything.

Add more hairy regexps to filter these out as well.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 test/lib/term | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/test/lib/term b/test/lib/term
index f596364c..89e4fdbe 100755
--- a/test/lib/term
+++ b/test/lib/term
@@ -203,7 +203,7 @@ pane_wait() {
 
 	__done=0
 	while
-		__l="$(tail -1 ${LOGDIR}/pane_${__lc}.log | sed 's/^[[[][^a-zA-Z]*[a-zA-Z]//g')"
+		__l="$(tail -1 ${LOGDIR}/pane_${__lc}.log | sed 's/^[[[][^a-zA-Z]*[a-zA-Z]//g;s/^[[]][^^[]*^[[\]//g')"
 		case ${__l} in
 		*"$ " | *"# ") return ;;
 		esac
@@ -215,7 +215,7 @@ pane_wait() {
 pane_parse() {
 	__pane_lc="$(echo "${1}" | tr [A-Z] [a-z])"
 
-	__buf="$(tail -n2 ${LOGDIR}/pane_${__pane_lc}.log | head -n1 | sed 's/^[^\r]*\r\([^\r]\)/\1/' | tr -d '\r\n')"
+	__buf="$(tail -n2 ${LOGDIR}/pane_${__pane_lc}.log | head -n1 | sed 's/^[^\r]*\r\([^\r]\)/\1/;s/^[[]][^^[]*^[[\]//g' | tr -d '\r\n')"
 
 	[ "# $(eval printf '%s' \"\$${1}_LAST_CMD\")" != "${__buf}" ] && \
 	[ "$ $(eval printf '%s' \"\$${1}_LAST_CMD\")" != "${__buf}" ] &&
-- 
2.52.0

[PATCH 5/5] test: Include sshd-auth in mbuto guest image

[David Gibson]

OpenSSH has split itself into several binaries for greater security.  We
already have logic to make sure we include the sshd-session binary.
OpenSSH 10 has added another: sshd-auth which we also need for a working
sshd within the guest.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 test/passt.mbuto | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/test/passt.mbuto b/test/passt.mbuto
index 598c2547..de35c3ce 100755
--- a/test/passt.mbuto
+++ b/test/passt.mbuto
@@ -24,6 +24,12 @@ for bin in /usr/lib/openssh/sshd-session /usr/lib/ssh/sshd-session \
 	command -v "${bin}" >/dev/null && PROGS="${PROGS} ${bin}"
 done
 
+# OpenSSH 10 adds sshd-auth as well
+for bin in /usr/lib/openssh/sshd-auth /usr/lib/ssh/sshd-auth \
+	   /usr/libexec/openssh/sshd-auth; do
+	command -v "${bin}" >/dev/null && PROGS="${PROGS} ${bin}"
+done
+
 KMODS="${KMODS:- virtio_net virtio_pci vmw_vsock_virtio_transport}"
 
 LINKS="${LINKS:-
-- 
2.52.0

Re: [PATCH 1/5] util: Be more defensive about buffer overruns in read_file()

[Laurent Vivier]

On 1/5/26 08:53, David Gibson wrote:
> clang-21.1.7 complains about read_file(), thinking that total_read might
> come to exceed buf_size, leading to an out of bounds access at the end of
> the function.  In fact, the semantics of read()'s return mean this can't
> ever happen.  But we already have to check for the total_read == buf_size
> case, so it's basically free to change it to >= and suppress the error.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   util.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util.c b/util.c
> index 27303950..a48f727c 100644
> --- a/util.c
> +++ b/util.c
> @@ -715,7 +715,7 @@ static ssize_t read_file(const char *path, char *buf, size_t buf_size)
>   
>   	close(fd);
>   
> -	if (total_read == buf_size) {
> +	if (total_read >= buf_size) {
>   		buf[buf_size - 1] = '\0';
>   		return -ENOBUFS;
>   	}

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Re: [PATCH 2/5] migrate: Don't use terminator element for versions[] array

[Laurent Vivier]

On 1/5/26 08:53, David Gibson wrote:
> When scanning the versions[] array we use a dummy entry to detect when
> we're finished.  Use ARRAY_SIZE() instead, which is almost as easy, a
> little safer, and doesn't cause clang-tidy 21.1.7 to complain.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   migrate.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/migrate.c b/migrate.c
> index 48d63a07..18ca18a8 100644
> --- a/migrate.c
> +++ b/migrate.c
> @@ -123,7 +123,6 @@ static const struct migrate_version versions[] = {
>   	 * MSS and omitted timestamps, which meant it usually wouldn't work.
>   	 * Therefore we don't attempt to support compatibility with it.
>   	 */
> -	{ 0 },
>   };
>   
>   /* Current encoding version */
> @@ -196,7 +195,7 @@ static const struct migrate_version *migrate_target_read_header(int fd)
>   		return NULL;
>   	}
>   
> -	for (v = versions; v->id; v++)
> +	for (v = versions; (v - versions) < ARRAY_SIZE(versions); v++)
>   		if (v->id <= id && v->id >= compat_id)
>   			return v;
>   

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Re: [PATCH 2/5] migrate: Don't use terminator element for versions[] array

[Laurent Vivier]

On 1/5/26 08:53, David Gibson wrote:
> When scanning the versions[] array we use a dummy entry to detect when
> we're finished.  Use ARRAY_SIZE() instead, which is almost as easy, a
> little safer, and doesn't cause clang-tidy 21.1.7 to complain.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   migrate.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/migrate.c b/migrate.c
> index 48d63a07..18ca18a8 100644
> --- a/migrate.c
> +++ b/migrate.c
> @@ -123,7 +123,6 @@ static const struct migrate_version versions[] = {
>   	 * MSS and omitted timestamps, which meant it usually wouldn't work.
>   	 * Therefore we don't attempt to support compatibility with it.
>   	 */
> -	{ 0 },
>   };
>   
>   /* Current encoding version */
> @@ -196,7 +195,7 @@ static const struct migrate_version *migrate_target_read_header(int fd)
>   		return NULL;
>   	}
>   
> -	for (v = versions; v->id; v++)
> +	for (v = versions; (v - versions) < ARRAY_SIZE(versions); v++)
>   		if (v->id <= id && v->id >= compat_id)
>   			return v;
>   

As we don't need to check v->id on the loop perhaps we can use something more standard like:

for (i = 0; i < ARRAY_SIZE(versions); i++)
	if (versions[i].id <= id && versions[i].id >= compat_id)
		return &versions[i];

Thanks,
Laurent

Re: [PATCH 3/5] treewide: Don't rely on terminator records in ip[46].dns arrays

[Laurent Vivier]

On 1/5/26 08:53, David Gibson wrote:
> In our arrays of DNS resolvers to pass to the guest we use a blank entry
> to indicate the end of the list.  We rely on this when scanning the array,
> not having separate bounds checking.  clang-tidy 21.1.7 has fancier
> checking for array overruns in loops, but it's not able to reason that
> there's always a terminating entry, so complains.
> 
> Indeed, it's correct to do so in this case.  Although we allow space in the
> arrays for the terminator (size MAXNS + 1), add_dns[46]() check only for
>      idx >= ARRAY_SIZE()
> before adding an entry.  This allows it to consume the last slot with a
> "real" entry, meaning the places where we scan really could overrun.
> 
> Fix the bug, and make it easier to reason about (for both clang-tidy and
> people) by using ARRAY_SIZE() base bounds checking.  Treat the terminator
> explicitly as an early exit case using 'break'.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   conf.c   | 8 ++++++--
>   dhcp.c   | 4 +++-
>   dhcpv6.c | 4 +++-
>   ndp.c    | 4 +++-
>   passt.h  | 4 ++--
>   5 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/conf.c b/conf.c
> index 84ae12b2..ceb9aa55 100644
> --- a/conf.c
> +++ b/conf.c
> @@ -1159,7 +1159,9 @@ static void conf_print(const struct ctx *c)
>   				       buf4, sizeof(buf4)));
>   		}
>   
> -		for (i = 0; !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]); i++) {
> +		for (i = 0; i < ARRAY_SIZE(c->ip4.dns); i++) {
> +			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]))
> +			    break;

It should be "\t" rather than "    ".

Otherwise:

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

>   			if (!i)
>   				info("DNS:");
>   			inet_ntop(AF_INET, &c->ip4.dns[i], buf4, sizeof(buf4));
> @@ -1197,7 +1199,9 @@ static void conf_print(const struct ctx *c)
>   			       buf6, sizeof(buf6)));
>   
>   dns6:
> -		for (i = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]); i++) {
> +		for (i = 0; i < ARRAY_SIZE(c->ip6.dns); i++) {
> +			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]))
> +			    break;
>   			if (!i)
>   				info("DNS:");
>   			inet_ntop(AF_INET6, &c->ip6.dns[i], buf6, sizeof(buf6));
> diff --git a/dhcp.c b/dhcp.c
> index 6b9c2e3b..1ff8cba9 100644
> --- a/dhcp.c
> +++ b/dhcp.c
> @@ -430,7 +430,9 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
>   	}
>   
>   	for (i = 0, opts[6].slen = 0;
> -	     !c->no_dhcp_dns && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]); i++) {
> +	     !c->no_dhcp_dns && i < ARRAY_SIZE(c->ip4.dns); i++) {
> +		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]))
> +			break;
>   		((struct in_addr *)opts[6].s)[i] = c->ip4.dns[i];
>   		opts[6].slen += sizeof(uint32_t);
>   	}
> diff --git a/dhcpv6.c b/dhcpv6.c
> index e4df0db5..d94be23a 100644
> --- a/dhcpv6.c
> +++ b/dhcpv6.c
> @@ -425,7 +425,9 @@ static size_t dhcpv6_dns_fill(const struct ctx *c, char *buf, int offset)
>   	if (c->no_dhcp_dns)
>   		goto search;
>   
> -	for (i = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]); i++) {
> +	for (i = 0; i < ARRAY_SIZE(c->ip6.dns); i++) {
> +		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]))
> +			break;
>   		if (!i) {
>   			srv = (struct opt_dns_servers *)(buf + offset);
>   			offset += sizeof(struct opt_hdr);
> diff --git a/ndp.c b/ndp.c
> index eb9e3139..1f2bcb0c 100644
> --- a/ndp.c
> +++ b/ndp.c
> @@ -285,7 +285,9 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
>   		size_t dns_s_len = 0;
>   		int i, n;
>   
> -		for (n = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[n]); n++);
> +		for (n = 0; n < ARRAY_SIZE(c->ip6.dns); n++)
> +			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[n]))
> +				break;
>   		if (n) {
>   			struct opt_rdnss *rdnss = (struct opt_rdnss *)ptr;
>   			*rdnss = (struct opt_rdnss) {
> diff --git a/passt.h b/passt.h
> index 79d01ddb..87da76d3 100644
> --- a/passt.h
> +++ b/passt.h
> @@ -91,7 +91,7 @@ struct ip4_ctx {
>   	struct in_addr guest_gw;
>   	struct in_addr map_host_loopback;
>   	struct in_addr map_guest_addr;
> -	struct in_addr dns[MAXNS + 1];
> +	struct in_addr dns[MAXNS];
>   	struct in_addr dns_match;
>   	struct in_addr our_tap_addr;
>   
> @@ -132,7 +132,7 @@ struct ip6_ctx {
>   	struct in6_addr guest_gw;
>   	struct in6_addr map_host_loopback;
>   	struct in6_addr map_guest_addr;
> -	struct in6_addr dns[MAXNS + 1];
> +	struct in6_addr dns[MAXNS];
>   	struct in6_addr dns_match;
>   	struct in6_addr our_tap_ll;
>   

Re: [PATCH 4/5] test: Handle Operating System Command escapes in terminal output

[Laurent Vivier]

On 1/5/26 08:53, David Gibson wrote:
> Our "old style" test script stuff uses raw terminal output scraped from
> tmux.  This might include terminal escape sequences from the shell, or
> whatever we run in it: they think they're talking to a terminal emulator
> not a script, and they're not entirely incorrect.
> 
> In several places we filter out ANSI ESC-[ sequences to make this work.
> However, this doesn't include ESC-] "Operating System Command" sequences.
> Something I've updated in Fedora 43 generates heaps of these, which break
> everything.
> 
> Add more hairy regexps to filter these out as well.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   test/lib/term | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/test/lib/term b/test/lib/term
> index f596364c..89e4fdbe 100755
> --- a/test/lib/term
> +++ b/test/lib/term
> @@ -203,7 +203,7 @@ pane_wait() {
>   
>   	__done=0
>   	while
> -		__l="$(tail -1 ${LOGDIR}/pane_${__lc}.log | sed 's/^[[[][^a-zA-Z]*[a-zA-Z]//g')"
> +		__l="$(tail -1 ${LOGDIR}/pane_${__lc}.log | sed 's/^[[[][^a-zA-Z]*[a-zA-Z]//g;s/^[[]][^^[]*^[[\]//g')"
>   		case ${__l} in
>   		*"$ " | *"# ") return ;;
>   		esac
> @@ -215,7 +215,7 @@ pane_wait() {
>   pane_parse() {
>   	__pane_lc="$(echo "${1}" | tr [A-Z] [a-z])"
>   
> -	__buf="$(tail -n2 ${LOGDIR}/pane_${__pane_lc}.log | head -n1 | sed 's/^[^\r]*\r\([^\r]\)/\1/' | tr -d '\r\n')"
> +	__buf="$(tail -n2 ${LOGDIR}/pane_${__pane_lc}.log | head -n1 | sed 's/^[^\r]*\r\([^\r]\)/\1/;s/^[[]][^^[]*^[[\]//g' | tr -d '\r\n')"
>   
>   	[ "# $(eval printf '%s' \"\$${1}_LAST_CMD\")" != "${__buf}" ] && \
>   	[ "$ $(eval printf '%s' \"\$${1}_LAST_CMD\")" != "${__buf}" ] &&

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Re: [PATCH 5/5] test: Include sshd-auth in mbuto guest image

[Laurent Vivier]

On 1/5/26 08:53, David Gibson wrote:
> OpenSSH has split itself into several binaries for greater security.  We
> already have logic to make sure we include the sshd-session binary.
> OpenSSH 10 has added another: sshd-auth which we also need for a working
> sshd within the guest.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>   test/passt.mbuto | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/test/passt.mbuto b/test/passt.mbuto
> index 598c2547..de35c3ce 100755
> --- a/test/passt.mbuto
> +++ b/test/passt.mbuto
> @@ -24,6 +24,12 @@ for bin in /usr/lib/openssh/sshd-session /usr/lib/ssh/sshd-session \
>   	command -v "${bin}" >/dev/null && PROGS="${PROGS} ${bin}"
>   done
>   
> +# OpenSSH 10 adds sshd-auth as well
> +for bin in /usr/lib/openssh/sshd-auth /usr/lib/ssh/sshd-auth \
> +	   /usr/libexec/openssh/sshd-auth; do
> +	command -v "${bin}" >/dev/null && PROGS="${PROGS} ${bin}"
> +done
> +
>   KMODS="${KMODS:- virtio_net virtio_pci vmw_vsock_virtio_transport}"
>   
>   LINKS="${LINKS:-

Reviewed-by: Laurent Vivier <lvivier@redhat.com>

Re: [PATCH 2/5] migrate: Don't use terminator element for versions[] array

[David Gibson]

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

On Tue, Jan 06, 2026 at 02:47:18PM +0100, Laurent Vivier wrote:
> On 1/5/26 08:53, David Gibson wrote:
> > When scanning the versions[] array we use a dummy entry to detect when
> > we're finished.  Use ARRAY_SIZE() instead, which is almost as easy, a
> > little safer, and doesn't cause clang-tidy 21.1.7 to complain.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >   migrate.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/migrate.c b/migrate.c
> > index 48d63a07..18ca18a8 100644
> > --- a/migrate.c
> > +++ b/migrate.c
> > @@ -123,7 +123,6 @@ static const struct migrate_version versions[] = {
> >   	 * MSS and omitted timestamps, which meant it usually wouldn't work.
> >   	 * Therefore we don't attempt to support compatibility with it.
> >   	 */
> > -	{ 0 },
> >   };
> >   /* Current encoding version */
> > @@ -196,7 +195,7 @@ static const struct migrate_version *migrate_target_read_header(int fd)
> >   		return NULL;
> >   	}
> > -	for (v = versions; v->id; v++)
> > +	for (v = versions; (v - versions) < ARRAY_SIZE(versions); v++)
> >   		if (v->id <= id && v->id >= compat_id)
> >   			return v;
> 
> As we don't need to check v->id on the loop perhaps we can use something more standard like:
> 
> for (i = 0; i < ARRAY_SIZE(versions); i++)
> 	if (versions[i].id <= id && versions[i].id >= compat_id)
> 		return &versions[i];

Good idea, done.

-- 
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 3/5] treewide: Don't rely on terminator records in ip[46].dns arrays

[David Gibson]

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

On Tue, Jan 06, 2026 at 02:53:24PM +0100, Laurent Vivier wrote:
> On 1/5/26 08:53, David Gibson wrote:
> > In our arrays of DNS resolvers to pass to the guest we use a blank entry
> > to indicate the end of the list.  We rely on this when scanning the array,
> > not having separate bounds checking.  clang-tidy 21.1.7 has fancier
> > checking for array overruns in loops, but it's not able to reason that
> > there's always a terminating entry, so complains.
> > 
> > Indeed, it's correct to do so in this case.  Although we allow space in the
> > arrays for the terminator (size MAXNS + 1), add_dns[46]() check only for
> >      idx >= ARRAY_SIZE()
> > before adding an entry.  This allows it to consume the last slot with a
> > "real" entry, meaning the places where we scan really could overrun.
> > 
> > Fix the bug, and make it easier to reason about (for both clang-tidy and
> > people) by using ARRAY_SIZE() base bounds checking.  Treat the terminator
> > explicitly as an early exit case using 'break'.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >   conf.c   | 8 ++++++--
> >   dhcp.c   | 4 +++-
> >   dhcpv6.c | 4 +++-
> >   ndp.c    | 4 +++-
> >   passt.h  | 4 ++--
> >   5 files changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/conf.c b/conf.c
> > index 84ae12b2..ceb9aa55 100644
> > --- a/conf.c
> > +++ b/conf.c
> > @@ -1159,7 +1159,9 @@ static void conf_print(const struct ctx *c)
> >   				       buf4, sizeof(buf4)));
> >   		}
> > -		for (i = 0; !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]); i++) {
> > +		for (i = 0; i < ARRAY_SIZE(c->ip4.dns); i++) {
> > +			if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]))
> > +			    break;
> 
> It should be "\t" rather than "    ".

Oops, not sure how that happened.  Fixed.

> 
> Otherwise:
> 
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
> 
> >   			if (!i)
> >   				info("DNS:");
> >   			inet_ntop(AF_INET, &c->ip4.dns[i], buf4, sizeof(buf4));
> > @@ -1197,7 +1199,9 @@ static void conf_print(const struct ctx *c)
> >   			       buf6, sizeof(buf6)));
> >   dns6:
> > -		for (i = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]); i++) {
> > +		for (i = 0; i < ARRAY_SIZE(c->ip6.dns); i++) {
> > +			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]))
> > +			    break;
> >   			if (!i)
> >   				info("DNS:");
> >   			inet_ntop(AF_INET6, &c->ip6.dns[i], buf6, sizeof(buf6));
> > diff --git a/dhcp.c b/dhcp.c
> > index 6b9c2e3b..1ff8cba9 100644
> > --- a/dhcp.c
> > +++ b/dhcp.c
> > @@ -430,7 +430,9 @@ int dhcp(const struct ctx *c, struct iov_tail *data)
> >   	}
> >   	for (i = 0, opts[6].slen = 0;
> > -	     !c->no_dhcp_dns && !IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]); i++) {
> > +	     !c->no_dhcp_dns && i < ARRAY_SIZE(c->ip4.dns); i++) {
> > +		if (IN4_IS_ADDR_UNSPECIFIED(&c->ip4.dns[i]))
> > +			break;
> >   		((struct in_addr *)opts[6].s)[i] = c->ip4.dns[i];
> >   		opts[6].slen += sizeof(uint32_t);
> >   	}
> > diff --git a/dhcpv6.c b/dhcpv6.c
> > index e4df0db5..d94be23a 100644
> > --- a/dhcpv6.c
> > +++ b/dhcpv6.c
> > @@ -425,7 +425,9 @@ static size_t dhcpv6_dns_fill(const struct ctx *c, char *buf, int offset)
> >   	if (c->no_dhcp_dns)
> >   		goto search;
> > -	for (i = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]); i++) {
> > +	for (i = 0; i < ARRAY_SIZE(c->ip6.dns); i++) {
> > +		if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[i]))
> > +			break;
> >   		if (!i) {
> >   			srv = (struct opt_dns_servers *)(buf + offset);
> >   			offset += sizeof(struct opt_hdr);
> > diff --git a/ndp.c b/ndp.c
> > index eb9e3139..1f2bcb0c 100644
> > --- a/ndp.c
> > +++ b/ndp.c
> > @@ -285,7 +285,9 @@ static void ndp_ra(const struct ctx *c, const struct in6_addr *dst)
> >   		size_t dns_s_len = 0;
> >   		int i, n;
> > -		for (n = 0; !IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[n]); n++);
> > +		for (n = 0; n < ARRAY_SIZE(c->ip6.dns); n++)
> > +			if (IN6_IS_ADDR_UNSPECIFIED(&c->ip6.dns[n]))
> > +				break;
> >   		if (n) {
> >   			struct opt_rdnss *rdnss = (struct opt_rdnss *)ptr;
> >   			*rdnss = (struct opt_rdnss) {
> > diff --git a/passt.h b/passt.h
> > index 79d01ddb..87da76d3 100644
> > --- a/passt.h
> > +++ b/passt.h
> > @@ -91,7 +91,7 @@ struct ip4_ctx {
> >   	struct in_addr guest_gw;
> >   	struct in_addr map_host_loopback;
> >   	struct in_addr map_guest_addr;
> > -	struct in_addr dns[MAXNS + 1];
> > +	struct in_addr dns[MAXNS];
> >   	struct in_addr dns_match;
> >   	struct in_addr our_tap_addr;
> > @@ -132,7 +132,7 @@ struct ip6_ctx {
> >   	struct in6_addr guest_gw;
> >   	struct in6_addr map_host_loopback;
> >   	struct in6_addr map_guest_addr;
> > -	struct in6_addr dns[MAXNS + 1];
> > +	struct in6_addr dns[MAXNS];
> >   	struct in6_addr dns_match;
> >   	struct in6_addr our_tap_ll;
> 

-- 
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 --]