Hi,

I had a look in route timer queues in netinet and netinet6 and found
some inconsistencies.

- Timeout was a mixture of int, u_int and long.  Make timeout
  int with sysctl bounds checking and make absolute time time_t.

- Some code assumes that ..._timeout_q can be NULL and at some
  places this is checked.  Better make sure that all queues always
  exist.  The pool_get is only called from initialization and from
  syscall, so PR_WAITOK is possible.

- The only special hack I kept is when ip_mtudisc is set to 0.
  Then I destroy the queue and generate an empty one.

- If redirect timeout is 0, it does not time out.  Adopt IPv6 to
  behavior of IPv4.

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

ok?

bluhm

Index: net/route.c
===================================================================
RCS file: /cvs/src/sys/net/route.c,v
retrieving revision 1.404
diff -u -p -r1.404 route.c
--- net/route.c 19 Apr 2022 19:19:31 -0000      1.404
+++ net/route.c 19 Apr 2022 20:31:49 -0000
@@ -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 (*
     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();
 
Index: net/route.h
===================================================================
RCS file: /cvs/src/sys/net/route.h,v
retrieving revision 1.188
diff -u -p -r1.188 route.h
--- net/route.h 19 Apr 2022 15:44:56 -0000      1.188
+++ net/route.h 19 Apr 2022 20:31:49 -0000
@@ -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 *);
Index: netinet/ip_icmp.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.187
diff -u -p -r1.187 ip_icmp.c
--- netinet/ip_icmp.c   26 Jul 2021 20:44:44 -0000      1.187
+++ netinet/ip_icmp.c   19 Apr 2022 20:31:50 -0000
@@ -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
 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, vo
 
        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;
 
Index: netinet/ip_input.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_input.c,v
retrieving revision 1.366
diff -u -p -r1.366 ip_input.c
--- netinet/ip_input.c  22 Feb 2022 01:35:40 -0000      1.366
+++ netinet/ip_input.c  19 Apr 2022 20:31:50 -0000
@@ -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
                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
Index: netinet/ip_var.h
===================================================================
RCS file: /cvs/src/sys/netinet/ip_var.h,v
retrieving revision 1.90
diff -u -p -r1.90 ip_var.h
--- netinet/ip_var.h    25 Feb 2022 23:51:03 -0000      1.90
+++ netinet/ip_var.h    19 Apr 2022 20:31:50 -0000
@@ -204,7 +204,7 @@ extern int ip_defttl;                       /* default IP tt
 #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 */
Index: netinet6/icmp6.c
===================================================================
RCS file: /cvs/src/sys/netinet6/icmp6.c,v
retrieving revision 1.238
diff -u -p -r1.238 icmp6.c
--- netinet6/icmp6.c    22 Feb 2022 01:15:02 -0000      1.238
+++ netinet6/icmp6.c    19 Apr 2022 20:31:52 -0000
@@ -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
                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 *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, v
 
        /* 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);
 }
Index: netinet6/ip6_input.c
===================================================================
RCS file: /cvs/src/sys/netinet6/ip6_input.c,v
retrieving revision 1.240
diff -u -p -r1.240 ip6_input.c
--- netinet6/ip6_input.c        22 Feb 2022 01:35:41 -0000      1.240
+++ netinet6/ip6_input.c        19 Apr 2022 20:31:52 -0000
@@ -1454,11 +1454,10 @@ ip6_sysctl(int *name, u_int namelen, voi
 #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:

Reply via email to