From: Alexander Bluhm Subject: Re: Unlock ip6_sysctl() To: Vitaliy Makkoveev Cc: tech@openbsd.org Date: Wed, 30 Jul 2025 22:12:40 +0200 On Sun, Jul 27, 2025 at 10:31:01PM +0300, Vitaliy Makkoveev wrote: > Use temporary local buffer for lockless IO within ip6_sysctl_soiikey(). > Use existing `sysctl_lock' rwlock(9) to protect `ip6_soiikey'. Also, > replace temporary `ip_sysctl_lock' with `sysctl_lock' rwlock(9). It was > introduced as temporary to avoid recursive lock acquisition in > ip*_sysctl() until upcoming unlocked. The usage path are not the hot > paths, no reason to have dedicated locks for them. > > I want to deny negative values for 'net.inet6.ip6.neighborgcthresh' and > 'net.inet6.ip6.maxdynroutes' with the following diffs. Negative values > mean unlimited count of in-kernel objects and this is definitely bad > idea. During ip6_sysctl() unlocking I found some sysctl() variables > already modified, but these were forgotten. > > const struct sysctl_bounded_args ipv6ctl_vars[] = { > /* ... */ > { IPV6CTL_NEIGHBORGCTHRESH, &ip6_neighborgcthresh, -1, 5 * 2048}, > { IPV6CTL_MAXDYNROUTES, &ip6_maxdynroutes, -1, 5 * 4096}, > }; OK bluhm@ > Index: sys/netinet/ip_input.c > =================================================================== > RCS file: /cvs/src/sys/netinet/ip_input.c,v > retrieving revision 1.424 > diff -u -p -r1.424 ip_input.c > --- sys/netinet/ip_input.c 24 Jul 2025 22:31:19 -0000 1.424 > +++ sys/netinet/ip_input.c 27 Jul 2025 18:58:30 -0000 > @@ -1723,9 +1723,6 @@ ip_forward(struct mbuf *m, struct ifnet > > #ifndef SMALL_KERNEL > > -/* Temporary, to avoid sysctl_lock recursion. */ > -struct rwlock ip_sysctl_lock = RWLOCK_INITIALIZER("ipslk"); > - > int > ip_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp, > size_t newlen) > @@ -1759,10 +1756,10 @@ ip_sysctl(int *name, u_int namelen, void > error = sysctl_int_bounded(oldp, oldlenp, newp, newlen, > &newval, 0, INT_MAX); > if (error == 0 && oldval != newval) { > - rw_enter_write(&ip_sysctl_lock); > + rw_enter_write(&sysctl_lock); > atomic_store_int(&ip_mtudisc_timeout, newval); > rt_timer_queue_change(&ip_mtudisc_timeout_q, newval); > - rw_exit_write(&ip_sysctl_lock); > + rw_exit_write(&sysctl_lock); > } > > return (error); > Index: sys/netinet/ip_var.h > =================================================================== > RCS file: /cvs/src/sys/netinet/ip_var.h,v > retrieving revision 1.122 > diff -u -p -r1.122 ip_var.h > --- sys/netinet/ip_var.h 21 Jun 2025 14:21:17 -0000 1.122 > +++ sys/netinet/ip_var.h 27 Jul 2025 18:58:30 -0000 > @@ -285,9 +285,5 @@ int rip_send(struct socket *, struct mb > extern struct socket *ip_mrouter[]; /* multicast routing daemon */ > #endif > > -#ifndef SMALL_KERNEL > -extern struct rwlock ip_sysctl_lock; > -#endif > - > #endif /* _KERNEL */ > #endif /* _NETINET_IP_VAR_H_ */ > Index: sys/netinet6/in6_proto.c > =================================================================== > RCS file: /cvs/src/sys/netinet6/in6_proto.c,v > retrieving revision 1.148 > diff -u -p -r1.148 in6_proto.c > --- sys/netinet6/in6_proto.c 27 Jul 2025 17:46:58 -0000 1.148 > +++ sys/netinet6/in6_proto.c 27 Jul 2025 18:58:30 -0000 > @@ -120,6 +120,7 @@ const struct protosw inet6sw[] = { > { > .pr_domain = &inet6domain, > .pr_protocol = IPPROTO_IPV6, > + .pr_flags = PR_MPSYSCTL, > .pr_init = ip6_init, > .pr_slowtimo = frag6_slowtimo, > .pr_sysctl = ip6_sysctl > Index: sys/netinet6/ip6_input.c > =================================================================== > RCS file: /cvs/src/sys/netinet6/ip6_input.c,v > retrieving revision 1.293 > diff -u -p -r1.293 ip6_input.c > --- sys/netinet6/ip6_input.c 27 Jul 2025 17:46:58 -0000 1.293 > +++ sys/netinet6/ip6_input.c 27 Jul 2025 18:58:30 -0000 > @@ -1484,14 +1484,27 @@ ip6_sysctl_ip6stat(void *oldp, size_t *o > int > ip6_sysctl_soiikey(void *oldp, size_t *oldlenp, void *newp, size_t newlen) > { > + uint8_t soiikey[sizeof(ip6_soiikey)]; > int error; > > error = suser(curproc); > if (error != 0) > return (error); > > - return (sysctl_struct(oldp, oldlenp, newp, newlen, ip6_soiikey, > - sizeof(ip6_soiikey))); > + rw_enter_read(&sysctl_lock); > + memcpy(soiikey, ip6_soiikey, sizeof(ip6_soiikey)); > + rw_exit_read(&sysctl_lock); > + > + error = sysctl_struct(oldp, oldlenp, newp, newlen, soiikey, > + sizeof(soiikey)); > + > + if (error == 0 && newp) { > + rw_enter_write(&sysctl_lock); > + memcpy(ip6_soiikey, soiikey, sizeof(soiikey)); > + rw_exit_write(&sysctl_lock); > + } > + > + return (error); > } > > int > @@ -1533,10 +1546,10 @@ ip6_sysctl(int *name, u_int namelen, voi > error = sysctl_int_bounded(oldp, oldlenp, newp, newlen, > &newval, 0, INT_MAX); > if (error == 0 && oldval != newval) { > - rw_enter_write(&ip_sysctl_lock); > + rw_enter_write(&sysctl_lock); > atomic_store_int(&ip6_mtudisc_timeout, newval); > rt_timer_queue_change(&icmp6_mtudisc_timeout_q, newval); > - rw_exit_write(&ip_sysctl_lock); > + rw_exit_write(&sysctl_lock); > } > > return (error); > Index: sys/sys/sysctl.h > =================================================================== > RCS file: /cvs/src/sys/sys/sysctl.h,v > retrieving revision 1.245 > diff -u -p -r1.245 sysctl.h > --- sys/sys/sysctl.h 24 Jul 2025 19:42:41 -0000 1.245 > +++ sys/sys/sysctl.h 27 Jul 2025 18:58:30 -0000 > @@ -1030,6 +1030,8 @@ struct sysctl_bounded_args { > */ > typedef int (sysctlfn)(int *, u_int, void *, size_t *, void *, size_t, struct proc *); > > +extern struct rwlock sysctl_lock; > + > int sysctl_vslock(void *, size_t); > void sysctl_vsunlock(void *, size_t); >