From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: sysctl: unlock IPCTL_IPPORT_* cases of ip_sysctl()
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Tue, 24 Jun 2025 18:01:29 +0200

On Tue, Jun 24, 2025 at 06:16:26PM +0300, Vitaliy Makkoveev wrote:
> Read-only accessed together in in_pcbpickport().

OK bluhm@

> Index: sys/netinet/in_pcb.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/in_pcb.c,v
> diff -u -p -r1.316 in_pcb.c
> --- sys/netinet/in_pcb.c	16 Jun 2025 07:11:58 -0000	1.316
> +++ sys/netinet/in_pcb.c	24 Jun 2025 15:08:53 -0000
> @@ -101,6 +101,11 @@
>  #include <net/toeplitz.h>
>  #endif
>  
> +/*
> + * Locks used to protect data:
> + *	a	atomic
> + */
> +
>  const struct in_addr zeroin_addr;
>  const union inpaddru zeroin46_addr;
>  
> @@ -108,10 +113,10 @@ const union inpaddru zeroin46_addr;
>   * These configure the range of local port addresses assigned to
>   * "unspecified" outgoing connections/packets/whatever.
>   */
> -int ipport_firstauto = IPPORT_RESERVED;
> -int ipport_lastauto = IPPORT_USERRESERVED;
> -int ipport_hifirstauto = IPPORT_HIFIRSTAUTO;
> -int ipport_hilastauto = IPPORT_HILASTAUTO;
> +int ipport_firstauto = IPPORT_RESERVED;		/* [a] */
> +int ipport_lastauto = IPPORT_USERRESERVED;	/* [a] */
> +int ipport_hifirstauto = IPPORT_HIFIRSTAUTO;	/* [a] */
> +int ipport_hilastauto = IPPORT_HILASTAUTO;	/* [a] */
>  
>  struct baddynamicports baddynamicports;
>  struct baddynamicports rootonlyports;
> @@ -450,16 +455,16 @@ in_pcbpickport(u_int16_t *lport, const v
>  	MUTEX_ASSERT_LOCKED(&table->inpt_mtx);
>  
>  	if (inp->inp_flags & INP_HIGHPORT) {
> -		first = ipport_hifirstauto;	/* sysctl */
> -		last = ipport_hilastauto;
> +		first = atomic_load_int(&ipport_hifirstauto);	/* sysctl */
> +		last = atomic_load_int(&ipport_hilastauto);
>  	} else if (inp->inp_flags & INP_LOWPORT) {
>  		if (suser(p))
>  			return (EACCES);
>  		first = IPPORT_RESERVED-1; /* 1023 */
>  		last = 600;		   /* not IPPORT_RESERVED/2 */
>  	} else {
> -		first = ipport_firstauto;	/* sysctl */
> -		last = ipport_lastauto;
> +		first = atomic_load_int(&ipport_firstauto);	/* sysctl */
> +		last = atomic_load_int(&ipport_lastauto);
>  	}
>  	if (first < last) {
>  		lower = first;
> Index: sys/netinet/ip_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_input.c,v
> diff -u -p -r1.413 ip_input.c
> --- sys/netinet/ip_input.c	23 Jun 2025 20:59:25 -0000	1.413
> +++ sys/netinet/ip_input.c	24 Jun 2025 15:08:53 -0000
> @@ -119,14 +119,14 @@ const struct sysctl_bounded_args ipctl_v
>  #ifdef MROUTING
>  	{ IPCTL_MRTPROTO, &ip_mrtproto, SYSCTL_INT_READONLY },
>  #endif
> -};
> -
> -const struct sysctl_bounded_args ipctl_vars[] = {
> -	{ IPCTL_DEFTTL, &ip_defttl, 0, 255 },
>  	{ IPCTL_IPPORT_FIRSTAUTO, &ipport_firstauto, 0, 65535 },
>  	{ IPCTL_IPPORT_LASTAUTO, &ipport_lastauto, 0, 65535 },
>  	{ IPCTL_IPPORT_HIFIRSTAUTO, &ipport_hifirstauto, 0, 65535 },
>  	{ IPCTL_IPPORT_HILASTAUTO, &ipport_hilastauto, 0, 65535 },
> +};
> +
> +const struct sysctl_bounded_args ipctl_vars[] = {
> +	{ IPCTL_DEFTTL, &ip_defttl, 0, 255 },
>  	{ IPCTL_IPPORT_MAXQUEUE, &ip_maxqueue, 0, 10000 },
>  	{ IPCTL_MFORWARDING, &ipmforwarding, 0, 1 },
>  	{ IPCTL_ARPTIMEOUT, &arpt_keep, 0, INT_MAX },
> @@ -1839,6 +1839,10 @@ ip_sysctl(int *name, u_int namelen, void
>  #ifdef MROUTING
>  	case IPCTL_MRTPROTO:
>  #endif
> +	case IPCTL_IPPORT_FIRSTAUTO:
> +	case IPCTL_IPPORT_LASTAUTO:
> +	case IPCTL_IPPORT_HIFIRSTAUTO:
> +	case IPCTL_IPPORT_HILASTAUTO:
>  		return (sysctl_bounded_arr(
>  		    ipctl_vars_unlocked, nitems(ipctl_vars_unlocked),
>  		    name, namelen, oldp, oldlenp, newp, newlen));