From: Vitaliy Makkoveev Subject: Re: sysctl sendredirects unlock To: Alexander Bluhm Cc: tech@openbsd.org Date: Fri, 19 Jul 2024 18:29:36 +0300 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)); >