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

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>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
[sbrivio: Fix up comments to @dns in structs ip4_ctx and ip6_ctx]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

5 files changed, 19 insertions, 9 deletions

diff --git a/conf.c b/conf.c
index 84ae12b..b1fc4b9 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 6b9c2e3..1ff8cba 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 e4df0db..d94be23 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 eb9e313..1f2bcb0 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 79d01dd..f3664ce 100644
--- a/passt.h
+++ b/passt.h
@@ -74,7 +74,7 @@ enum passt_modes {
  *                      host's 127.0.0.1
  * @map_guest_addr:	Outbound connections to this address are NATted to the
  *                      guest's assigned address
- * @dns:		DNS addresses for DHCP, zero-terminated
+ * @dns:		DNS addresses for DHCP
  * @dns_match:		Forward DNS query if sent to this address
  * @our_tap_addr:	IPv4 address for passt's use on tap
  * @dns_host:		Use this DNS on the host for forwarding
@@ -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;
 
@@ -115,7 +115,7 @@ struct ip4_ctx {
  *                      host's [::1]
  * @map_guest_addr:	Outbound connections to this address are NATted to the
  *                      guest's assigned address
- * @dns:		DNS addresses for DHCPv6 and NDP, zero-terminated
+ * @dns:		DNS addresses for DHCPv6 and NDP
  * @dns_match:		Forward DNS query if sent to this address
  * @our_tap_ll:		Link-local IPv6 address for passt's use on tap
  * @dns_host:		Use this DNS on the host for forwarding
@@ -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;