From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: Unlock ICMPV6CTL_REDIRTIMEOUT case of icmp6_sysctl()
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Fri, 1 Aug 2025 13:39:39 +0200

On Fri, Aug 01, 2025 at 02:00:24PM +0300, Vitaliy Makkoveev wrote:
> rt_timer_queue_change() is mp-safe, so make unlocking in the way of
> similar IP{V6}CTL_MTUDISCTIMEOUT cases of ip{,6}_sysctl().

OK bluhm@

> Index: sys/netinet6/icmp6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/icmp6.c,v
> diff -u -p -r1.271 icmp6.c
> --- sys/netinet6/icmp6.c	27 Jul 2025 17:46:58 -0000	1.271
> +++ sys/netinet6/icmp6.c	1 Aug 2025 08:36:10 -0000
> @@ -1801,6 +1801,12 @@ icmp6_sysctl_icmp6stat(void *oldp, size_
>  	return (ret);
>  }
>  
> +/*
> + * Temporary, should be replaced with sysctl_lock after icmp6_sysctl()
> + * unlocking.
> + */
> +struct rwlock icmp6_sysctl_lock = RWLOCK_INITIALIZER("icmp6rwl"); 
> +
>  int
>  icmp6_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp,
>      void *newp, size_t newlen)
> @@ -1812,15 +1818,22 @@ icmp6_sysctl(int *name, u_int namelen, v
>  		return (ENOTDIR);
>  
>  	switch (name[0]) {
> -	case ICMPV6CTL_REDIRTIMEOUT:
> -		NET_LOCK();
> +	case ICMPV6CTL_REDIRTIMEOUT: {
> +		int oldval, newval, error;
> +
> +		oldval = newval = atomic_load_int(&icmp6_redirtimeout);
>  		error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
> -		    &icmp6_redirtimeout, 0, INT_MAX);
> -		rt_timer_queue_change(&icmp6_redirect_timeout_q,
> -		    icmp6_redirtimeout);
> -		NET_UNLOCK();
> -		break;
> +		    &newval, 0, INT_MAX);
> +		if (error == 0 && oldval != newval) {
> +			rw_enter_write(&icmp6_sysctl_lock);
> +			atomic_store_int(&icmp6_redirtimeout, newval);
> +			rt_timer_queue_change(&icmp6_redirect_timeout_q,
> +			    newval);
> +			rw_exit_write(&icmp6_sysctl_lock);
> +		}
>  
> +		return (error);
> +	}
>  	case ICMPV6CTL_STATS:
>  		error = icmp6_sysctl_icmp6stat(oldp, oldlenp, newp);
>  		break;