From 4afaba8b8fa575ffc3d3af2709a4bf9f8c4476a1 Mon Sep 17 00:00:00 2001
From: kn <kn@openbsd.org>
Date: Fri, 6 Jan 2023 14:29:47 +0000
Subject: [PATCH] Clarify nd6_option() return semantics

nd_opts_last is set only once in nd6_options() during struct init and
guaranteed non-NULL as it is set to the function's argument *opt which
is passed in as (struct_ptr + 1) in both callers.

nd6_option(), the internal helper, returns a pointer to the next option
or NULL, which means either "no option, ok" or "invalid option, fail".

Failure is signaled through nd_opts_last being NULL after nd6_option()
returned, which only happens if nd6_option() zeroed the whole *ndopts.

Move the two cases under mnemonic labels and zap the now obviously
redundant bzero() call in nd6_options().

OK claudio
---
 sys/netinet6/nd6.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/sys/netinet6/nd6.c b/sys/netinet6/nd6.c
index f13f31875b4..9d430d3d32e 100644
--- a/sys/netinet6/nd6.c
+++ b/sys/netinet6/nd6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nd6.c,v 1.261 2023/01/06 14:24:36 kn Exp $	*/
+/*	$OpenBSD: nd6.c,v 1.262 2023/01/06 14:29:47 kn Exp $	*/
 /*	$KAME: nd6.c,v 1.280 2002/06/08 19:52:07 itojun Exp $	*/
 
 /*
@@ -148,17 +148,14 @@ nd6_option(struct nd_opts *ndopts)
 	struct nd_opt_hdr *nd_opt;
 	int olen;
 
-	if (!ndopts->nd_opts_search)
-		return NULL;
-	if (ndopts->nd_opts_done)
-		return NULL;
+	if (ndopts->nd_opts_search == NULL || ndopts->nd_opts_done)
+		goto skip;
 
 	nd_opt = ndopts->nd_opts_search;
 
 	/* make sure nd_opt_len is inside the buffer */
 	if ((caddr_t)&nd_opt->nd_opt_len >= (caddr_t)ndopts->nd_opts_last) {
-		bzero(ndopts, sizeof(*ndopts));
-		return NULL;
+		goto invalid;
 	}
 
 	olen = nd_opt->nd_opt_len << 3;
@@ -167,21 +164,23 @@ nd6_option(struct nd_opts *ndopts)
 		 * Message validation requires that all included
 		 * options have a length that is greater than zero.
 		 */
-		bzero(ndopts, sizeof(*ndopts));
-		return NULL;
+		goto invalid;
 	}
 
 	ndopts->nd_opts_search = (struct nd_opt_hdr *)((caddr_t)nd_opt + olen);
 	if (ndopts->nd_opts_search > ndopts->nd_opts_last) {
 		/* option overruns the end of buffer, invalid */
-		bzero(ndopts, sizeof(*ndopts));
-		return NULL;
+		goto invalid;
 	} else if (ndopts->nd_opts_search == ndopts->nd_opts_last) {
 		/* reached the end of options chain */
 		ndopts->nd_opts_done = 1;
 		ndopts->nd_opts_search = NULL;
 	}
 	return nd_opt;
+invalid:
+	bzero(ndopts, sizeof(*ndopts));
+skip:
+	return NULL;
 }
 
 /*
@@ -212,7 +211,6 @@ nd6_options(void *opt, int icmp6len, struct nd_opts *ndopts)
 			 * options have a length that is greater than zero.
 			 */
 			icmp6stat_inc(icp6s_nd_badopt);
-			bzero(ndopts, sizeof(*ndopts));
 			return -1;
 		}
 
-- 
2.20.1