Index | Thread | Search

From:
Vitaliy Makkoveev <mvs@openbsd.org>
Subject:
Re: sysctl sendredirects unlock
To:
Alexander Bluhm <bluhm@openbsd.org>
Cc:
tech@openbsd.org
Date:
Fri, 19 Jul 2024 18:29:36 +0300

Download raw body.

Thread
On Thu, Jul 18, 2024 at 11:12:08AM +0200, Alexander Bluhm wrote:
> Hi,
> 
> Unlock sysctl net.inet.ip.redirect and net.inet6.ip6.redirect.
> Variable ip and ip6 sendredirects is only read once during packet
> processing.  Use atomic_load_int() to access the value in exaclty
> one read instruction.  No memory barriers needed as there is no
> correlation with other values.
> 
> While there, sort the ip and ip6 checks, so the difference is easier
> to see.  Move access to global variable to the end.
> 
> ok?
> 

ok mvs

> bluhm
> 
> Index: netinet/ip_input.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/ip_input.c,v
> diff -u -p -r1.399 ip_input.c
> --- netinet/ip_input.c	14 Jul 2024 18:53:39 -0000	1.399
> +++ netinet/ip_input.c	18 Jul 2024 08:08:45 -0000
> @@ -94,7 +94,7 @@
>  int	ip_forwarding = 0;			/* [a] */
>  int	ipmforwarding = 0;
>  int	ipmultipath = 0;
> -int	ip_sendredirects = 1;
> +int	ip_sendredirects = 1;			/* [a] */
>  int	ip_dosourceroute = 0;
>  int	ip_defttl = IPDEFTTL;
>  int	ip_mtudisc = 1;
> @@ -113,13 +113,13 @@ int	ip_frags = 0;
>  
>  const struct sysctl_bounded_args ipctl_vars_unlocked[] = {
>  	{ IPCTL_FORWARDING, &ip_forwarding, 0, 2 },
> +	{ IPCTL_SENDREDIRECTS, &ip_sendredirects, 0, 1 },
>  };
>  
>  const struct sysctl_bounded_args ipctl_vars[] = {
>  #ifdef MROUTING
>  	{ IPCTL_MRTPROTO, &ip_mrtproto, SYSCTL_INT_READONLY },
>  #endif
> -	{ IPCTL_SENDREDIRECTS, &ip_sendredirects, 0, 1 },
>  	{ IPCTL_DEFTTL, &ip_defttl, 0, 255 },
>  	{ IPCTL_DIRECTEDBCAST, &ip_directedbcast, 0, 1 },
>  	{ IPCTL_IPPORT_FIRSTAUTO, &ipport_firstauto, 0, 65535 },
> @@ -1605,10 +1605,11 @@ ip_forward(struct mbuf *m, struct ifnet 
>  	 * Don't send redirect if we advertise destination's arp address
>  	 * as ours (proxy arp).
>  	 */
> -	if ((rt->rt_ifidx == ifp->if_index) &&
> -	    (rt->rt_flags & (RTF_DYNAMIC|RTF_MODIFIED)) == 0 &&
> -	    satosin(rt_key(rt))->sin_addr.s_addr != 0 &&
> -	    ip_sendredirects && !ISSET(flags, IP_REDIRECT) &&
> +	if (rt->rt_ifidx == ifp->if_index &&
> +	    !ISSET(rt->rt_flags, RTF_DYNAMIC|RTF_MODIFIED) &&
> +	    satosin(rt_key(rt))->sin_addr.s_addr != INADDR_ANY &&
> +	    !ISSET(flags, IP_REDIRECT) &&
> +	    atomic_load_int(&ip_sendredirects) &&
>  	    !arpproxy(satosin(rt_key(rt))->sin_addr, rtableid)) {
>  		if ((ip->ip_src.s_addr & ifatoia(rt->rt_ifa)->ia_netmask) ==
>  		    ifatoia(rt->rt_ifa)->ia_net) {
> @@ -1803,6 +1804,7 @@ ip_sysctl(int *name, u_int namelen, void
>  		NET_UNLOCK();
>  		return (error);
>  	case IPCTL_FORWARDING:
> +	case IPCTL_SENDREDIRECTS:
>  		return (sysctl_bounded_arr(
>  		    ipctl_vars_unlocked, nitems(ipctl_vars_unlocked),
>  		    name, namelen, oldp, oldlenp, newp, newlen));
> Index: netinet6/in6_proto.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/in6_proto.c,v
> diff -u -p -r1.115 in6_proto.c
> --- netinet6/in6_proto.c	12 Jul 2024 19:50:35 -0000	1.115
> +++ netinet6/in6_proto.c	18 Jul 2024 08:08:45 -0000
> @@ -343,10 +343,10 @@ const struct domain inet6domain = {
>  /*
>   * Internet configuration info
>   */
> -int	ip6_forwarding = 0;	/* no forwarding unless sysctl'd to enable */
> +int	ip6_forwarding = 0;	/* [a] no forwarding unless sysctl to enable */
>  int	ip6_mforwarding = 0;	/* no multicast forwarding unless ... */
>  int	ip6_multipath = 0;	/* no using multipath routes unless ... */
> -int	ip6_sendredirects = 1;
> +int	ip6_sendredirects = 1;	/* [a] */
>  int	ip6_defhlim = IPV6_DEFHLIM;
>  int	ip6_defmcasthlim = IPV6_DEFAULT_MULTICAST_HOPS;
>  int	ip6_maxfragpackets = 200;
> Index: netinet6/ip6_forward.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_forward.c,v
> diff -u -p -r1.123 ip6_forward.c
> --- netinet6/ip6_forward.c	13 Jul 2024 10:09:40 -0000	1.123
> +++ netinet6/ip6_forward.c	18 Jul 2024 08:08:45 -0000
> @@ -280,8 +280,9 @@ reroute:
>  		goto freecopy;
>  	}
>  	if (rt->rt_ifidx == ifidx &&
> -	    ip6_sendredirects && !ISSET(flags, IPV6_REDIRECT) &&
> -	    (rt->rt_flags & (RTF_DYNAMIC|RTF_MODIFIED)) == 0) {
> +	    !ISSET(rt->rt_flags, RTF_DYNAMIC|RTF_MODIFIED) &&
> +	    !ISSET(flags, IPV6_REDIRECT) &&
> +	    atomic_load_int(&ip6_sendredirects)) {
>  		if ((ifp->if_flags & IFF_POINTOPOINT) &&
>  		    nd6_is_addr_neighbor(&ro->ro_dstsin6, ifp)) {
>  			/*
> Index: netinet6/ip6_input.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/ip6_input.c,v
> diff -u -p -r1.265 ip6_input.c
> --- netinet6/ip6_input.c	14 Jul 2024 18:53:39 -0000	1.265
> +++ netinet6/ip6_input.c	18 Jul 2024 08:08:45 -0000
> @@ -1445,6 +1445,7 @@ extern int ip6_mrtproto;
>  
>  const struct sysctl_bounded_args ipv6ctl_vars_unlocked[] = {
>  	{ IPV6CTL_FORWARDING, &ip6_forwarding, 0, 2 },
> +	{ IPV6CTL_SENDREDIRECTS, &ip6_sendredirects, 0, 1 },
>  };
>  
>  const struct sysctl_bounded_args ipv6ctl_vars[] = {
> @@ -1452,7 +1453,6 @@ const struct sysctl_bounded_args ipv6ctl
>  #ifdef MROUTING
>  	{ IPV6CTL_MRTPROTO, &ip6_mrtproto, SYSCTL_INT_READONLY },
>  #endif
> -	{ IPV6CTL_SENDREDIRECTS, &ip6_sendredirects, 0, 1 },
>  	{ IPV6CTL_DEFHLIM, &ip6_defhlim, 0, 255 },
>  	{ IPV6CTL_MAXFRAGPACKETS, &ip6_maxfragpackets, 0, 1000 },
>  	{ IPV6CTL_LOG_INTERVAL, &ip6_log_interval, 0, INT_MAX },
> @@ -1572,6 +1572,7 @@ ip6_sysctl(int *name, u_int namelen, voi
>  		NET_UNLOCK();
>  		return (error);
>  	case IPV6CTL_FORWARDING:
> +	case IPV6CTL_SENDREDIRECTS:
>  		return (sysctl_bounded_arr(
>  		    ipv6ctl_vars_unlocked, nitems(ipv6ctl_vars_unlocked),
>  		    name, namelen, oldp, oldlenp, newp, newlen));
>