From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Re: sysctl: unlock IPV6CTL_MAXFRAGPACKETS case of ip6_sysctl()
To: Alexander Bluhm <alexander.bluhm@gmx.net>
Cc: tech@openbsd.org
Date: Fri, 18 Jul 2025 20:54:12 +0300

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'.

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().

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?

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));