Route timeout was a mixture of int, u_int and long. Use type int

for timeout, add sysctl bounds checking between 0 and max int, and
use time_t for absolute times.

Some code assumes that the route timeout queue can be NULL and at
some places this was checked.  Better make sure that all queues
always exist.  The pool_get for struct rttimer_queue is only called
from initialization and from syscall, so PR_WAITOK is possible.

Keep the special hack when ip_mtudisc is set to 0.  Destroy the
queue and generate an empty one.

If redirect timeout is 0, it should not time out.  Check the value
in IPv6 to make the behavior like IPv4.

Sysctl net.inet6.icmp6.redirtimeout had no effect as the queue
timeout was not modified.  Make icmp6_sysctl() look like icmp_sysctl().

OK claudio@

7 files changed, 61 insertions, 79 deletions

diff --git a/sys/net/route.c b/sys/net/route.c
index 0730e11be14..8779fa70b06 100644
--- a/sys/net/route.c
+++ b/sys/net/route.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: route.c,v 1.404 2022/04/19 19:19:31 bluhm Exp $	*/
+/*	$OpenBSD: route.c,v 1.405 2022/04/20 09:38:25 bluhm Exp $	*/
 /*	$NetBSD: route.c,v 1.14 1996/02/13 22:00:46 christos Exp $	*/
 
 /*
@@ -1399,13 +1399,11 @@ rt_timer_init(void)
 }
 
 struct rttimer_queue *
-rt_timer_queue_create(u_int timeout)
+rt_timer_queue_create(int timeout)
 {
 	struct rttimer_queue	*rtq;
 
-	rtq = pool_get(&rttimer_queue_pool, PR_NOWAIT | PR_ZERO);
-	if (rtq == NULL)
-		return (NULL);
+	rtq = pool_get(&rttimer_queue_pool, PR_WAITOK | PR_ZERO);
 
 	rtq->rtq_timeout = timeout;
 	rtq->rtq_count = 0;
@@ -1416,7 +1414,7 @@ rt_timer_queue_create(u_int timeout)
 }
 
 void
-rt_timer_queue_change(struct rttimer_queue *rtq, long timeout)
+rt_timer_queue_change(struct rttimer_queue *rtq, int timeout)
 {
 	rtq->rtq_timeout = timeout;
 }
@@ -1470,10 +1468,10 @@ rt_timer_add(struct rtentry *rt, void (*func)(struct rtentry *,
     struct rttimer *), struct rttimer_queue *queue, u_int rtableid)
 {
 	struct rttimer	*r;
-	long		 current_time;
+	time_t		 current_time;
 
 	current_time = getuptime();
-	rt->rt_expire = getuptime() + queue->rtq_timeout;
+	rt->rt_expire = current_time + queue->rtq_timeout;
 
 	/*
 	 * If there's already a timer with this action, destroy it before
@@ -1514,7 +1512,7 @@ rt_timer_timer(void *arg)
 	struct timeout		*to = (struct timeout *)arg;
 	struct rttimer_queue	*rtq;
 	struct rttimer		*r;
-	long			 current_time;
+	time_t			 current_time;
 
 	current_time = getuptime();
 
diff --git a/sys/net/route.h b/sys/net/route.h
index 99883769604..eeb59fb40de 100644
--- a/sys/net/route.h
+++ b/sys/net/route.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: route.h,v 1.188 2022/04/19 15:44:56 bluhm Exp $	*/
+/*	$OpenBSD: route.h,v 1.189 2022/04/20 09:38:26 bluhm Exp $	*/
 /*	$NetBSD: route.h,v 1.9 1996/02/13 22:00:49 christos Exp $	*/
 
 /*
@@ -411,10 +411,10 @@ struct rttimer {
 };
 
 struct rttimer_queue {
-	long				rtq_timeout;
-	unsigned long			rtq_count;
 	TAILQ_HEAD(, rttimer)		rtq_head;
 	LIST_ENTRY(rttimer_queue)	rtq_link;
+	unsigned long			rtq_count;
+	int				rtq_timeout;
 };
 
 const char	*rtlabel_id2name(u_int16_t);
@@ -456,8 +456,8 @@ int			 rt_timer_add(struct rtentry *,
 		             void(*)(struct rtentry *, struct rttimer *),
 			     struct rttimer_queue *, u_int);
 void			 rt_timer_remove_all(struct rtentry *);
-struct rttimer_queue	*rt_timer_queue_create(u_int);
-void			 rt_timer_queue_change(struct rttimer_queue *, long);
+struct rttimer_queue	*rt_timer_queue_create(int);
+void			 rt_timer_queue_change(struct rttimer_queue *, int);
 void			 rt_timer_queue_destroy(struct rttimer_queue *);
 unsigned long		 rt_timer_queue_count(struct rttimer_queue *);
 void			 rt_timer_timer(void *);
diff --git a/sys/netinet/ip_icmp.c b/sys/netinet/ip_icmp.c
index 88d49863b2d..97b06361473 100644
--- a/sys/netinet/ip_icmp.c
+++ b/sys/netinet/ip_icmp.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_icmp.c,v 1.187 2021/07/26 20:44:44 bluhm Exp $	*/
+/*	$OpenBSD: ip_icmp.c,v 1.188 2022/04/20 09:38:26 bluhm Exp $	*/
 /*	$NetBSD: ip_icmp.c,v 1.19 1996/02/13 23:42:22 christos Exp $	*/
 
 /*
@@ -120,7 +120,7 @@ int	icmp_redirtimeout = 10 * 60;
 static int icmperrpps_count = 0;
 static struct timeval icmperrppslim_last;
 
-static struct rttimer_queue *icmp_redirect_timeout_q = NULL;
+struct rttimer_queue *icmp_redirect_timeout_q;
 struct cpumem *icmpcounters;
 
 const struct sysctl_bounded_args icmpctl_vars[] =  {
@@ -141,15 +141,8 @@ int icmp_sysctl_icmpstat(void *, size_t *, void *);
 void
 icmp_init(void)
 {
+	icmp_redirect_timeout_q = rt_timer_queue_create(icmp_redirtimeout);
 	icmpcounters = counters_alloc(icps_ncounters);
-	/*
-	 * This is only useful if the user initializes redirtimeout to
-	 * something other than zero.
-	 */
-	if (icmp_redirtimeout != 0) {
-		icmp_redirect_timeout_q =
-		    rt_timer_queue_create(icmp_redirtimeout);
-	}
 }
 
 struct mbuf *
