Index | Thread | Search

From:
Alexander Bluhm <bluhm@openbsd.org>
Subject:
Re: sysctl: relax netlock around ESPCTL_UDPENCAP_ENABLE and ESPCTL_UDPENCAP_PORT
To:
Vitaliy Makkoveev <mvs@openbsd.org>
Cc:
tech@openbsd.org
Date:
Sat, 14 Jun 2025 21:11:19 +0200

Download raw body.

Thread
On Sat, Jun 14, 2025 at 07:23:31PM +0300, Vitaliy Makkoveev wrote:
> Updated to use atomic_load_int(). `udpencap_enable' and `udpencap_port'
> mostly used together, so unlock them both.

OK bluhm@

> Index: sys/net/pfkeyv2.c
> ===================================================================
> RCS file: /cvs/src/sys/net/pfkeyv2.c,v
> retrieving revision 1.268
> diff -u -p -r1.268 pfkeyv2.c
> --- sys/net/pfkeyv2.c	3 Jun 2025 14:49:05 -0000	1.268
> +++ sys/net/pfkeyv2.c	14 Jun 2025 15:37:52 -0000
> @@ -1287,7 +1287,7 @@ pfkeyv2_dosend(struct socket *so, void *
>  #ifdef IPSEC
>  		/* UDP encap has to be enabled and is only supported for ESP */
>  		if (headers[SADB_X_EXT_UDPENCAP] &&
> -		    (!udpencap_enable ||
> +		    (!atomic_load_int(&udpencap_enable) ||
>  		    smsg->sadb_msg_satype != SADB_SATYPE_ESP)) {
>  			rval = EINVAL;
>  			goto ret;
> @@ -1462,7 +1462,7 @@ pfkeyv2_dosend(struct socket *so, void *
>  #ifdef IPSEC
>  		/* UDP encap has to be enabled and is only supported for ESP */
>  		if (headers[SADB_X_EXT_UDPENCAP] &&
> -		    (!udpencap_enable ||
> +		    (!atomic_load_int(&udpencap_enable) ||
>  		    smsg->sadb_msg_satype != SADB_SATYPE_ESP)) {
>  			rval = EINVAL;
>  			goto ret;
> Index: sys/netinet/in_pcb.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in_pcb.c,v
> retrieving revision 1.315
> diff -u -p -r1.315 in_pcb.c
> --- sys/netinet/in_pcb.c	3 Jun 2025 16:51:26 -0000	1.315
> +++ sys/netinet/in_pcb.c	14 Jun 2025 15:37:52 -0000
> @@ -202,7 +202,7 @@ in_baddynamic(u_int16_t port, u_int16_t 
>  	case IPPROTO_UDP:
>  #ifdef IPSEC
>  		/* Cannot preset this as it is a sysctl */
> -		if (port == udpencap_port)
> +		if (port == atomic_load_int(&udpencap_port))
>  			return (1);
>  #endif
>  		return (DP_ISSET(baddynamicports.udp, port));
> Index: sys/netinet/in_proto.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in_proto.c,v
> retrieving revision 1.124
> diff -u -p -r1.124 in_proto.c
> --- sys/netinet/in_proto.c	12 Jun 2025 20:37:59 -0000	1.124
> +++ sys/netinet/in_proto.c	14 Jun 2025 15:37:52 -0000
> @@ -310,7 +310,7 @@ const struct protosw inetsw[] = {
>    .pr_type	= SOCK_RAW,
>    .pr_domain	= &inetdomain,
>    .pr_protocol	= IPPROTO_ESP,
> -  .pr_flags	= PR_ATOMIC|PR_ADDR,
> +  .pr_flags	= PR_ATOMIC|PR_ADDR|PR_MPSYSCTL,
>    .pr_input	= esp46_input,
>    .pr_ctlinput	= esp4_ctlinput,
>    .pr_ctloutput	= rip_ctloutput,
> Index: sys/netinet/ipsec_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ipsec_input.c,v
> retrieving revision 1.217
> diff -u -p -r1.217 ipsec_input.c
> --- sys/netinet/ipsec_input.c	3 Jun 2025 14:49:05 -0000	1.217
> +++ sys/netinet/ipsec_input.c	14 Jun 2025 15:37:52 -0000
> @@ -124,12 +124,10 @@ 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},
>  };
> +
>  const struct sysctl_bounded_args ahctl_vars[] = {
>  	{AHCTL_ENABLE, &ah_enable, 0, 1},
>  };
> @@ -718,8 +716,6 @@ int
>  esp_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
>      size_t newlen)
>  {
> -	int error;
> -
>  	/* All sysctl names at this level are terminal. */
>  	if (namelen != 1)
>  		return (ENOTDIR);
> @@ -727,16 +723,9 @@ 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_locked,
> -		    nitems(espctl_vars_locked),
> -		    name, namelen, oldp, oldlenp, newp, newlen);
> -		NET_UNLOCK();
> -		return (error);
> +		return (sysctl_bounded_arr(espctl_vars, nitems(espctl_vars),
> +		    name, namelen, oldp, oldlenp, newp, newlen));
>  	}
>  }
>  
> Index: sys/netinet/ipsec_output.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ipsec_output.c,v
> retrieving revision 1.102
> diff -u -p -r1.102 ipsec_output.c
> --- sys/netinet/ipsec_output.c	3 Jun 2025 14:49:05 -0000	1.102
> +++ sys/netinet/ipsec_output.c	14 Jun 2025 15:37:52 -0000
> @@ -51,6 +51,11 @@
>  #include <crypto/cryptodev.h>
>  #include <crypto/xform.h>
>  
> +/*
> + * Locks used to protect data:
> + *	a	atomic
> + */
> +
>  #ifdef ENCDEBUG
>  #define DPRINTF(fmt, args...)						\
>  	do {								\
> @@ -62,8 +67,8 @@
>  	do { } while (0)
>  #endif
>  
> -int	udpencap_enable = 1;	/* enabled by default */
> -int	udpencap_port = 4500;	/* triggers decapsulation */
> +int	udpencap_enable = 1;	/* [a] enabled by default */
> +int	udpencap_port = 4500;	/* [a] triggers decapsulation */
>  
>  /*
>   * Loop over a tdb chain, taking into consideration protocol tunneling. The
> @@ -417,8 +422,10 @@ ipsp_process_done(struct mbuf *m, struct
>  		struct mbuf *mi;
>  		struct udphdr *uh;
>  		int iphlen;
> +		int udpencap_port_local = atomic_load_int(&udpencap_port);
>  
> -		if (!udpencap_enable || !udpencap_port) {
> +		if (!atomic_load_int(&udpencap_enable) ||
> +		    !udpencap_port_local) {
>  			error = ENXIO;
>  			goto drop;
>  		}
> @@ -445,7 +452,7 @@ ipsp_process_done(struct mbuf *m, struct
>  			goto drop;
>  		}
>  		uh = (struct udphdr *)(mtod(mi, caddr_t) + roff);
> -		uh->uh_sport = uh->uh_dport = htons(udpencap_port);
> +		uh->uh_sport = uh->uh_dport = htons(udpencap_port_local);
>  		if (tdb->tdb_udpencap_port)
>  			uh->uh_dport = tdb->tdb_udpencap_port;
>  
> Index: sys/netinet/udp_usrreq.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/udp_usrreq.c,v
> retrieving revision 1.342
> diff -u -p -r1.342 udp_usrreq.c
> --- sys/netinet/udp_usrreq.c	12 Jun 2025 20:37:59 -0000	1.342
> +++ sys/netinet/udp_usrreq.c	14 Jun 2025 15:37:52 -0000
> @@ -212,6 +212,9 @@ udp_input(struct mbuf **mp, int *offp, i
>  	} srcsa, dstsa;
>  	struct ip6_hdr *ip6 = NULL;
>  	u_int32_t ipsecflowinfo = 0;
> +#ifdef IPSEC
> +	int udpencap_port_local = atomic_load_int(&udpencap_port);
> +#endif /* IPSEC */
>  
>  	udpstat_inc(udps_ipackets);
>  
> @@ -305,11 +308,12 @@ 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 && atomic_load_int(&esp_enable) &&
> +	if (atomic_load_int(&udpencap_enable) && udpencap_port_local &&
> +	    atomic_load_int(&esp_enable) &&
>  #if NPF > 0
>  	    !(m->m_pkthdr.pf.flags & PF_TAG_DIVERTED) &&
>  #endif
> -	    uh->uh_dport == htons(udpencap_port)) {
> +	    uh->uh_dport == htons(udpencap_port_local)) {
>  		u_int32_t spi;
>  		int skip = iphlen + sizeof(struct udphdr);
>  
> @@ -909,12 +913,16 @@ udp_ctlinput(int cmd, struct sockaddr *s
>  	if (ip) {
>  		struct inpcb *inp;
>  		struct socket *so = NULL;
> +#ifdef IPSEC
> +		int udpencap_port_local = atomic_load_int(&udpencap_port);
> +#endif
>  
>  		uhp = (struct udphdr *)((caddr_t)ip + (ip->ip_hl << 2));
>  #ifdef IPSEC
>  		/* PMTU discovery for udpencap */
> -		if (cmd == PRC_MSGSIZE && ip_mtudisc && udpencap_enable &&
> -		    udpencap_port && uhp->uh_sport == htons(udpencap_port)) {
> +		if (cmd == PRC_MSGSIZE && ip_mtudisc &&
> +		    atomic_load_int(&udpencap_enable) && udpencap_port_local &&
> +		    uhp->uh_sport == udpencap_port_local) {
>  			udpencap_ctlinput(cmd, sa, rdomain, v);
>  			return;
>  		}
> Index: sys/netinet6/in6_proto.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6_proto.c,v
> retrieving revision 1.128
> diff -u -p -r1.128 in6_proto.c
> --- sys/netinet6/in6_proto.c	12 Jun 2025 20:37:59 -0000	1.128
> +++ sys/netinet6/in6_proto.c	14 Jun 2025 15:37:52 -0000
> @@ -223,7 +223,7 @@ const struct protosw inet6sw[] = {
>    .pr_type	= SOCK_RAW,
>    .pr_domain	= &inet6domain,
>    .pr_protocol	= IPPROTO_ESP,
> -  .pr_flags	= PR_ATOMIC|PR_ADDR,
> +  .pr_flags	= PR_ATOMIC|PR_ADDR|PR_MPSYSCTL,
>    .pr_input	= esp46_input,
>    .pr_ctloutput	= rip6_ctloutput,
>    .pr_usrreqs	= &rip6_usrreqs,
>