From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: Unlock IPV6CTL_MAXFRAGS case of ip6_sysctl()
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Fri, 25 Jul 2025 00:42:46 +0200

On Fri, Jul 25, 2025 at 01:19:32AM +0300, Vitaliy Makkoveev wrote:
> The frag6_input() assumes that `ip6_maxfrags' could be negative and
> accept all the fragments without the limit in this case, meanwhile the
> sysctl(2) interface denies negative values for `ip6_maxfrags'. Unlimited
> queue is bad thing, so adjust the frag6_input() to be consistent with
> sysctl(2). Don't check `ip6_maxfrags' against null because in such case
> the "frag6_nfrags >= ip6_maxfrags" condition is always true. Unused in
> ramdisk.

OK bluhm@

> Index: sys/netinet6/frag6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/frag6.c,v
> retrieving revision 1.94
> diff -u -p -r1.94 frag6.c
> --- sys/netinet6/frag6.c	21 Jul 2025 11:07:31 -0000	1.94
> +++ sys/netinet6/frag6.c	24 Jul 2025 22:18:01 -0000
> @@ -46,12 +46,17 @@
>  #include <netinet/icmp6.h>
>  #include <netinet/ip.h>		/* for ECN definitions */
>  
> -/* Protects `frag6_queue', `frag6_nfragpackets' and `frag6_nfrags'. */
> +
> +/*
> + * Locks used to protect global variables in this file:
> + *	Q	frag6_mutex
> + */
> +
>  struct mutex frag6_mutex = MUTEX_INITIALIZER(IPL_SOFTNET);
>  
> -u_int frag6_nfragpackets;
> -u_int frag6_nfrags;
> -TAILQ_HEAD(ip6q_head, ip6q) frag6_queue;	/* ip6 reassemble queue */
> +u_int frag6_nfragpackets;			/* [Q] */
> +u_int frag6_nfrags;				/* [Q] */
> +TAILQ_HEAD(ip6q_head, ip6q) frag6_queue;	/* [Q] ip6 reassemble queue */
>  
>  void frag6_freef(struct ip6q *);
>  void frag6_unlink(struct ip6q *, struct ip6q_head *);
> @@ -173,9 +178,8 @@ frag6_input(struct mbuf **mp, int *offp,
>  	/*
>  	 * Enforce upper bound on number of fragments.
>  	 * If maxfrag is 0, never accept fragments.
> -	 * If maxfrag is -1, accept all fragments without limitation.
>  	 */
> -	if (ip6_maxfrags >= 0 && frag6_nfrags >= (u_int)ip6_maxfrags) {
> +	if (frag6_nfrags >= atomic_load_int(&ip6_maxfrags)) {
>  		mtx_leave(&frag6_mutex);
>  		goto dropfrag;
>  	}
> Index: sys/netinet6/in6_proto.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6_proto.c,v
> retrieving revision 1.143
> diff -u -p -r1.143 in6_proto.c
> --- sys/netinet6/in6_proto.c	24 Jul 2025 21:34:07 -0000	1.143
> +++ sys/netinet6/in6_proto.c	24 Jul 2025 22:18:01 -0000
> @@ -355,7 +355,7 @@ int	ip6_sendredirects = 1;	/* [a] */
>  int	ip6_defhlim = IPV6_DEFHLIM;			/* [a] */
>  int	ip6_defmcasthlim = IPV6_DEFAULT_MULTICAST_HOPS; /* [a] */
>  int	ip6_maxfragpackets = 200;			/* [a] */
> -int	ip6_maxfrags = 200;
> +int	ip6_maxfrags = 200;	/* [a] */
>  int	ip6_log_interval = 5;	/* [a] */
>  int	ip6_hdrnestlimit = 10;	/* [a] appropriate? */
>  int	ip6_dad_count = 1;	/* [a] DupAddrDetectionTransmits */
> Index: sys/netinet6/ip6_input.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
> retrieving revision 1.288
> diff -u -p -r1.288 ip6_input.c
> --- sys/netinet6/ip6_input.c	24 Jul 2025 21:35:53 -0000	1.288
> +++ sys/netinet6/ip6_input.c	24 Jul 2025 22:18:01 -0000
> @@ -1454,10 +1454,10 @@ const struct sysctl_bounded_args ipv6ctl
>  	{ IPV6CTL_AUTO_FLOWLABEL, &ip6_auto_flowlabel, 0, 1 },
>  	{ IPV6CTL_DEFMCASTHLIM, &ip6_defmcasthlim, 0, 255 },
>  	{ IPV6CTL_USE_DEPRECATED, &ip6_use_deprecated, 0, 1 },
> +	{ IPV6CTL_MAXFRAGS, &ip6_maxfrags, 0, 1000 },
>  };
>  
>  const struct sysctl_bounded_args ipv6ctl_vars[] = {
> -	{ IPV6CTL_MAXFRAGS, &ip6_maxfrags, 0, 1000 },
>  	{ IPV6CTL_MFORWARDING, &ip6_mforwarding, 0, 1 },
>  	{ IPV6CTL_MCAST_PMTU, &ip6_mcast_pmtu, 0, 1 },
>  	{ IPV6CTL_NEIGHBORGCTHRESH, &ip6_neighborgcthresh, -1, 5 * 2048 },
> @@ -1574,6 +1574,7 @@ ip6_sysctl(int *name, u_int namelen, voi
>  	case IPV6CTL_AUTO_FLOWLABEL:
>  	case IPV6CTL_DEFMCASTHLIM:
>  	case IPV6CTL_USE_DEPRECATED:
> +	case IPV6CTL_MAXFRAGS:
>  		return (sysctl_bounded_arr(
>  		    ipv6ctl_vars_unlocked, nitems(ipv6ctl_vars_unlocked),
>  		    name, namelen, oldp, oldlenp, newp, newlen));