Date: Mon, 6 Dec 2021 17:32:49 GMT From: Gleb Smirnoff <glebius@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: d74b7baeb0d4 - main - ifnet_byindex() actually requires network epoch Message-ID: <202112061732.1B6HWnYV068607@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by glebius: URL: https://cgit.FreeBSD.org/src/commit/?id=d74b7baeb0d419fce46994075b6ccf944a0fae9a commit d74b7baeb0d419fce46994075b6ccf944a0fae9a Author: Gleb Smirnoff <glebius@FreeBSD.org> AuthorDate: 2021-12-04 17:49:36 +0000 Commit: Gleb Smirnoff <glebius@FreeBSD.org> CommitDate: 2021-12-06 17:32:31 +0000 ifnet_byindex() actually requires network epoch Sweep over potentially unsafe calls to ifnet_byindex() and wrap them in epoch. Most of the code touched remains unsafe, as the returned pointer is being used after epoch exit. Mark that with a comment. Validate the index argument inside the function, reducing argument validation requirement from the callers and making V_if_index private to if.c. Reviewed by: melifaro Differential revision: https://reviews.freebsd.org/D33263 --- sys/dev/hyperv/netvsc/if_hn.c | 6 ++++ sys/net/if.c | 16 ++++------ sys/net/if_var.h | 10 +++--- sys/netinet/igmp.c | 8 ++--- sys/netinet/in_mcast.c | 49 +++++++++++++--------------- sys/netinet/udp_usrreq.c | 11 ++++--- sys/netinet6/in6_mcast.c | 74 +++++++++++++++++++++++++++---------------- sys/netinet6/ip6_mroute.c | 9 ++++-- sys/netinet6/ip6_output.c | 6 ++-- sys/netinet6/mld6.c | 10 ++---- sys/netinet6/nd6_rtr.c | 21 ++++++------ sys/netinet6/scope6.c | 47 +++++++++++++++++---------- 12 files changed, 147 insertions(+), 120 deletions(-) diff --git a/sys/dev/hyperv/netvsc/if_hn.c b/sys/dev/hyperv/netvsc/if_hn.c index de464662c2ef..025baaa60152 100644 --- a/sys/dev/hyperv/netvsc/if_hn.c +++ b/sys/dev/hyperv/netvsc/if_hn.c @@ -4736,11 +4736,13 @@ hn_vflist_sysctl(SYSCTL_HANDLER_ARGS) first = true; for (i = 0; i < hn_vfmap_size; ++i) { + struct epoch_tracker et; struct ifnet *ifp; if (hn_vfmap[i] == NULL) continue; + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(i); if (ifp != NULL) { if (first) @@ -4749,6 +4751,7 @@ hn_vflist_sysctl(SYSCTL_HANDLER_ARGS) sbuf_printf(sb, " %s", ifp->if_xname); first = false; } + NET_EPOCH_EXIT(et); } rm_runlock(&hn_vfmap_lock, &pt); @@ -4778,12 +4781,14 @@ hn_vfmap_sysctl(SYSCTL_HANDLER_ARGS) first = true; for (i = 0; i < hn_vfmap_size; ++i) { + struct epoch_tracker et; struct ifnet *ifp, *hn_ifp; hn_ifp = hn_vfmap[i]; if (hn_ifp == NULL) continue; + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(i); if (ifp != NULL) { if (first) { @@ -4795,6 +4800,7 @@ hn_vfmap_sysctl(SYSCTL_HANDLER_ARGS) } first = false; } + NET_EPOCH_EXIT(et); } rm_runlock(&hn_vfmap_lock, &pt); diff --git a/sys/net/if.c b/sys/net/if.c index ad6d0bcf827a..87c3e4af4380 100644 --- a/sys/net/if.c +++ b/sys/net/if.c @@ -343,9 +343,11 @@ MALLOC_DEFINE(M_IFADDR, "ifaddr", "interface address"); MALLOC_DEFINE(M_IFMADDR, "ether_multi", "link-level multicast address"); struct ifnet * -ifnet_byindex(u_short idx) +ifnet_byindex(u_int idx) { + NET_EPOCH_ASSERT(); + if (__predict_false(idx > V_if_index)) return (NULL); @@ -353,12 +355,10 @@ ifnet_byindex(u_short idx) } struct ifnet * -ifnet_byindex_ref(u_short idx) +ifnet_byindex_ref(u_int idx) { struct ifnet *ifp; - NET_EPOCH_ASSERT(); - ifp = ifnet_byindex(idx); if (ifp == NULL || (ifp->if_flags & IFF_DYING)) return (NULL); @@ -679,9 +679,7 @@ if_free(struct ifnet *ifp) */ CURVNET_SET_QUIET(ifp->if_vnet); IFNET_WLOCK(); - KASSERT(ifp == ifnet_byindex(ifp->if_index), - ("%s: freeing unallocated ifnet", ifp->if_xname)); - + MPASS(V_ifindex_table[ifp->if_index] == ifp); ifindex_free(ifp->if_index); IFNET_WUNLOCK(); @@ -831,9 +829,7 @@ if_attach_internal(struct ifnet *ifp, int vmove, struct if_clone *ifc) struct sockaddr_dl *sdl; struct ifaddr *ifa; - if (ifp->if_index == 0 || ifp != ifnet_byindex(ifp->if_index)) - panic ("%s: BUG: if_attach called without if_alloc'd input()\n", - ifp->if_xname); + MPASS(V_ifindex_table[ifp->if_index] == ifp); #ifdef VIMAGE ifp->if_vnet = curvnet; diff --git a/sys/net/if_var.h b/sys/net/if_var.h index a0739fd6b76b..1647eb351db3 100644 --- a/sys/net/if_var.h +++ b/sys/net/if_var.h @@ -613,12 +613,12 @@ extern struct sx ifnet_sxlock; #define IFNET_RUNLOCK() sx_sunlock(&ifnet_sxlock) /* - * Look up an ifnet given its index; the _ref variant also acquires a - * reference that must be freed using if_rele(). It is almost always a bug - * to call ifnet_byindex() instead of ifnet_byindex_ref(). + * Look up an ifnet given its index. The returned value protected from + * being freed by the network epoch. The _ref variant also acquires a + * reference that must be freed using if_rele(). */ -struct ifnet *ifnet_byindex(u_short idx); -struct ifnet *ifnet_byindex_ref(u_short idx); +struct ifnet *ifnet_byindex(u_int); +struct ifnet *ifnet_byindex_ref(u_int); /* * Given the index, ifaddr_byindex() returns the one and only diff --git a/sys/netinet/igmp.c b/sys/netinet/igmp.c index e7636330d267..58d66ebafe64 100644 --- a/sys/netinet/igmp.c +++ b/sys/netinet/igmp.c @@ -482,6 +482,7 @@ out_locked: static int sysctl_igmp_ifinfo(SYSCTL_HANDLER_ARGS) { + struct epoch_tracker et; int *name; int error; u_int namelen; @@ -504,14 +505,11 @@ sysctl_igmp_ifinfo(SYSCTL_HANDLER_ARGS) IN_MULTI_LIST_LOCK(); IGMP_LOCK(); - if (name[0] <= 0 || name[0] > V_if_index) { - error = ENOENT; - goto out_locked; - } - error = ENOENT; + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(name[0]); + NET_EPOCH_EXIT(et); if (ifp == NULL) goto out_locked; diff --git a/sys/netinet/in_mcast.c b/sys/netinet/in_mcast.c index 6ac81aa98e44..3f25471f0858 100644 --- a/sys/netinet/in_mcast.c +++ b/sys/netinet/in_mcast.c @@ -1376,6 +1376,7 @@ in_leavegroup_locked(struct in_multi *inm, /*const*/ struct in_mfilter *imf) static int inp_block_unblock_source(struct inpcb *inp, struct sockopt *sopt) { + struct epoch_tracker et; struct group_source_req gsr; sockunion_t *gsa, *ssa; struct ifnet *ifp; @@ -1414,8 +1415,6 @@ inp_block_unblock_source(struct inpcb *inp, struct sockopt *sopt) ssa->sin.sin_addr = mreqs.imr_sourceaddr; if (!in_nullhost(mreqs.imr_interface)) { - struct epoch_tracker et; - NET_EPOCH_ENTER(et); INADDR_TO_IFP(mreqs.imr_interface, ifp); /* XXXGL: ifref? */ @@ -1445,10 +1444,11 @@ inp_block_unblock_source(struct inpcb *inp, struct sockopt *sopt) ssa->sin.sin_len != sizeof(struct sockaddr_in)) return (EINVAL); - if (gsr.gsr_interface == 0 || V_if_index < gsr.gsr_interface) - return (EADDRNOTAVAIL); - + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(gsr.gsr_interface); + NET_EPOCH_EXIT(et); + if (ifp == NULL) + return (EADDRNOTAVAIL); if (sopt->sopt_name == MCAST_BLOCK_SOURCE) doblock = 1; @@ -1624,6 +1624,7 @@ inp_freemoptions(struct ip_moptions *imo) static int inp_get_source_filters(struct inpcb *inp, struct sockopt *sopt) { + struct epoch_tracker et; struct __msfilterreq msfr; sockunion_t *gsa; struct ifnet *ifp; @@ -1649,10 +1650,9 @@ inp_get_source_filters(struct inpcb *inp, struct sockopt *sopt) if (error) return (error); - if (msfr.msfr_ifindex == 0 || V_if_index < msfr.msfr_ifindex) - return (EINVAL); - + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(msfr.msfr_ifindex); + NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifnet pointer left */ if (ifp == NULL) return (EINVAL); @@ -2026,11 +2026,11 @@ inp_join_group(struct inpcb *inp, struct sockopt *sopt) if (!IN_MULTICAST(ntohl(gsa->sin.sin_addr.s_addr))) return (EINVAL); - if (gsr.gsr_interface == 0 || V_if_index < gsr.gsr_interface) - return (EADDRNOTAVAIL); NET_EPOCH_ENTER(et); ifp = ifnet_byindex_ref(gsr.gsr_interface); NET_EPOCH_EXIT(et); + if (ifp == NULL) + return (EADDRNOTAVAIL); break; default: @@ -2243,6 +2243,7 @@ out_inp_unlocked: static int inp_leave_group(struct inpcb *inp, struct sockopt *sopt) { + struct epoch_tracker et; struct group_source_req gsr; struct ip_mreq_source mreqs; sockunion_t *gsa, *ssa; @@ -2304,8 +2305,6 @@ inp_leave_group(struct inpcb *inp, struct sockopt *sopt) * using an IPv4 address as a key is racy. */ if (!in_nullhost(mreqs.imr_interface)) { - struct epoch_tracker et; - NET_EPOCH_ENTER(et); INADDR_TO_IFP(mreqs.imr_interface, ifp); /* XXXGL ifref? */ @@ -2340,11 +2339,9 @@ inp_leave_group(struct inpcb *inp, struct sockopt *sopt) return (EINVAL); } - if (gsr.gsr_interface == 0 || V_if_index < gsr.gsr_interface) - return (EADDRNOTAVAIL); - + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(gsr.gsr_interface); - + NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */ if (ifp == NULL) return (EADDRNOTAVAIL); break; @@ -2481,13 +2478,17 @@ inp_set_multicast_if(struct inpcb *inp, struct sockopt *sopt) if (error) return (error); - if (mreqn.imr_ifindex < 0 || V_if_index < mreqn.imr_ifindex) + if (mreqn.imr_ifindex < 0) return (EINVAL); if (mreqn.imr_ifindex == 0) { ifp = NULL; } else { + struct epoch_tracker et; + + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(mreqn.imr_ifindex); + NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */ if (ifp == NULL) return (EADDRNOTAVAIL); } @@ -2536,6 +2537,7 @@ inp_set_multicast_if(struct inpcb *inp, struct sockopt *sopt) static int inp_set_source_filters(struct inpcb *inp, struct sockopt *sopt) { + struct epoch_tracker et; struct __msfilterreq msfr; sockunion_t *gsa; struct ifnet *ifp; @@ -2566,10 +2568,9 @@ inp_set_source_filters(struct inpcb *inp, struct sockopt *sopt) gsa->sin.sin_port = 0; /* ignore port */ - if (msfr.msfr_ifindex == 0 || V_if_index < msfr.msfr_ifindex) - return (EADDRNOTAVAIL); - + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(msfr.msfr_ifindex); + NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */ if (ifp == NULL) return (EADDRNOTAVAIL); @@ -2881,13 +2882,6 @@ sysctl_ip_mcast_filters(SYSCTL_HANDLER_ARGS) if (namelen != 2) return (EINVAL); - ifindex = name[0]; - if (ifindex <= 0 || ifindex > V_if_index) { - CTR2(KTR_IGMPV3, "%s: ifindex %u out of range", - __func__, ifindex); - return (ENOENT); - } - group.s_addr = name[1]; if (!IN_MULTICAST(ntohl(group.s_addr))) { CTR2(KTR_IGMPV3, "%s: group 0x%08x is not multicast", @@ -2895,6 +2889,7 @@ sysctl_ip_mcast_filters(SYSCTL_HANDLER_ARGS) return (EINVAL); } + ifindex = name[0]; NET_EPOCH_ENTER(et); ifp = ifnet_byindex(ifindex); if (ifp == NULL) { diff --git a/sys/netinet/udp_usrreq.c b/sys/netinet/udp_usrreq.c index 237287fef1e6..fe5327b3bd3c 100644 --- a/sys/netinet/udp_usrreq.c +++ b/sys/netinet/udp_usrreq.c @@ -1100,15 +1100,16 @@ udp_v4mapped_pktinfo(struct cmsghdr *cm, struct sockaddr_in * src, return (EINVAL); /* Validate the interface index if specified. */ - if (pktinfo->ipi6_ifindex > V_if_index) - return (ENXIO); - - ifp = NULL; if (pktinfo->ipi6_ifindex) { + struct epoch_tracker et; + + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(pktinfo->ipi6_ifindex); + NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */ if (ifp == NULL) return (ENXIO); - } + } else + ifp = NULL; if (ifp != NULL && !IN6_IS_ADDR_UNSPECIFIED(&pktinfo->ipi6_addr)) { ia.s_addr = pktinfo->ipi6_addr.s6_addr32[3]; if (in_ifhasaddr(ifp, ia) == 0) diff --git a/sys/netinet6/in6_mcast.c b/sys/netinet6/in6_mcast.c index 7326befc6d01..b1b161ace1b8 100644 --- a/sys/netinet6/in6_mcast.c +++ b/sys/netinet6/in6_mcast.c @@ -1402,6 +1402,7 @@ static int in6p_block_unblock_source(struct inpcb *inp, struct sockopt *sopt) { struct group_source_req gsr; + struct epoch_tracker et; sockunion_t *gsa, *ssa; struct ifnet *ifp; struct in6_mfilter *imf; @@ -1439,10 +1440,16 @@ in6p_block_unblock_source(struct inpcb *inp, struct sockopt *sopt) ssa->sin6.sin6_len != sizeof(struct sockaddr_in6)) return (EINVAL); - if (gsr.gsr_interface == 0 || V_if_index < gsr.gsr_interface) - return (EADDRNOTAVAIL); - + /* + * XXXGL: this function should use ifnet_byindex_ref, or + * expand the epoch section all the way to where we put + * the reference. + */ + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(gsr.gsr_interface); + NET_EPOCH_EXIT(et); + if (ifp == NULL) + return (EADDRNOTAVAIL); if (sopt->sopt_name == MCAST_BLOCK_SOURCE) doblock = 1; @@ -1629,6 +1636,7 @@ ip6_freemoptions(struct ip6_moptions *imo) static int in6p_get_source_filters(struct inpcb *inp, struct sockopt *sopt) { + struct epoch_tracker et; struct __msfilterreq msfr; sockunion_t *gsa; struct ifnet *ifp; @@ -1662,9 +1670,13 @@ in6p_get_source_filters(struct inpcb *inp, struct sockopt *sopt) if (!IN6_IS_ADDR_MULTICAST(&gsa->sin6.sin6_addr)) return (EINVAL); - if (msfr.msfr_ifindex == 0 || V_if_index < msfr.msfr_ifindex) - return (EADDRNOTAVAIL); + /* + * XXXGL: this function should use ifnet_byindex_ref, or expand the + * epoch section all the way to where the interface is referenced. + */ + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(msfr.msfr_ifindex); + NET_EPOCH_EXIT(et); if (ifp == NULL) return (EADDRNOTAVAIL); (void)in6_setscope(&gsa->sin6.sin6_addr, ifp, NULL); @@ -1858,12 +1870,16 @@ in6p_lookup_mcast_ifp(const struct inpcb *inp, const struct sockaddr_in6 *gsin6) * * FIXME: The KAME use of the unspecified address (::) * to join *all* multicast groups is currently unsupported. + * + * XXXGL: this function multiple times uses ifnet_byindex() without + * proper protection - staying in epoch, or putting reference on ifnet. */ static int in6p_join_group(struct inpcb *inp, struct sockopt *sopt) { struct in6_multi_head inmh; struct group_source_req gsr; + struct epoch_tracker et; sockunion_t *gsa, *ssa; struct ifnet *ifp; struct in6_mfilter *imf; @@ -1905,9 +1921,11 @@ in6p_join_group(struct inpcb *inp, struct sockopt *sopt) if (mreq.ipv6mr_interface == 0) { ifp = in6p_lookup_mcast_ifp(inp, &gsa->sin6); } else { - if (V_if_index < mreq.ipv6mr_interface) - return (EADDRNOTAVAIL); + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(mreq.ipv6mr_interface); + NET_EPOCH_EXIT(et); + if (ifp == NULL) + return (EADDRNOTAVAIL); } CTR3(KTR_MLD, "%s: ipv6mr_interface = %d, ifp = %p", __func__, mreq.ipv6mr_interface, ifp); @@ -1946,10 +1964,11 @@ in6p_join_group(struct inpcb *inp, struct sockopt *sopt) ssa->sin6.sin6_port = 0; ssa->sin6.sin6_scope_id = 0; } - - if (gsr.gsr_interface == 0 || V_if_index < gsr.gsr_interface) - return (EADDRNOTAVAIL); + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(gsr.gsr_interface); + NET_EPOCH_EXIT(et); + if (ifp == NULL) + return (EADDRNOTAVAIL); break; default: @@ -2168,6 +2187,7 @@ in6p_leave_group(struct inpcb *inp, struct sockopt *sopt) { struct ipv6_mreq mreq; struct group_source_req gsr; + struct epoch_tracker et; sockunion_t *gsa, *ssa; struct ifnet *ifp; struct in6_mfilter *imf; @@ -2266,9 +2286,9 @@ in6p_leave_group(struct inpcb *inp, struct sockopt *sopt) * XXX SCOPE6 lock potentially taken here. */ if (ifindex != 0) { - if (V_if_index < ifindex) - return (EADDRNOTAVAIL); + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(ifindex); + NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */ if (ifp == NULL) return (EADDRNOTAVAIL); (void)in6_setscope(&gsa->sin6.sin6_addr, ifp, NULL); @@ -2293,7 +2313,9 @@ in6p_leave_group(struct inpcb *inp, struct sockopt *sopt) ip6_sprintf(ip6tbuf, &gsa->sin6.sin6_addr)); ifp = in6p_lookup_mcast_ifp(inp, &gsa->sin6); } else { + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(ifindex); + NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */ } if (ifp == NULL) return (EADDRNOTAVAIL); @@ -2410,6 +2432,7 @@ out_in6p_locked: static int in6p_set_multicast_if(struct inpcb *inp, struct sockopt *sopt) { + struct epoch_tracker et; struct ifnet *ifp; struct ip6_moptions *imo; u_int ifindex; @@ -2421,19 +2444,19 @@ in6p_set_multicast_if(struct inpcb *inp, struct sockopt *sopt) error = sooptcopyin(sopt, &ifindex, sizeof(u_int), sizeof(u_int)); if (error) return (error); - if (V_if_index < ifindex) - return (EINVAL); + NET_EPOCH_ENTER(et); if (ifindex == 0) ifp = NULL; else { ifp = ifnet_byindex(ifindex); - if (ifp == NULL) - return (EINVAL); - if ((ifp->if_flags & IFF_MULTICAST) == 0) + if (ifp == NULL || (ifp->if_flags & IFF_MULTICAST) == 0) { + NET_EPOCH_EXIT(et); return (EADDRNOTAVAIL); + } } imo = in6p_findmoptions(inp); - imo->im6o_multicast_ifp = ifp; + imo->im6o_multicast_ifp = ifp; /* XXXGL: reference?! */ + NET_EPOCH_EXIT(et); INP_WUNLOCK(inp); return (0); @@ -2442,12 +2465,13 @@ in6p_set_multicast_if(struct inpcb *inp, struct sockopt *sopt) /* * Atomically set source filters on a socket for an IPv6 multicast group. * - * SMPng: NOTE: Potentially calls malloc(M_WAITOK) with Giant held. + * XXXGL: unsafely exits epoch with ifnet pointer */ static int in6p_set_source_filters(struct inpcb *inp, struct sockopt *sopt) { struct __msfilterreq msfr; + struct epoch_tracker et; sockunion_t *gsa; struct ifnet *ifp; struct in6_mfilter *imf; @@ -2477,9 +2501,9 @@ in6p_set_source_filters(struct inpcb *inp, struct sockopt *sopt) gsa->sin6.sin6_port = 0; /* ignore port */ - if (msfr.msfr_ifindex == 0 || V_if_index < msfr.msfr_ifindex) - return (EADDRNOTAVAIL); + NET_EPOCH_ENTER(et); ifp = ifnet_byindex(msfr.msfr_ifindex); + NET_EPOCH_EXIT(et); if (ifp == NULL) return (EADDRNOTAVAIL); (void)in6_setscope(&gsa->sin6.sin6_addr, ifp, NULL); @@ -2758,13 +2782,6 @@ sysctl_ip6_mcast_filters(SYSCTL_HANDLER_ARGS) if (namelen != 5) return (EINVAL); - ifindex = name[0]; - if (ifindex <= 0 || ifindex > V_if_index) { - CTR2(KTR_MLD, "%s: ifindex %u out of range", - __func__, ifindex); - return (ENOENT); - } - memcpy(&mcaddr, &name[1], sizeof(struct in6_addr)); if (!IN6_IS_ADDR_MULTICAST(&mcaddr)) { CTR2(KTR_MLD, "%s: group %s is not multicast", @@ -2772,6 +2789,7 @@ sysctl_ip6_mcast_filters(SYSCTL_HANDLER_ARGS) return (EINVAL); } + ifindex = name[0]; NET_EPOCH_ENTER(et); ifp = ifnet_byindex(ifindex); if (ifp == NULL) { diff --git a/sys/netinet6/ip6_mroute.c b/sys/netinet6/ip6_mroute.c index 087e0c5059fd..05324dfb94bf 100644 --- a/sys/netinet6/ip6_mroute.c +++ b/sys/netinet6/ip6_mroute.c @@ -677,6 +677,7 @@ static struct sockaddr_in6 sin6 = { sizeof(sin6), AF_INET6 }; static int add_m6if(struct mif6ctl *mifcp) { + struct epoch_tracker et; struct mif6 *mifp; struct ifnet *ifp; int error; @@ -692,12 +693,14 @@ add_m6if(struct mif6ctl *mifcp) MIF6_UNLOCK(); return (EADDRINUSE); /* XXX: is it appropriate? */ } - if (mifcp->mif6c_pifi == 0 || mifcp->mif6c_pifi > V_if_index) { + + NET_EPOCH_ENTER(et); + if ((ifp = ifnet_byindex(mifcp->mif6c_pifi)) == NULL) { + NET_EPOCH_EXIT(et); MIF6_UNLOCK(); return (ENXIO); } - - ifp = ifnet_byindex(mifcp->mif6c_pifi); + NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */ if (mifcp->mif6c_flags & MIFF_REGISTER) { if (reg_mif_num == (mifi_t)-1) { diff --git a/sys/netinet6/ip6_output.c b/sys/netinet6/ip6_output.c index 6091951e2ba2..7d8793b691b4 100644 --- a/sys/netinet6/ip6_output.c +++ b/sys/netinet6/ip6_output.c @@ -2819,8 +2819,8 @@ ip6_setpktopts(struct mbuf *control, struct ip6_pktopts *opt, return (EINVAL); /* - * ip6_setpktopt can call ifnet_by_index(), so it's imperative that we are - * in the net epoch here. + * ip6_setpktopt can call ifnet_byindex(), so it's imperative that we + * are in the network epoch here. */ NET_EPOCH_ASSERT(); @@ -2959,8 +2959,6 @@ ip6_setpktopt(int optname, u_char *buf, int len, struct ip6_pktopts *opt, if (IN6_IS_ADDR_MULTICAST(&pktinfo->ipi6_addr)) return (EINVAL); /* validate the interface index if specified. */ - if (pktinfo->ipi6_ifindex > V_if_index) - return (ENXIO); if (pktinfo->ipi6_ifindex) { ifp = ifnet_byindex(pktinfo->ipi6_ifindex); if (ifp == NULL) diff --git a/sys/netinet6/mld6.c b/sys/netinet6/mld6.c index c4948158bba8..1f79ef39e40e 100644 --- a/sys/netinet6/mld6.c +++ b/sys/netinet6/mld6.c @@ -356,13 +356,13 @@ out_locked: * Expose struct mld_ifsoftc to userland, keyed by ifindex. * For use by ifmcstat(8). * - * SMPng: NOTE: Does an unlocked ifindex space read. * VIMAGE: Assume curvnet set by caller. The node handler itself * is not directly virtualized. */ static int sysctl_mld_ifinfo(SYSCTL_HANDLER_ARGS) { + struct epoch_tracker et; int *name; int error; u_int namelen; @@ -385,14 +385,9 @@ sysctl_mld_ifinfo(SYSCTL_HANDLER_ARGS) IN6_MULTI_LOCK(); IN6_MULTI_LIST_LOCK(); MLD_LOCK(); - - if (name[0] <= 0 || name[0] > V_if_index) { - error = ENOENT; - goto out_locked; - } + NET_EPOCH_ENTER(et); error = ENOENT; - ifp = ifnet_byindex(name[0]); if (ifp == NULL) goto out_locked; @@ -415,6 +410,7 @@ sysctl_mld_ifinfo(SYSCTL_HANDLER_ARGS) } out_locked: + NET_EPOCH_EXIT(et); MLD_UNLOCK(); IN6_MULTI_LIST_UNLOCK(); IN6_MULTI_UNLOCK(); diff --git a/sys/netinet6/nd6_rtr.c b/sys/netinet6/nd6_rtr.c index cec9fccd63c4..07a4c2cbe7e5 100644 --- a/sys/netinet6/nd6_rtr.c +++ b/sys/netinet6/nd6_rtr.c @@ -2425,18 +2425,21 @@ rt6_flush(struct in6_addr *gateway, struct ifnet *ifp) int nd6_setdefaultiface(int ifindex) { - int error = 0; - - if (ifindex < 0 || V_if_index < ifindex) - return (EINVAL); - if (ifindex != 0 && !ifnet_byindex(ifindex)) - return (EINVAL); if (V_nd6_defifindex != ifindex) { V_nd6_defifindex = ifindex; - if (V_nd6_defifindex > 0) + if (V_nd6_defifindex != 0) { + struct epoch_tracker et; + + /* + * XXXGL: this function should use ifnet_byindex_ref! + */ + NET_EPOCH_ENTER(et); V_nd6_defifp = ifnet_byindex(V_nd6_defifindex); - else + NET_EPOCH_EXIT(et); + if (V_nd6_defifp == NULL) + return (EINVAL); + } else V_nd6_defifp = NULL; /* @@ -2447,7 +2450,7 @@ nd6_setdefaultiface(int ifindex) scope6_setdefault(V_nd6_defifp); } - return (error); + return (0); } bool diff --git a/sys/netinet6/scope6.c b/sys/netinet6/scope6.c index 099f8a78e14d..7957cec44f79 100644 --- a/sys/netinet6/scope6.c +++ b/sys/netinet6/scope6.c @@ -177,16 +177,22 @@ scope6_set(struct ifnet *ifp, struct scope6_id *idlist) return (EINVAL); } - if (i == IPV6_ADDR_SCOPE_LINKLOCAL && - idlist->s6id_list[i] > V_if_index) { - /* - * XXX: theoretically, there should be no - * relationship between link IDs and interface - * IDs, but we check the consistency for - * safety in later use. - */ - IF_AFDATA_WUNLOCK(ifp); - return (EINVAL); + if (i == IPV6_ADDR_SCOPE_LINKLOCAL) { + struct epoch_tracker et; + + NET_EPOCH_ENTER(et); + if (!ifnet_byindex(idlist->s6id_list[i])) { + /* + * XXX: theoretically, there should be + * no relationship between link IDs and + * interface IDs, but we check the + * consistency for safety in later use. + */ + NET_EPOCH_EXIT(et); + IF_AFDATA_WUNLOCK(ifp); + return (EINVAL); + } + NET_EPOCH_EXIT(et); } /* @@ -325,14 +331,20 @@ sa6_embedscope(struct sockaddr_in6 *sin6, int defaultok) if (zoneid != 0 && (IN6_IS_SCOPE_LINKLOCAL(&sin6->sin6_addr) || IN6_IS_ADDR_MC_INTFACELOCAL(&sin6->sin6_addr))) { + struct epoch_tracker et; + /* * At this moment, we only check interface-local and * link-local scope IDs, and use interface indices as the * zone IDs assuming a one-to-one mapping between interfaces * and links. */ - if (V_if_index < zoneid || ifnet_byindex(zoneid) == NULL) + NET_EPOCH_ENTER(et); + if (ifnet_byindex(zoneid) == NULL) { + NET_EPOCH_EXIT(et); return (ENXIO); + } + NET_EPOCH_EXIT(et); /* XXX assignment to 16bit from 32bit variable */ sin6->sin6_addr.s6_addr16[1] = htons(zoneid & 0xffff); @@ -358,14 +370,15 @@ sa6_recoverscope(struct sockaddr_in6 *sin6) */ zoneid = ntohs(sin6->sin6_addr.s6_addr16[1]); if (zoneid) { + struct epoch_tracker et; + + NET_EPOCH_ENTER(et); /* sanity check */ - if (V_if_index < zoneid) - return (ENXIO); -#if 0 - /* XXX: Disabled due to possible deadlock. */ - if (!ifnet_byindex(zoneid)) + if (!ifnet_byindex(zoneid)) { + NET_EPOCH_EXIT(et); return (ENXIO); -#endif + } + NET_EPOCH_EXIT(et); if (sin6->sin6_scope_id != 0 && zoneid != sin6->sin6_scope_id) { log(LOG_NOTICE,
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202112061732.1B6HWnYV068607>