@@ -640,12 +633,11 @@ reflect:
 #endif
 		rtredirect(sintosa(&sdst), sintosa(&sgw),
 		    sintosa(&ssrc), &newrt, m->m_pkthdr.ph_rtableid);
-		if (newrt != NULL && icmp_redirtimeout != 0) {
-			(void)rt_timer_add(newrt, icmp_redirect_timeout,
+		if (newrt != NULL && icmp_redirtimeout > 0) {
+			rt_timer_add(newrt, icmp_redirect_timeout,
 			    icmp_redirect_timeout_q, m->m_pkthdr.ph_rtableid);
 		}
-		if (newrt != NULL)
-			rtfree(newrt);
+		rtfree(newrt);
 		pfctlinput(PRC_REDIRECT_HOST, sintosa(&sdst));
 		break;
 	}
@@ -889,21 +881,11 @@ icmp_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
 
 	switch (name[0]) {
 	case ICMPCTL_REDIRTIMEOUT:
-
 		NET_LOCK();
-		error = sysctl_int(oldp, oldlenp, newp, newlen,
-		    &icmp_redirtimeout);
-		if (icmp_redirect_timeout_q != NULL) {
-			if (icmp_redirtimeout == 0) {
-				rt_timer_queue_destroy(icmp_redirect_timeout_q);
-				icmp_redirect_timeout_q = NULL;
-			} else
-				rt_timer_queue_change(icmp_redirect_timeout_q,
-				    icmp_redirtimeout);
-		} else if (icmp_redirtimeout > 0) {
-			icmp_redirect_timeout_q =
-			    rt_timer_queue_create(icmp_redirtimeout);
-		}
+		error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
+		    &icmp_redirtimeout, 0, INT_MAX);
+		rt_timer_queue_change(icmp_redirect_timeout_q,
+		    icmp_redirtimeout);
 		NET_UNLOCK();
 		break;
 
diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index 7e0ba1ee412..b365659a5f9 100644
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_input.c,v 1.366 2022/02/22 01:35:40 guenther Exp $	*/
+/*	$OpenBSD: ip_input.c,v 1.367 2022/04/20 09:38:26 bluhm Exp $	*/
 /*	$NetBSD: ip_input.c,v 1.30 1996/03/16 23:53:58 christos Exp $	*/
 
 /*
@@ -91,10 +91,10 @@ int	ipsendredirects = 1;
 int	ip_dosourceroute = 0;
 int	ip_defttl = IPDEFTTL;
 int	ip_mtudisc = 1;
-u_int	ip_mtudisc_timeout = IPMTUDISCTIMEOUT;
+int	ip_mtudisc_timeout = IPMTUDISCTIMEOUT;
 int	ip_directedbcast = 0;
 
-struct rttimer_queue *ip_mtudisc_timeout_q = NULL;
+struct rttimer_queue *ip_mtudisc_timeout_q;
 
 /* Protects `ipq' and `ip_frags'. */
 struct mutex	ipq_mutex = MUTEX_INITIALIZER(IPL_SOFTNET);
@@ -201,9 +201,7 @@ ip_init(void)
 		    pr->pr_protocol < IPPROTO_MAX)
 			ip_protox[pr->pr_protocol] = pr - inetsw;
 	LIST_INIT(&ipq);
