From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: Unlock ICMPV6CTL_ND6_MMAXTRIES case of icmp6_sysctl()
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Mon, 4 Aug 2025 11:54:37 +0200

On Mon, Aug 04, 2025 at 05:48:24AM +0300, Vitaliy Makkoveev wrote:
> Loaded only once in nd6_llinfo_timer().

ln->ln_asked is never negative.  So the unsigned cast in atomic_load_int()
does not matter.

OK bluhm@

> Index: sys/netinet6/icmp6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/icmp6.c,v
> retrieving revision 1.275
> diff -u -p -r1.275 icmp6.c
> --- sys/netinet6/icmp6.c	3 Aug 2025 11:12:58 -0000	1.275
> +++ sys/netinet6/icmp6.c	4 Aug 2025 00:56:38 -0000
> @@ -1775,10 +1775,10 @@ icmp6_mtudisc_timeout(struct rtentry *rt
>  const struct sysctl_bounded_args icmpv6ctl_vars_unlocked[] = {
>  	{ ICMPV6CTL_ND6_DELAY, &nd6_delay, 0, INT_MAX },
>  	{ ICMPV6CTL_ND6_UMAXTRIES, &nd6_umaxtries, 0, INT_MAX },
> +	{ ICMPV6CTL_ND6_MMAXTRIES, &nd6_mmaxtries, 0, INT_MAX },
>  };
>  
>  const struct sysctl_bounded_args icmpv6ctl_vars[] = {
> -	{ ICMPV6CTL_ND6_MMAXTRIES, &nd6_mmaxtries, 0, INT_MAX },
>  	{ ICMPV6CTL_ERRPPSLIMIT, &icmp6errppslim, -1, 1000 },
>  	{ ICMPV6CTL_ND6_MAXNUDHINT, &nd6_maxnudhint, 0, INT_MAX },
>  	{ ICMPV6CTL_MTUDISC_HIWAT, &icmp6_mtudisc_hiwat, -1, INT_MAX },
> @@ -1846,6 +1846,7 @@ icmp6_sysctl(int *name, u_int namelen, v
>  
>  	case ICMPV6CTL_ND6_DELAY:
>  	case ICMPV6CTL_ND6_UMAXTRIES:
> +	case ICMPV6CTL_ND6_MMAXTRIES:
>  		error = sysctl_bounded_arr(icmpv6ctl_vars_unlocked,
>  		    nitems(icmpv6ctl_vars_unlocked), name, namelen,
>  		    oldp, oldlenp, newp, newlen);
> Index: sys/netinet6/nd6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/nd6.c,v
> retrieving revision 1.296
> diff -u -p -r1.296 nd6.c
> --- sys/netinet6/nd6.c	3 Aug 2025 11:08:40 -0000	1.296
> +++ sys/netinet6/nd6.c	4 Aug 2025 00:56:38 -0000
> @@ -77,7 +77,7 @@ int	nd6_timer_next	= -1;	/* at which upt
>  time_t	nd6_expire_next	= -1;	/* at which uptime nd6_expire runs */
>  int	nd6_delay	= 5;	/* [a] delay first probe time 5 second */
>  int	nd6_umaxtries	= 3;	/* [a] maximum unicast query */
> -int	nd6_mmaxtries	= 3;	/* maximum multicast query */
> +int	nd6_mmaxtries	= 3;	/* [a] maximum multicast query */
>  int	nd6_gctimer	= (60 * 60 * 24); /* 1 day: garbage collection timer */
>  
>  /* preventing too many loops in ND option parsing */
> @@ -296,7 +296,7 @@ nd6_llinfo_timer(struct rtentry *rt, int
>  
>  	switch (ln->ln_state) {
>  	case ND6_LLINFO_INCOMPLETE:
> -		if (ln->ln_asked < nd6_mmaxtries) {
> +		if (ln->ln_asked < atomic_load_int(&nd6_mmaxtries)) {
>  			ln->ln_asked++;
>  			nd6_llinfo_settimer(ln, RETRANS_TIMER / 1000);
>  			nd6_ns_output(ifp, NULL, &dst->sin6_addr,