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

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

...

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