From: Alexander Bluhm Subject: Re: sysctl: unlock IPV6CTL_MAXFRAGPACKETS case of ip6_sysctl() To: Vitaliy Makkoveev Cc: tech@openbsd.org Date: Mon, 21 Jul 2025 11:29:38 +0200 On Fri, Jul 18, 2025 at 08:54:12PM +0300, Vitaliy Makkoveev wrote: > On Fri, Jul 18, 2025 at 03:19:41PM +0200, Alexander Bluhm wrote: > > On Fri, Jul 18, 2025 at 12:14:30PM +0300, Vitaliy Makkoveev wrote: > > > Use cached value in both frag6_input() and frag6_slowtimo(). > > > > Can you make ip6_maxfragpackets_local u_int and avoid the casts? > > > > otherwise OK bluhm@ > > > > The stack and the sysctl interface are inconsistent. It is assumed that > `ip6_maxfragpackets' could be negative, and we accept all fragments in > this case, but sysctl interface doesn't allow to set negative value to > `ip6_maxfragpackets'. The limitation in sysctl that forces ip6_maxfragpackets into 0 to 1000 boundaries is newer than this the ip6_maxfragpackets >= 0 check. > However, frag6_slowtimo() omits the "ip6_maxfragpackets >= 0" check, and > seems it doesn't accept such case, so I think we should adjust the > commentary in the frag6_input(). Accepting unlimited fragemnts is a bad idea anyway. So I am fine with removing this check. > Note, we don't need extra "ip6_maxfragpackets == 0" check because if > `ip6_maxfragpackets' is 0 the "frag6_nfragpackets >= ip6_maxfragpackets" > check will be true in any case. > > ok? OK bluhm@ > Index: sys/netinet6/frag6.c > =================================================================== > RCS file: /cvs/src/sys/netinet6/frag6.c,v > retrieving revision 1.93 > diff -u -p -r1.93 frag6.c > --- sys/netinet6/frag6.c 8 Jul 2025 00:47:41 -0000 1.93 > +++ sys/netinet6/frag6.c 18 Jul 2025 17:43:20 -0000 > @@ -196,11 +196,9 @@ frag6_input(struct mbuf **mp, int *offp, > * Enforce upper bound on number of fragmented packets > * for which we attempt reassembly; > * If maxfragpackets is 0, never accept fragments. > - * If maxfragpackets is -1, accept all fragments without > - * limitation. > */ > - if (ip6_maxfragpackets >= 0 && > - frag6_nfragpackets >= (u_int)ip6_maxfragpackets) { > + if (frag6_nfragpackets >= > + atomic_load_int(&ip6_maxfragpackets)) { > mtx_leave(&frag6_mutex); > goto dropfrag; > } > @@ -574,6 +572,7 @@ frag6_slowtimo(void) > { > struct ip6q_head rmq6; > struct ip6q *q6, *nq6; > + u_int ip6_maxfragpackets_local = atomic_load_int(&ip6_maxfragpackets); > > TAILQ_INIT(&rmq6); > > @@ -591,7 +590,7 @@ frag6_slowtimo(void) > * (due to the limit being lowered), drain off > * enough to get down to the new limit. > */ > - while (frag6_nfragpackets > (u_int)ip6_maxfragpackets && > + while (frag6_nfragpackets > ip6_maxfragpackets_local && > !TAILQ_EMPTY(&frag6_queue)) { > ip6stat_inc(ip6s_fragoverflow); > frag6_unlink(TAILQ_LAST(&frag6_queue, ip6q_head), &rmq6); > Index: sys/netinet6/in6_proto.c > =================================================================== > RCS file: /cvs/src/sys/netinet6/in6_proto.c,v > retrieving revision 1.136 > diff -u -p -r1.136 in6_proto.c > --- sys/netinet6/in6_proto.c 18 Jul 2025 08:39:14 -0000 1.136 > +++ sys/netinet6/in6_proto.c 18 Jul 2025 17:43:20 -0000 > @@ -354,7 +354,7 @@ int ip6_multipath = 0; /* [a] no using m > int ip6_sendredirects = 1; /* [a] */ > int ip6_defhlim = IPV6_DEFHLIM; /* [a] */ > int ip6_defmcasthlim = IPV6_DEFAULT_MULTICAST_HOPS; > -int ip6_maxfragpackets = 200; > +int ip6_maxfragpackets = 200; /* [a] */ > int ip6_maxfrags = 200; > int ip6_log_interval = 5; > int ip6_hdrnestlimit = 10; /* appropriate? */ > Index: sys/netinet6/ip6_input.c > =================================================================== > RCS file: /cvs/src/sys/netinet6/ip6_input.c,v > retrieving revision 1.280 > diff -u -p -r1.280 ip6_input.c > --- sys/netinet6/ip6_input.c 18 Jul 2025 08:39:14 -0000 1.280 > +++ sys/netinet6/ip6_input.c 18 Jul 2025 17:43:20 -0000 > @@ -1447,10 +1447,10 @@ const struct sysctl_bounded_args ipv6ctl > { IPV6CTL_MRTPROTO, &ip6_mrtproto, SYSCTL_INT_READONLY }, > #endif > { IPV6CTL_DEFHLIM, &ip6_defhlim, 0, 255 }, > + { IPV6CTL_MAXFRAGPACKETS, &ip6_maxfragpackets, 0, 1000 }, > }; > > const struct sysctl_bounded_args ipv6ctl_vars[] = { > - { IPV6CTL_MAXFRAGPACKETS, &ip6_maxfragpackets, 0, 1000 }, > { IPV6CTL_LOG_INTERVAL, &ip6_log_interval, 0, INT_MAX }, > { IPV6CTL_HDRNESTLIMIT, &ip6_hdrnestlimit, 0, 100 }, > { IPV6CTL_DAD_COUNT, &ip6_dad_count, 0, 10 }, > @@ -1570,6 +1570,7 @@ ip6_sysctl(int *name, u_int namelen, voi > case IPV6CTL_MRTPROTO: > #endif > case IPV6CTL_DEFHLIM: > + case IPV6CTL_MAXFRAGPACKETS: > return (sysctl_bounded_arr( > ipv6ctl_vars_unlocked, nitems(ipv6ctl_vars_unlocked), > name, namelen, oldp, oldlenp, newp, newlen));