-	if (ip_mtudisc != 0)
-		ip_mtudisc_timeout_q =
-		    rt_timer_queue_create(ip_mtudisc_timeout);
+	ip_mtudisc_timeout_q = rt_timer_queue_create(ip_mtudisc_timeout);
 
 	/* Fill in list of ports not to allocate dynamically. */
 	memset(&baddynamicports, 0, sizeof(baddynamicports));
@@ -1616,24 +1614,20 @@ ip_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp, void *newp,
 		return (error);
 	case IPCTL_MTUDISC:
 		NET_LOCK();
-		error = sysctl_int(oldp, oldlenp, newp, newlen,
-		    &ip_mtudisc);
-		if (ip_mtudisc != 0 && ip_mtudisc_timeout_q == NULL) {
+		error = sysctl_int(oldp, oldlenp, newp, newlen, &ip_mtudisc);
+		if (ip_mtudisc == 0) {
+			rt_timer_queue_destroy(ip_mtudisc_timeout_q);
 			ip_mtudisc_timeout_q =
 			    rt_timer_queue_create(ip_mtudisc_timeout);
-		} else if (ip_mtudisc == 0 && ip_mtudisc_timeout_q != NULL) {
-			rt_timer_queue_destroy(ip_mtudisc_timeout_q);
-			ip_mtudisc_timeout_q = NULL;
 		}
 		NET_UNLOCK();
 		return error;
 	case IPCTL_MTUDISCTIMEOUT:
 		NET_LOCK();
-		error = sysctl_int(oldp, oldlenp, newp, newlen,
-		   &ip_mtudisc_timeout);
-		if (ip_mtudisc_timeout_q != NULL)
-			rt_timer_queue_change(ip_mtudisc_timeout_q,
-					      ip_mtudisc_timeout);
+		error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
+		    &ip_mtudisc_timeout, 0, INT_MAX);
+		rt_timer_queue_change(ip_mtudisc_timeout_q,
+		    ip_mtudisc_timeout);
 		NET_UNLOCK();
 		return (error);
 #ifdef IPSEC
diff --git a/sys/netinet/ip_var.h b/sys/netinet/ip_var.h
index 22cf4383019..4b978b0d66d 100644
--- a/sys/netinet/ip_var.h
+++ b/sys/netinet/ip_var.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip_var.h,v 1.90 2022/02/25 23:51:03 guenther Exp $	*/
+/*	$OpenBSD: ip_var.h,v 1.91 2022/04/20 09:38:26 bluhm Exp $	*/
 /*	$NetBSD: ip_var.h,v 1.16 1996/02/13 23:43:20 christos Exp $	*/
 
 /*
@@ -204,7 +204,7 @@ extern int ip_defttl;			/* default IP ttl */
 #define IPMTUDISCTIMEOUT (10 * 60)	/* as per RFC 1191 */
 
 extern int ip_mtudisc;			/* mtu discovery */
-extern u_int ip_mtudisc_timeout;	/* seconds to timeout mtu discovery */
+extern int ip_mtudisc_timeout;		/* seconds to timeout mtu discovery */
 
 extern int ipport_firstauto;		/* min port for port allocation */
 extern int ipport_lastauto;		/* max port for port allocation */
diff --git a/sys/netinet6/icmp6.c b/sys/netinet6/icmp6.c
index e3b79a13b08..ee1685ece95 100644
--- a/sys/netinet6/icmp6.c
+++ b/sys/netinet6/icmp6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: icmp6.c,v 1.238 2022/02/22 01:15:02 guenther Exp $	*/
+/*	$OpenBSD: icmp6.c,v 1.239 2022/04/20 09:38:26 bluhm Exp $	*/
 /*	$KAME: icmp6.c,v 1.217 2001/06/20 15:03:29 jinmei Exp $	*/
 
 /*
@@ -118,7 +118,7 @@ struct icmp6_mtudisc_callback {
 LIST_HEAD(, icmp6_mtudisc_callback) icmp6_mtudisc_callbacks =
     LIST_HEAD_INITIALIZER(icmp6_mtudisc_callbacks);
 
-struct rttimer_queue *icmp6_mtudisc_timeout_q = NULL;
+struct rttimer_queue *icmp6_mtudisc_timeout_q;
 
 /* XXX do these values make any sense? */
 static int icmp6_mtudisc_hiwat = 1280;
