On Tue, Jun 11, 2019 at 01:29:26PM -0300, Martin Pieuchot wrote:
> On 09/06/19(Sun) 15:41, Martin Pieuchot wrote:
> > [...]
> > Another way to prevent stack exhaustion would be to return a reference
> > to any `rt' that needs to be deleted instead of deleting it in place.
>
> Diff below does that by adding a new `prt' argument to rtable_walk().
> I'm willing to create a manual for rtable_walk(9) if you agree with the
> direction below.
OK bluhm@
> Index: net/if_spppsubr.c
> ===================================================================
> RCS file: /cvs/src/sys/net/if_spppsubr.c,v
> retrieving revision 1.174
> diff -u -p -r1.174 if_spppsubr.c
> --- net/if_spppsubr.c 19 Feb 2018 08:59:52 -0000 1.174
> +++ net/if_spppsubr.c 11 Jun 2019 15:30:10 -0000
> @@ -4167,7 +4167,7 @@ sppp_update_gw(struct ifnet *ifp)
>
> /* update routing table */
> for (tid = 0; tid <= RT_TABLEID_MAX; tid++) {
> - rtable_walk(tid, AF_INET, sppp_update_gw_walker, ifp);
> + rtable_walk(tid, AF_INET, NULL, sppp_update_gw_walker, ifp);
> }
> }
>
> Index: net/route.c
> ===================================================================
> RCS file: /cvs/src/sys/net/route.c,v
> retrieving revision 1.384
> diff -u -p -r1.384 route.c
> --- net/route.c 11 May 2019 16:47:02 -0000 1.384
> +++ net/route.c 11 Jun 2019 16:24:39 -0000
> @@ -156,7 +156,7 @@ void rt_timer_init(void);
> int rt_setgwroute(struct rtentry *, u_int);
> void rt_putgwroute(struct rtentry *);
> int rtflushclone1(struct rtentry *, void *, u_int);
> -void rtflushclone(unsigned int, struct rtentry *);
> +int rtflushclone(struct ifnet *ifp, struct rtentry *, unsigned int);
> int rt_ifa_purge_walker(struct rtentry *, void *, unsigned int);
> struct rtentry *rt_match(struct sockaddr *, uint32_t *, int, unsigned int);
> int rt_clone(struct rtentry **, struct sockaddr *, unsigned int);
> @@ -701,7 +701,6 @@ rtflushclone1(struct rtentry *rt, void *
> {
> struct rtentry *cloningrt = arg;
> struct ifnet *ifp;
> - int error;
>
> if (!ISSET(rt->rt_flags, RTF_CLONED))
> return 0;
> @@ -721,23 +720,35 @@ rtflushclone1(struct rtentry *rt, void *
> if (ifp == NULL)
> return 0;
>
> - error = rtdeletemsg(rt, ifp, id);
> - if (error == 0)
> - error = EAGAIN;
> -
> if_put(ifp);
> - return error;
> + return EEXIST;
> }
>
> -void
> -rtflushclone(unsigned int rtableid, struct rtentry *parent)
> +int
> +rtflushclone(struct ifnet *ifp, struct rtentry *parent, unsigned int
> rtableid)
> {
> + struct rtentry *rt = NULL;
> + int error;
>
> #ifdef DIAGNOSTIC
> if (!parent || (parent->rt_flags & RTF_CLONING) == 0)
> panic("rtflushclone: called with a non-cloning route");
> #endif
> - rtable_walk(rtableid, rt_key(parent)->sa_family, rtflushclone1, parent);
> +
> + do {
> + error = rtable_walk(rtableid, rt_key(parent)->sa_family, &rt,
> + rtflushclone1, parent);
> + if (rt != NULL && error == EEXIST) {
> + error = rtdeletemsg(rt, ifp, rtableid);
> + if (error == 0)
> + error = EAGAIN;
> + }
> + rtfree(rt);
> + rt = NULL;
> + } while (error == EAGAIN);
> +
> + return error;
> +
> }
>
> int
> @@ -779,7 +790,7 @@ rtrequest_delete(struct rt_addrinfo *inf
>
> /* Clean up any cloned children. */
> if (ISSET(rt->rt_flags, RTF_CLONING))
> - rtflushclone(tableid, rt);
> + rtflushclone(ifp, rt, tableid);
>
> rtfree(rt->rt_parent);
> rt->rt_parent = NULL;
> @@ -1241,12 +1252,13 @@ rt_ifa_dellocal(struct ifaddr *ifa)
> /*
> * Remove all addresses attached to ``ifa''.
> */
> -void
> +int
> rt_ifa_purge(struct ifaddr *ifa)
> {
> struct ifnet *ifp = ifa->ifa_ifp;
> + struct rtentry *rt = NULL;
> unsigned int rtableid;
> - int i;
> + int error, af = ifa->ifa_addr->sa_family;
>
> KASSERT(ifp != NULL);
>
> @@ -1254,27 +1266,38 @@ rt_ifa_purge(struct ifaddr *ifa)
> /* skip rtables that are not in the rdomain of the ifp */
> if (rtable_l2(rtableid) != ifp->if_rdomain)
> continue;
> - for (i = 1; i <= AF_MAX; i++) {
> - rtable_walk(rtableid, i, rt_ifa_purge_walker, ifa);
> - }
> +
> + do {
> + error = rtable_walk(rtableid, af, &rt,
> + rt_ifa_purge_walker, ifa);
> + if (rt != NULL && error == EEXIST) {
> + error = rtdeletemsg(rt, ifp, rtableid);
> + if (error == 0)
> + error = EAGAIN;
> + }
> + rtfree(rt);
> + rt = NULL;
> + } while (error == EAGAIN);
> +
> + if (error == EAFNOSUPPORT)
> + error = 0;
> +
> + if (error)
> + break;
> }
> +
> + return error;
> }
>
> int
> rt_ifa_purge_walker(struct rtentry *rt, void *vifa, unsigned int rtableid)
> {
> struct ifaddr *ifa = vifa;
> - struct ifnet *ifp = ifa->ifa_ifp;
> - int error;
>
> - if (rt->rt_ifa != ifa)
> - return (0);
> -
> - if ((error = rtdeletemsg(rt, ifp, rtableid))) {
> - return (error);
> - }
> + if (rt->rt_ifa == ifa)
> + return EEXIST;
>
> - return (EAGAIN);
> + return 0;
> }
>
> /*
> @@ -1596,23 +1619,42 @@ rtlabel_unref(u_int16_t id)
> }
> }
>
> -void
> +int
> rt_if_track(struct ifnet *ifp)
> {
> - int i;
> - u_int tid;
> + unsigned int rtableid;
> + struct rtentry *rt = NULL;
> + int i, error;
>
> - for (tid = 0; tid < rtmap_limit; tid++) {
> + for (rtableid = 0; rtableid < rtmap_limit; rtableid++) {
> /* skip rtables that are not in the rdomain of the ifp */
> - if (rtable_l2(tid) != ifp->if_rdomain)
> + if (rtable_l2(rtableid) != ifp->if_rdomain)
> continue;
> for (i = 1; i <= AF_MAX; i++) {
> - if (!rtable_mpath_capable(tid, i))
> + if (!rtable_mpath_capable(rtableid, i))
> continue;
>
> - rtable_walk(tid, i, rt_if_linkstate_change, ifp);
> + do {
> + error = rtable_walk(rtableid, i, &rt,
> + rt_if_linkstate_change, ifp);
> + if (rt != NULL && error == EEXIST) {
> + error = rtdeletemsg(rt, ifp, rtableid);
> + if (error == 0)
> + error = EAGAIN;
> + }
> + rtfree(rt);
> + rt = NULL;
> + } while (error == EAGAIN);
> +
> + if (error == EAFNOSUPPORT)
> + error = 0;
> +
> + if (error)
> + break;
> }
> }
> +
> + return (error);
> }
>
> int
> @@ -1647,9 +1689,7 @@ rt_if_linkstate_change(struct rtentry *r
> */
> if (ISSET(rt->rt_flags, RTF_CLONED|RTF_DYNAMIC) &&
> !ISSET(rt->rt_flags, RTF_CACHED|RTF_BFD)) {
> - if ((error = rtdeletemsg(rt, ifp, id)))
> - return (error);
> - return (EAGAIN);
> + return (EEXIST);
> }
>
> if (!ISSET(rt->rt_flags, RTF_UP))
> @@ -1780,7 +1820,7 @@ int
> db_show_arptab(void)
> {
> db_printf("Route tree for AF_INET\n");
> - rtable_walk(0, AF_INET, db_show_rtentry, NULL);
> + rtable_walk(0, AF_INET, NULL, db_show_rtentry, NULL);
> return (0);
> }
> #endif /* DDB */
> Index: net/route.h
> ===================================================================
> RCS file: /cvs/src/sys/net/route.h,v
> retrieving revision 1.175
> diff -u -p -r1.175 route.h
> --- net/route.h 28 Apr 2019 17:59:51 -0000 1.175
> +++ net/route.h 11 Jun 2019 15:30:10 -0000
> @@ -451,7 +451,7 @@ void rtfree(struct rtentry *);
>
> int rt_ifa_add(struct ifaddr *, int, struct sockaddr *, unsigned int);
> int rt_ifa_del(struct ifaddr *, int, struct sockaddr *, unsigned int);
> -void rt_ifa_purge(struct ifaddr *);
> +int rt_ifa_purge(struct ifaddr *);
> int rt_ifa_addlocal(struct ifaddr *);
> int rt_ifa_dellocal(struct ifaddr *);
> void rtredirect(struct sockaddr *, struct sockaddr *, struct sockaddr *,
> struct rtentry **, unsigned int);
> @@ -459,7 +459,7 @@ int rtrequest(int, struct rt_addrinfo *
> u_int);
> int rtrequest_delete(struct rt_addrinfo *, u_int8_t, struct ifnet *,
> struct rtentry **, u_int);
> -void rt_if_track(struct ifnet *);
> +int rt_if_track(struct ifnet *);
> int rt_if_linkstate_change(struct rtentry *, void *, u_int);
> int rtdeletemsg(struct rtentry *, struct ifnet *, u_int);
> #endif /* _KERNEL */
> Index: net/rtable.c
> ===================================================================
> RCS file: /cvs/src/sys/net/rtable.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 rtable.c
> --- net/rtable.c 5 Mar 2019 19:07:56 -0000 1.68
> +++ net/rtable.c 11 Jun 2019 16:01:39 -0000
> @@ -664,6 +664,7 @@ leave:
> struct rtable_walk_cookie {
> int (*rwc_func)(struct rtentry *, void *, unsigned int);
> void *rwc_arg;
> + struct rtentry **rwc_prt;
> unsigned int rwc_rid;
> };
>
> @@ -679,16 +680,21 @@ rtable_walk_helper(struct art_node *an,
> int error = 0;
>
> SRPL_FOREACH(rt, &sr, &an->an_rtlist, rt_next) {
> - if ((error = (*rwc->rwc_func)(rt, rwc->rwc_arg, rwc->rwc_rid)))
> + error = (*rwc->rwc_func)(rt, rwc->rwc_arg, rwc->rwc_rid);
> + if (error != 0)
> break;
> }
> + if (rwc->rwc_prt != NULL && rt != NULL) {
> + rtref(rt);
> + *rwc->rwc_prt = rt;
> + }
> SRPL_LEAVE(&sr);
>
> return (error);
> }
>
> int
> -rtable_walk(unsigned int rtableid, sa_family_t af,
> +rtable_walk(unsigned int rtableid, sa_family_t af, struct rtentry **prt,
> int (*func)(struct rtentry *, void *, unsigned int), void *arg)
> {
> struct art_root *ar;
> @@ -701,10 +707,10 @@ rtable_walk(unsigned int rtableid, sa_fa
>
> rwc.rwc_func = func;
> rwc.rwc_arg = arg;
> + rwc.rwc_prt = prt;
> rwc.rwc_rid = rtableid;
>
> - while ((error = art_walk(ar, rtable_walk_helper, &rwc)) == EAGAIN)
> - continue;
> + error = art_walk(ar, rtable_walk_helper, &rwc);
>
> return (error);
> }
> Index: net/rtable.h
> ===================================================================
> RCS file: /cvs/src/sys/net/rtable.h,v
> retrieving revision 1.23
> diff -u -p -r1.23 rtable.h
> --- net/rtable.h 28 Apr 2019 17:59:51 -0000 1.23
> +++ net/rtable.h 11 Jun 2019 15:30:10 -0000
> @@ -48,7 +48,7 @@ int rtable_insert(unsigned int, struct
> struct rtentry *);
> int rtable_delete(unsigned int, struct sockaddr *,
> struct sockaddr *, struct rtentry *);
> -int rtable_walk(unsigned int, sa_family_t,
> +int rtable_walk(unsigned int, sa_family_t, struct rtentry **,
> int (*)(struct rtentry *, void *, unsigned int), void *);
>
> int rtable_mpath_capable(unsigned int, sa_family_t);
> Index: net/rtsock.c
> ===================================================================
> RCS file: /cvs/src/sys/net/rtsock.c,v
> retrieving revision 1.287
> diff -u -p -r1.287 rtsock.c
> --- net/rtsock.c 5 Jun 2019 12:53:43 -0000 1.287
> +++ net/rtsock.c 11 Jun 2019 15:30:10 -0000
> @@ -931,7 +931,7 @@ rtm_output(struct rt_msghdr *rtm, struct
> NET_LOCK();
> ifp->if_rtrequest(ifp, RTM_INVALIDATE, rt);
> /* Reset the MTU of the gateway route. */
> - rtable_walk(tableid, rt_key(rt)->sa_family,
> + rtable_walk(tableid, rt_key(rt)->sa_family, NULL,
> route_cleargateway, rt);
> NET_UNLOCK();
> if_put(ifp);
> @@ -1911,7 +1911,8 @@ sysctl_rtable(int *name, u_int namelen,
> if (af != 0 && af != i)
> continue;
>
> - error = rtable_walk(tableid, i, sysctl_dumpentry, &w);
> + error = rtable_walk(tableid, i, NULL, sysctl_dumpentry,
> + &w);
> if (error == EAFNOSUPPORT)
> error = 0;
> if (error)
> Index: netinet/ip_mroute.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet/ip_mroute.c,v
> retrieving revision 1.126
> diff -u -p -r1.126 ip_mroute.c
> --- netinet/ip_mroute.c 4 Jun 2019 16:11:13 -0000 1.126
> +++ netinet/ip_mroute.c 11 Jun 2019 15:30:10 -0000
> @@ -476,8 +476,10 @@ mrt_sysctl_mfc(void *oldp, size_t *oldle
> msa.msa_len = *oldlenp;
> msa.msa_needed = 0;
>
> - for (rtableid = 0; rtableid <= RT_TABLEID_MAX; rtableid++)
> - rtable_walk(rtableid, AF_INET, mrt_rtwalk_mfcsysctl, &msa);
> + for (rtableid = 0; rtableid <= RT_TABLEID_MAX; rtableid++) {
> + rtable_walk(rtableid, AF_INET, NULL, mrt_rtwalk_mfcsysctl,
> + &msa);
> + }
>
> if (msa.msa_minfos != NULL && msa.msa_needed > 0 &&
> (error = copyout(msa.msa_minfos, oldp, msa.msa_needed)) != 0) {
> @@ -549,7 +551,7 @@ ip_mrouter_done(struct socket *so)
> NET_ASSERT_LOCKED();
>
> /* Delete all remaining installed multicast routes. */
> - rtable_walk(rtableid, AF_INET, mrouter_rtwalk_delete, NULL);
> + rtable_walk(rtableid, AF_INET, NULL, mrouter_rtwalk_delete, NULL);
>
> TAILQ_FOREACH(ifp, &ifnet, if_list) {
> if (ifp->if_rdomain != rtableid)
> Index: netinet6/ip6_mroute.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_mroute.c,v
> retrieving revision 1.119
> diff -u -p -r1.119 ip6_mroute.c
> --- netinet6/ip6_mroute.c 4 Jun 2019 16:11:13 -0000 1.119
> +++ netinet6/ip6_mroute.c 11 Jun 2019 15:30:10 -0000
> @@ -454,8 +454,10 @@ mrt6_sysctl_mfc(void *oldp, size_t *oldl
> msa.ms6a_len = *oldlenp;
> msa.ms6a_needed = 0;
>
> - for (rtableid = 0; rtableid <= RT_TABLEID_MAX; rtableid++)
> - rtable_walk(rtableid, AF_INET6, mrt6_rtwalk_mf6csysctl, &msa);
> + for (rtableid = 0; rtableid <= RT_TABLEID_MAX; rtableid++) {
> + rtable_walk(rtableid, AF_INET6, NULL, mrt6_rtwalk_mf6csysctl,
> + &msa);
> + }
>
> if (msa.ms6a_minfos != NULL && msa.ms6a_needed > 0 &&
> (error = copyout(msa.ms6a_minfos, oldp, msa.ms6a_needed)) != 0) {
> @@ -523,7 +525,7 @@ ip6_mrouter_done(struct socket *so)
> NET_ASSERT_LOCKED();
>
> /* Delete all remaining installed multicast routes. */
> - rtable_walk(rtableid, AF_INET6, mrouter6_rtwalk_delete, NULL);
> + rtable_walk(rtableid, AF_INET6, NULL, mrouter6_rtwalk_delete, NULL);
>
> /* Unregister all interfaces in the domain. */
> TAILQ_FOREACH(ifp, &ifnet, if_list) {
> Index: netinet6/nd6.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/nd6.h,v
> retrieving revision 1.74
> diff -u -p -r1.74 nd6.h
> --- netinet6/nd6.h 27 Nov 2017 15:41:30 -0000 1.74
> +++ netinet6/nd6.h 11 Jun 2019 15:30:10 -0000
> @@ -193,7 +193,7 @@ void nd6_dad_stop(struct ifaddr *);
> void nd6_rtr_cache(struct mbuf *, int, int, int);
>
> int in6_ifdel(struct ifnet *, struct in6_addr *);
> -void rt6_flush(struct in6_addr *, struct ifnet *);
> +int rt6_flush(struct in6_addr *, struct ifnet *);
>
> void nd6_expire_timer_update(struct in6_ifaddr *);
> #endif /* _KERNEL */
> Index: netinet6/nd6_rtr.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/nd6_rtr.c,v
> retrieving revision 1.166
> diff -u -p -r1.166 nd6_rtr.c
> --- netinet6/nd6_rtr.c 23 Jan 2018 22:06:42 -0000 1.166
> +++ netinet6/nd6_rtr.c 11 Jun 2019 16:26:43 -0000
> @@ -173,28 +173,48 @@ nd6_rtr_cache(struct mbuf *m, int off, i
> * it shouldn't be called when acting as a router.
> * The gateway must already contain KAME's hack for link-local scope.
> */
> -void
> +int
> rt6_flush(struct in6_addr *gateway, struct ifnet *ifp)
> {
> + struct rt_addrinfo info;
> + struct sockaddr_in6 sa_mask;
> + struct rtentry *rt = NULL;
> + int error;
> +
> NET_ASSERT_LOCKED();
>
> /* We'll care only link-local addresses */
> if (!IN6_IS_ADDR_LINKLOCAL(gateway))
> - return;
> + return (0);
>
> KASSERT(gateway->s6_addr16[1] != 0);
>
> - rtable_walk(ifp->if_rdomain, AF_INET6, rt6_deleteroute, gateway);
> + do {
> + error = rtable_walk(ifp->if_rdomain, AF_INET6, &rt,
> + rt6_deleteroute, gateway);
> + if (rt != NULL && error == EEXIST) {
> + memset(&info, 0, sizeof(info));
> + info.rti_flags = rt->rt_flags;
> + info.rti_info[RTAX_DST] = rt_key(rt);
> + info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
> + info.rti_info[RTAX_NETMASK] = rt_plen2mask(rt,
> + &sa_mask);
> + error = rtrequest_delete(&info, RTP_ANY, ifp, NULL,
> + ifp->if_rdomain);
> + if (error == 0)
> + error = EAGAIN;
> + }
> + rtfree(rt);
> + rt = NULL;
> + } while (error == EAGAIN);
> +
> + return (error);
> }
>
> int
> rt6_deleteroute(struct rtentry *rt, void *arg, unsigned int id)
> {
> - struct ifnet *ifp;
> - struct rt_addrinfo info;
> struct in6_addr *gate = (struct in6_addr *)arg;
> - struct sockaddr_in6 sa_mask;
> - int error = 0;
>
> if (rt->rt_gateway == NULL || rt->rt_gateway->sa_family != AF_INET6)
> return (0);
> @@ -217,16 +237,5 @@ rt6_deleteroute(struct rtentry *rt, void
> if ((rt->rt_flags & RTF_HOST) == 0)
> return (0);
>
> - ifp = if_get(rt->rt_ifidx);
> - if (ifp != NULL) {
> - bzero(&info, sizeof(info));
> - info.rti_flags = rt->rt_flags;
> - info.rti_info[RTAX_DST] = rt_key(rt);
> - info.rti_info[RTAX_GATEWAY] = rt->rt_gateway;
> - info.rti_info[RTAX_NETMASK] = rt_plen2mask(rt, &sa_mask);
> - error = rtrequest_delete(&info, RTP_ANY, ifp, NULL, id);
> - }
> - if_put(ifp);
> -
> - return (error != 0 ? error : EAGAIN);
> + return (EEXIST);
> }
