From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: sysctl net.inet.ip.directed-broadcast unlock
To: Alexander Bluhm <bluhm@openbsd.org>
Cc: tech@openbsd.org
Date: Tue, 6 Aug 2024 16:58:48 +0300

On Tue, Aug 06, 2024 at 03:52:23PM +0200, Alexander Bluhm wrote:
> Hi,
> 
> ip_directedbcast is either read in ip_input() or pf_test() during
> packet processing.  So writing sysctl net.inet.ip.directed-broadcast
> it does not need net lock.
> 
> ok?
> 

ok mvs

> bluhm
> 
> Index: net/pf.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf.c,v
> diff -u -p -r1.1203 pf.c
> --- net/pf.c	14 Jul 2024 18:53:39 -0000	1.1203
> +++ net/pf.c	6 Aug 2024 13:43:11 -0000
> @@ -7978,7 +7978,7 @@ done:
>  					action = PF_DROP;
>  					goto out;
>  				}
> -				if (ip_directedbcast)
> +				if (atomic_load_int(&ip_directedbcast))
>  					SET(flags, IP_ALLOWBROADCAST);
>  				ip_forward(pd.m, ifp, NULL, flags);
>  			} else
> Index: netinet/ip_input.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
> diff -u -p -r1.400 ip_input.c
> --- netinet/ip_input.c	19 Jul 2024 16:58:31 -0000	1.400
> +++ netinet/ip_input.c	6 Aug 2024 13:42:42 -0000
> @@ -99,7 +99,7 @@ int	ip_dosourceroute = 0;
>  int	ip_defttl = IPDEFTTL;
>  int	ip_mtudisc = 1;
>  int	ip_mtudisc_timeout = IPMTUDISCTIMEOUT;
> -int	ip_directedbcast = 0;
> +int	ip_directedbcast = 0;			/* [a] */
>  
>  /* Protects `ipq' and `ip_frags'. */
>  struct mutex	ipq_mutex = MUTEX_INITIALIZER(IPL_SOFTNET);
> @@ -114,6 +114,7 @@ int	ip_frags = 0;
>  const struct sysctl_bounded_args ipctl_vars_unlocked[] = {
>  	{ IPCTL_FORWARDING, &ip_forwarding, 0, 2 },
>  	{ IPCTL_SENDREDIRECTS, &ip_sendredirects, 0, 1 },
> +	{ IPCTL_DIRECTEDBCAST, &ip_directedbcast, 0, 1 },
>  };
>  
>  const struct sysctl_bounded_args ipctl_vars[] = {
> @@ -121,7 +122,6 @@ const struct sysctl_bounded_args ipctl_v
>  	{ IPCTL_MRTPROTO, &ip_mrtproto, SYSCTL_INT_READONLY },
>  #endif
>  	{ IPCTL_DEFTTL, &ip_defttl, 0, 255 },
> -	{ IPCTL_DIRECTEDBCAST, &ip_directedbcast, 0, 1 },
>  	{ IPCTL_IPPORT_FIRSTAUTO, &ipport_firstauto, 0, 65535 },
>  	{ IPCTL_IPPORT_LASTAUTO, &ipport_lastauto, 0, 65535 },
>  	{ IPCTL_IPPORT_HIFIRSTAUTO, &ipport_hifirstauto, 0, 65535 },
> @@ -483,7 +483,7 @@ ip_input_if(struct mbuf **mp, int *offp,
>  		SET(flags, IP_FORWARDING);
>  		break;
>  	}
> -	if (ip_directedbcast)
> +	if (atomic_load_int(&ip_directedbcast))
>  		SET(flags, IP_ALLOWBROADCAST);
>  
>  	hlen = ip->ip_hl << 2;
> @@ -1805,6 +1805,7 @@ ip_sysctl(int *name, u_int namelen, void
>  		return (error);
>  	case IPCTL_FORWARDING:
>  	case IPCTL_SENDREDIRECTS:
> +	case IPCTL_DIRECTEDBCAST:
>  		return (sysctl_bounded_arr(
>  		    ipctl_vars_unlocked, nitems(ipctl_vars_unlocked),
>  		    name, namelen, oldp, oldlenp, newp, newlen));
>