From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: esp, sysctl: move `esp_enable' out of netlock
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Thu, 22 May 2025 23:28:51 +0900

On Wed, May 14, 2025 at 09:19:56PM +0300, Vitaliy Makkoveev wrote:
> As usual, atomically accessed integer. The `espctl_vars' are more
> complicated than already unlocked `ah_ctlvars' and `ipcomp_ctlvars',
> so unlock them step-by-step.

All esp_enable loads are in independent packet paths.

OK bluhm@

> Index: sys/net/pfkeyv2.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.267
> diff -u -p -r1.267 pfkeyv2.c
> --- sys/net/pfkeyv2.c	13 May 2025 09:16:33 -0000	1.267
> +++ sys/net/pfkeyv2.c	14 May 2025 18:12:09 -0000
> @@ -1068,7 +1068,7 @@ pfkeyv2_get_proto_alg(u_int8_t satype, u
>  		break;
>  
>  	case SADB_SATYPE_ESP:
> -		if (!esp_enable)
> +		if (!atomic_load_int(&esp_enable))
>  			return (EOPNOTSUPP);
>  
>  		*sproto = IPPROTO_ESP;
> Index: sys/netinet/ipsec_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ipsec_input.c,v
> retrieving revision 1.215
> diff -u -p -r1.215 ipsec_input.c
> --- sys/netinet/ipsec_input.c	14 May 2025 14:32:15 -0000	1.215
> +++ sys/netinet/ipsec_input.c	14 May 2025 18:12:09 -0000
> @@ -118,12 +118,15 @@ int ipsec_soft_first_use = IPSEC_DEFAULT
>  int ipsec_exp_first_use = IPSEC_DEFAULT_EXP_FIRST_USE;		/* [a] */
>  int ipsec_expire_acquire = IPSEC_DEFAULT_EXPIRE_ACQUIRE;	/* [a] */
>  
> -int esp_enable = 1;
> +int esp_enable = 1;		/* [a] */
>  int ah_enable = 1;		/* [a] */
>  int ipcomp_enable = 0;		/* [a] */
>  
>  const struct sysctl_bounded_args espctl_vars[] = {
>  	{ESPCTL_ENABLE, &esp_enable, 0, 1},
> +};
> +
> +const struct sysctl_bounded_args espctl_vars_locked[] = {
>  	{ESPCTL_UDPENCAP_ENABLE, &udpencap_enable, 0, 1},
>  	{ESPCTL_UDPENCAP_PORT, &udpencap_port, 0, 65535},
>  };
> @@ -724,9 +727,13 @@ esp_sysctl(int *name, u_int namelen, voi
>  	switch (name[0]) {
>  	case ESPCTL_STATS:
>  		return (esp_sysctl_espstat(oldp, oldlenp, newp));
> +	case ESPCTL_ENABLE:
> +		error = sysctl_bounded_arr(espctl_vars, nitems(espctl_vars),
> +		    name, namelen, oldp, oldlenp, newp, newlen);
>  	default:
>  		NET_LOCK();
> -		error = sysctl_bounded_arr(espctl_vars, nitems(espctl_vars),
> +		error = sysctl_bounded_arr(espctl_vars_locked,
> +		    nitems(espctl_vars_locked),
>  		    name, namelen, oldp, oldlenp, newp, newlen);
>  		NET_UNLOCK();
>  		return (error);
> @@ -876,7 +883,7 @@ esp46_input(struct mbuf **mp, int *offp,
>  #if NPF > 0
>  	    ((*mp)->m_pkthdr.pf.flags & PF_TAG_DIVERTED) ||
>  #endif
> -	    !esp_enable)
> +	    !atomic_load_int(&esp_enable))
>  		return ipsec_input_disabled(mp, offp, proto, af, ns);
>  
>  	protoff = ipsec_protoff(*mp, *offp, af);
> Index: sys/netinet/ipsec_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ipsec_output.c,v
> retrieving revision 1.101
> diff -u -p -r1.101 ipsec_output.c
> --- sys/netinet/ipsec_output.c	14 May 2025 14:32:15 -0000	1.101
> +++ sys/netinet/ipsec_output.c	14 May 2025 18:12:09 -0000
> @@ -91,7 +91,7 @@ ipsp_process_packet(struct mbuf *m, stru
>  #endif
>  
>  	/* Check that the transform is allowed by the administrator. */
> -	if ((tdb->tdb_sproto == IPPROTO_ESP && !esp_enable) ||
> +	if ((tdb->tdb_sproto == IPPROTO_ESP && !atomic_load_int(&esp_enable)) ||
>  	    (tdb->tdb_sproto == IPPROTO_AH && !atomic_load_int(&ah_enable)) ||
>  	    (tdb->tdb_sproto == IPPROTO_IPCOMP &&
>  	    !atomic_load_int(&ipcomp_enable))) {
> Index: sys/netinet/udp_usrreq.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
> retrieving revision 1.337
> diff -u -p -r1.337 udp_usrreq.c
> --- sys/netinet/udp_usrreq.c	12 May 2025 17:21:21 -0000	1.337
> +++ sys/netinet/udp_usrreq.c	14 May 2025 18:12:09 -0000
> @@ -303,7 +303,7 @@ udp_input(struct mbuf **mp, int *offp, i
>  	CLR(m->m_pkthdr.csum_flags, M_UDP_CSUM_OUT);
>  
>  #ifdef IPSEC
> -	if (udpencap_enable && udpencap_port && esp_enable &&
> +	if (udpencap_enable && udpencap_port && atomic_load_int(&esp_enable) &&
>  #if NPF > 0
>  	    !(m->m_pkthdr.pf.flags & PF_TAG_DIVERTED) &&
>  #endif