@@ -127,7 +127,7 @@ static int icmp6_mtudisc_lowat = 256;
 /*
  * keep track of # of redirect routes.
  */
-static struct rttimer_queue *icmp6_redirect_timeout_q = NULL;
+struct rttimer_queue *icmp6_redirect_timeout_q;
 
 /* XXX experimental, turned off */
 static int icmp6_redirect_lowat = -1;
@@ -1404,12 +1404,11 @@ icmp6_redirect_input(struct mbuf *m, int off)
 		memcpy(&ssrc.sin6_addr, &src6, sizeof(struct in6_addr));
 		rtredirect(sin6tosa(&sdst), sin6tosa(&sgw), sin6tosa(&ssrc),
 		    &newrt, m->m_pkthdr.ph_rtableid);
-
-		if (newrt) {
-			(void)rt_timer_add(newrt, icmp6_redirect_timeout,
+		if (newrt != NULL && icmp6_redirtimeout > 0) {
+			rt_timer_add(newrt, icmp6_redirect_timeout,
 			    icmp6_redirect_timeout_q, m->m_pkthdr.ph_rtableid);
-			rtfree(newrt);
 		}
+		rtfree(newrt);
 	}
 	/* finally update cached route in each socket via pfctlinput */
 	{
@@ -1881,7 +1880,6 @@ icmp6_redirect_timeout(struct rtentry *rt, struct rttimer *r)
 }
 
 const struct sysctl_bounded_args icmpv6ctl_vars[] = {
-	{ ICMPV6CTL_REDIRTIMEOUT, &icmp6_redirtimeout, 0, INT_MAX },
 	{ ICMPV6CTL_ND6_DELAY, &nd6_delay, 0, INT_MAX },
 	{ ICMPV6CTL_ND6_UMAXTRIES, &nd6_umaxtries, 0, INT_MAX },
 	{ ICMPV6CTL_ND6_MMAXTRIES, &nd6_mmaxtries, 0, INT_MAX },
@@ -1916,19 +1914,30 @@ icmp6_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp,
 
 	/* All sysctl names at this level are terminal. */
 	if (namelen != 1)
-		return ENOTDIR;
+		return (ENOTDIR);
 
 	switch (name[0]) {
+	case ICMPV6CTL_REDIRTIMEOUT:
+		NET_LOCK();
+		error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
+		    &icmp6_redirtimeout, 0, INT_MAX);
+		rt_timer_queue_change(icmp6_redirect_timeout_q,
+		    icmp6_redirtimeout);
+		NET_UNLOCK();
+		break;
 
 	case ICMPV6CTL_STATS:
-		return icmp6_sysctl_icmp6stat(oldp, oldlenp, newp);
+		error = icmp6_sysctl_icmp6stat(oldp, oldlenp, newp);
+		break;
+
 	default:
 		NET_LOCK();
 		error = sysctl_bounded_arr(icmpv6ctl_vars,
 		    nitems(icmpv6ctl_vars), name, namelen, oldp, oldlenp, newp,
 		    newlen);
 		NET_UNLOCK();
-		return (error);
+		break;
 	}
-	/* NOTREACHED */
+
+	return (error);
 }
diff --git a/sys/netinet6/ip6_input.c b/sys/netinet6/ip6_input.c
index fa20f0685ea..ddaf164632c 100644
--- a/sys/netinet6/ip6_input.c
+++ b/sys/netinet6/ip6_input.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ip6_input.c,v 1.240 2022/02/22 01:35:41 guenther Exp $	*/
+/*	$OpenBSD: ip6_input.c,v 1.241 2022/04/20 09:38:26 bluhm Exp $	*/
 /*	$KAME: ip6_input.c,v 1.188 2001/03/29 05:34:31 itojun Exp $	*/
 
 /*
@@ -1454,11 +1454,10 @@ ip6_sysctl(int *name, u_int namelen, void *oldp, size_t *oldlenp,
 #endif
 	case IPV6CTL_MTUDISCTIMEOUT:
 		NET_LOCK();
-		error = sysctl_int(oldp, oldlenp, newp, newlen,
-		    &ip6_mtudisc_timeout);
-		if (icmp6_mtudisc_timeout_q != NULL)
-			rt_timer_queue_change(icmp6_mtudisc_timeout_q,
-			    ip6_mtudisc_timeout);
+		error = sysctl_int_bounded(oldp, oldlenp, newp, newlen,
+		    &ip6_mtudisc_timeout, 0, INT_MAX);
+		rt_timer_queue_change(icmp6_mtudisc_timeout_q,
+		    ip6_mtudisc_timeout);
 		NET_UNLOCK();
 		return (error);
 	case IPV6CTL_IFQUEUE: