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
This commit is contained in:
Gleb Smirnoff 2021-12-04 09:49:36 -08:00
parent 7b40b00fad
commit d74b7baeb0
12 changed files with 147 additions and 120 deletions

View File

@ -4736,11 +4736,13 @@ hn_vflist_sysctl(SYSCTL_HANDLER_ARGS)
first = true; first = true;
for (i = 0; i < hn_vfmap_size; ++i) { for (i = 0; i < hn_vfmap_size; ++i) {
struct epoch_tracker et;
struct ifnet *ifp; struct ifnet *ifp;
if (hn_vfmap[i] == NULL) if (hn_vfmap[i] == NULL)
continue; continue;
NET_EPOCH_ENTER(et);
ifp = ifnet_byindex(i); ifp = ifnet_byindex(i);
if (ifp != NULL) { if (ifp != NULL) {
if (first) if (first)
@ -4749,6 +4751,7 @@ hn_vflist_sysctl(SYSCTL_HANDLER_ARGS)
sbuf_printf(sb, " %s", ifp->if_xname); sbuf_printf(sb, " %s", ifp->if_xname);
first = false; first = false;
} }
NET_EPOCH_EXIT(et);
} }
rm_runlock(&hn_vfmap_lock, &pt); rm_runlock(&hn_vfmap_lock, &pt);
@ -4778,12 +4781,14 @@ hn_vfmap_sysctl(SYSCTL_HANDLER_ARGS)
first = true; first = true;
for (i = 0; i < hn_vfmap_size; ++i) { for (i = 0; i < hn_vfmap_size; ++i) {
struct epoch_tracker et;
struct ifnet *ifp, *hn_ifp; struct ifnet *ifp, *hn_ifp;
hn_ifp = hn_vfmap[i]; hn_ifp = hn_vfmap[i];
if (hn_ifp == NULL) if (hn_ifp == NULL)
continue; continue;
NET_EPOCH_ENTER(et);
ifp = ifnet_byindex(i); ifp = ifnet_byindex(i);
if (ifp != NULL) { if (ifp != NULL) {
if (first) { if (first) {
@ -4795,6 +4800,7 @@ hn_vfmap_sysctl(SYSCTL_HANDLER_ARGS)
} }
first = false; first = false;
} }
NET_EPOCH_EXIT(et);
} }
rm_runlock(&hn_vfmap_lock, &pt); rm_runlock(&hn_vfmap_lock, &pt);

View File

@ -343,9 +343,11 @@ MALLOC_DEFINE(M_IFADDR, "ifaddr", "interface address");
MALLOC_DEFINE(M_IFMADDR, "ether_multi", "link-level multicast address"); MALLOC_DEFINE(M_IFMADDR, "ether_multi", "link-level multicast address");
struct ifnet * struct ifnet *
ifnet_byindex(u_short idx) ifnet_byindex(u_int idx)
{ {
NET_EPOCH_ASSERT();
if (__predict_false(idx > V_if_index)) if (__predict_false(idx > V_if_index))
return (NULL); return (NULL);
@ -353,12 +355,10 @@ ifnet_byindex(u_short idx)
} }
struct ifnet * struct ifnet *
ifnet_byindex_ref(u_short idx) ifnet_byindex_ref(u_int idx)
{ {
struct ifnet *ifp; struct ifnet *ifp;
NET_EPOCH_ASSERT();
ifp = ifnet_byindex(idx); ifp = ifnet_byindex(idx);
if (ifp == NULL || (ifp->if_flags & IFF_DYING)) if (ifp == NULL || (ifp->if_flags & IFF_DYING))
return (NULL); return (NULL);
@ -679,9 +679,7 @@ if_free(struct ifnet *ifp)
*/ */
CURVNET_SET_QUIET(ifp->if_vnet); CURVNET_SET_QUIET(ifp->if_vnet);
IFNET_WLOCK(); IFNET_WLOCK();
KASSERT(ifp == ifnet_byindex(ifp->if_index), MPASS(V_ifindex_table[ifp->if_index] == ifp);
("%s: freeing unallocated ifnet", ifp->if_xname));
ifindex_free(ifp->if_index); ifindex_free(ifp->if_index);
IFNET_WUNLOCK(); IFNET_WUNLOCK();
@ -831,9 +829,7 @@ if_attach_internal(struct ifnet *ifp, int vmove, struct if_clone *ifc)
struct sockaddr_dl *sdl; struct sockaddr_dl *sdl;
struct ifaddr *ifa; struct ifaddr *ifa;
if (ifp->if_index == 0 || ifp != ifnet_byindex(ifp->if_index)) MPASS(V_ifindex_table[ifp->if_index] == ifp);
panic ("%s: BUG: if_attach called without if_alloc'd input()\n",
ifp->if_xname);
#ifdef VIMAGE #ifdef VIMAGE
ifp->if_vnet = curvnet; ifp->if_vnet = curvnet;

View File

@ -613,12 +613,12 @@ extern struct sx ifnet_sxlock;
#define IFNET_RUNLOCK() sx_sunlock(&ifnet_sxlock) #define IFNET_RUNLOCK() sx_sunlock(&ifnet_sxlock)
/* /*
* Look up an ifnet given its index; the _ref variant also acquires a * Look up an ifnet given its index. The returned value protected from
* reference that must be freed using if_rele(). It is almost always a bug * being freed by the network epoch. The _ref variant also acquires a
* to call ifnet_byindex() instead of ifnet_byindex_ref(). * reference that must be freed using if_rele().
*/ */
struct ifnet *ifnet_byindex(u_short idx); struct ifnet *ifnet_byindex(u_int);
struct ifnet *ifnet_byindex_ref(u_short idx); struct ifnet *ifnet_byindex_ref(u_int);
/* /*
* Given the index, ifaddr_byindex() returns the one and only * Given the index, ifaddr_byindex() returns the one and only

View File

@ -482,6 +482,7 @@ out_locked:
static int static int
sysctl_igmp_ifinfo(SYSCTL_HANDLER_ARGS) sysctl_igmp_ifinfo(SYSCTL_HANDLER_ARGS)
{ {
struct epoch_tracker et;
int *name; int *name;
int error; int error;
u_int namelen; u_int namelen;
@ -504,14 +505,11 @@ sysctl_igmp_ifinfo(SYSCTL_HANDLER_ARGS)
IN_MULTI_LIST_LOCK(); IN_MULTI_LIST_LOCK();
IGMP_LOCK(); IGMP_LOCK();
if (name[0] <= 0 || name[0] > V_if_index) {
error = ENOENT;
goto out_locked;
}
error = ENOENT; error = ENOENT;
NET_EPOCH_ENTER(et);
ifp = ifnet_byindex(name[0]); ifp = ifnet_byindex(name[0]);
NET_EPOCH_EXIT(et);
if (ifp == NULL) if (ifp == NULL)
goto out_locked; goto out_locked;

View File

@ -1376,6 +1376,7 @@ in_leavegroup_locked(struct in_multi *inm, /*const*/ struct in_mfilter *imf)
static int static int
inp_block_unblock_source(struct inpcb *inp, struct sockopt *sopt) inp_block_unblock_source(struct inpcb *inp, struct sockopt *sopt)
{ {
struct epoch_tracker et;
struct group_source_req gsr; struct group_source_req gsr;
sockunion_t *gsa, *ssa; sockunion_t *gsa, *ssa;
struct ifnet *ifp; struct ifnet *ifp;
@ -1414,8 +1415,6 @@ inp_block_unblock_source(struct inpcb *inp, struct sockopt *sopt)
ssa->sin.sin_addr = mreqs.imr_sourceaddr; ssa->sin.sin_addr = mreqs.imr_sourceaddr;
if (!in_nullhost(mreqs.imr_interface)) { if (!in_nullhost(mreqs.imr_interface)) {
struct epoch_tracker et;
NET_EPOCH_ENTER(et); NET_EPOCH_ENTER(et);
INADDR_TO_IFP(mreqs.imr_interface, ifp); INADDR_TO_IFP(mreqs.imr_interface, ifp);
/* XXXGL: ifref? */ /* XXXGL: ifref? */
@ -1445,10 +1444,11 @@ inp_block_unblock_source(struct inpcb *inp, struct sockopt *sopt)
ssa->sin.sin_len != sizeof(struct sockaddr_in)) ssa->sin.sin_len != sizeof(struct sockaddr_in))
return (EINVAL); return (EINVAL);
if (gsr.gsr_interface == 0 || V_if_index < gsr.gsr_interface) NET_EPOCH_ENTER(et);
return (EADDRNOTAVAIL);
ifp = ifnet_byindex(gsr.gsr_interface); ifp = ifnet_byindex(gsr.gsr_interface);
NET_EPOCH_EXIT(et);
if (ifp == NULL)
return (EADDRNOTAVAIL);
if (sopt->sopt_name == MCAST_BLOCK_SOURCE) if (sopt->sopt_name == MCAST_BLOCK_SOURCE)
doblock = 1; doblock = 1;
@ -1624,6 +1624,7 @@ inp_freemoptions(struct ip_moptions *imo)
static int static int
inp_get_source_filters(struct inpcb *inp, struct sockopt *sopt) inp_get_source_filters(struct inpcb *inp, struct sockopt *sopt)
{ {
struct epoch_tracker et;
struct __msfilterreq msfr; struct __msfilterreq msfr;
sockunion_t *gsa; sockunion_t *gsa;
struct ifnet *ifp; struct ifnet *ifp;
@ -1649,10 +1650,9 @@ inp_get_source_filters(struct inpcb *inp, struct sockopt *sopt)
if (error) if (error)
return (error); return (error);
if (msfr.msfr_ifindex == 0 || V_if_index < msfr.msfr_ifindex) NET_EPOCH_ENTER(et);
return (EINVAL);
ifp = ifnet_byindex(msfr.msfr_ifindex); ifp = ifnet_byindex(msfr.msfr_ifindex);
NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifnet pointer left */
if (ifp == NULL) if (ifp == NULL)
return (EINVAL); 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))) if (!IN_MULTICAST(ntohl(gsa->sin.sin_addr.s_addr)))
return (EINVAL); return (EINVAL);
if (gsr.gsr_interface == 0 || V_if_index < gsr.gsr_interface)
return (EADDRNOTAVAIL);
NET_EPOCH_ENTER(et); NET_EPOCH_ENTER(et);
ifp = ifnet_byindex_ref(gsr.gsr_interface); ifp = ifnet_byindex_ref(gsr.gsr_interface);
NET_EPOCH_EXIT(et); NET_EPOCH_EXIT(et);
if (ifp == NULL)
return (EADDRNOTAVAIL);
break; break;
default: default:
@ -2243,6 +2243,7 @@ out_inp_unlocked:
static int static int
inp_leave_group(struct inpcb *inp, struct sockopt *sopt) inp_leave_group(struct inpcb *inp, struct sockopt *sopt)
{ {
struct epoch_tracker et;
struct group_source_req gsr; struct group_source_req gsr;
struct ip_mreq_source mreqs; struct ip_mreq_source mreqs;
sockunion_t *gsa, *ssa; 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. * using an IPv4 address as a key is racy.
*/ */
if (!in_nullhost(mreqs.imr_interface)) { if (!in_nullhost(mreqs.imr_interface)) {
struct epoch_tracker et;
NET_EPOCH_ENTER(et); NET_EPOCH_ENTER(et);
INADDR_TO_IFP(mreqs.imr_interface, ifp); INADDR_TO_IFP(mreqs.imr_interface, ifp);
/* XXXGL ifref? */ /* XXXGL ifref? */
@ -2340,11 +2339,9 @@ inp_leave_group(struct inpcb *inp, struct sockopt *sopt)
return (EINVAL); return (EINVAL);
} }
if (gsr.gsr_interface == 0 || V_if_index < gsr.gsr_interface) NET_EPOCH_ENTER(et);
return (EADDRNOTAVAIL);
ifp = ifnet_byindex(gsr.gsr_interface); ifp = ifnet_byindex(gsr.gsr_interface);
NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */
if (ifp == NULL) if (ifp == NULL)
return (EADDRNOTAVAIL); return (EADDRNOTAVAIL);
break; break;
@ -2481,13 +2478,17 @@ inp_set_multicast_if(struct inpcb *inp, struct sockopt *sopt)
if (error) if (error)
return (error); return (error);
if (mreqn.imr_ifindex < 0 || V_if_index < mreqn.imr_ifindex) if (mreqn.imr_ifindex < 0)
return (EINVAL); return (EINVAL);
if (mreqn.imr_ifindex == 0) { if (mreqn.imr_ifindex == 0) {
ifp = NULL; ifp = NULL;
} else { } else {
struct epoch_tracker et;
NET_EPOCH_ENTER(et);
ifp = ifnet_byindex(mreqn.imr_ifindex); ifp = ifnet_byindex(mreqn.imr_ifindex);
NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */
if (ifp == NULL) if (ifp == NULL)
return (EADDRNOTAVAIL); return (EADDRNOTAVAIL);
} }
@ -2536,6 +2537,7 @@ inp_set_multicast_if(struct inpcb *inp, struct sockopt *sopt)
static int static int
inp_set_source_filters(struct inpcb *inp, struct sockopt *sopt) inp_set_source_filters(struct inpcb *inp, struct sockopt *sopt)
{ {
struct epoch_tracker et;
struct __msfilterreq msfr; struct __msfilterreq msfr;
sockunion_t *gsa; sockunion_t *gsa;
struct ifnet *ifp; struct ifnet *ifp;
@ -2566,10 +2568,9 @@ inp_set_source_filters(struct inpcb *inp, struct sockopt *sopt)
gsa->sin.sin_port = 0; /* ignore port */ gsa->sin.sin_port = 0; /* ignore port */
if (msfr.msfr_ifindex == 0 || V_if_index < msfr.msfr_ifindex) NET_EPOCH_ENTER(et);
return (EADDRNOTAVAIL);
ifp = ifnet_byindex(msfr.msfr_ifindex); ifp = ifnet_byindex(msfr.msfr_ifindex);
NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */
if (ifp == NULL) if (ifp == NULL)
return (EADDRNOTAVAIL); return (EADDRNOTAVAIL);
@ -2881,13 +2882,6 @@ sysctl_ip_mcast_filters(SYSCTL_HANDLER_ARGS)
if (namelen != 2) if (namelen != 2)
return (EINVAL); 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]; group.s_addr = name[1];
if (!IN_MULTICAST(ntohl(group.s_addr))) { if (!IN_MULTICAST(ntohl(group.s_addr))) {
CTR2(KTR_IGMPV3, "%s: group 0x%08x is not multicast", CTR2(KTR_IGMPV3, "%s: group 0x%08x is not multicast",
@ -2895,6 +2889,7 @@ sysctl_ip_mcast_filters(SYSCTL_HANDLER_ARGS)
return (EINVAL); return (EINVAL);
} }
ifindex = name[0];
NET_EPOCH_ENTER(et); NET_EPOCH_ENTER(et);
ifp = ifnet_byindex(ifindex); ifp = ifnet_byindex(ifindex);
if (ifp == NULL) { if (ifp == NULL) {

View File

@ -1100,15 +1100,16 @@ udp_v4mapped_pktinfo(struct cmsghdr *cm, struct sockaddr_in * src,
return (EINVAL); return (EINVAL);
/* Validate the interface index if specified. */ /* Validate the interface index if specified. */
if (pktinfo->ipi6_ifindex > V_if_index)
return (ENXIO);
ifp = NULL;
if (pktinfo->ipi6_ifindex) { if (pktinfo->ipi6_ifindex) {
struct epoch_tracker et;
NET_EPOCH_ENTER(et);
ifp = ifnet_byindex(pktinfo->ipi6_ifindex); ifp = ifnet_byindex(pktinfo->ipi6_ifindex);
NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */
if (ifp == NULL) if (ifp == NULL)
return (ENXIO); return (ENXIO);
} } else
ifp = NULL;
if (ifp != NULL && !IN6_IS_ADDR_UNSPECIFIED(&pktinfo->ipi6_addr)) { if (ifp != NULL && !IN6_IS_ADDR_UNSPECIFIED(&pktinfo->ipi6_addr)) {
ia.s_addr = pktinfo->ipi6_addr.s6_addr32[3]; ia.s_addr = pktinfo->ipi6_addr.s6_addr32[3];
if (in_ifhasaddr(ifp, ia) == 0) if (in_ifhasaddr(ifp, ia) == 0)

View File

@ -1402,6 +1402,7 @@ static int
in6p_block_unblock_source(struct inpcb *inp, struct sockopt *sopt) in6p_block_unblock_source(struct inpcb *inp, struct sockopt *sopt)
{ {
struct group_source_req gsr; struct group_source_req gsr;
struct epoch_tracker et;
sockunion_t *gsa, *ssa; sockunion_t *gsa, *ssa;
struct ifnet *ifp; struct ifnet *ifp;
struct in6_mfilter *imf; 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)) ssa->sin6.sin6_len != sizeof(struct sockaddr_in6))
return (EINVAL); 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); ifp = ifnet_byindex(gsr.gsr_interface);
NET_EPOCH_EXIT(et);
if (ifp == NULL)
return (EADDRNOTAVAIL);
if (sopt->sopt_name == MCAST_BLOCK_SOURCE) if (sopt->sopt_name == MCAST_BLOCK_SOURCE)
doblock = 1; doblock = 1;
@ -1629,6 +1636,7 @@ ip6_freemoptions(struct ip6_moptions *imo)
static int static int
in6p_get_source_filters(struct inpcb *inp, struct sockopt *sopt) in6p_get_source_filters(struct inpcb *inp, struct sockopt *sopt)
{ {
struct epoch_tracker et;
struct __msfilterreq msfr; struct __msfilterreq msfr;
sockunion_t *gsa; sockunion_t *gsa;
struct ifnet *ifp; 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)) if (!IN6_IS_ADDR_MULTICAST(&gsa->sin6.sin6_addr))
return (EINVAL); 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); ifp = ifnet_byindex(msfr.msfr_ifindex);
NET_EPOCH_EXIT(et);
if (ifp == NULL) if (ifp == NULL)
return (EADDRNOTAVAIL); return (EADDRNOTAVAIL);
(void)in6_setscope(&gsa->sin6.sin6_addr, ifp, NULL); (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 (::) * FIXME: The KAME use of the unspecified address (::)
* to join *all* multicast groups is currently unsupported. * 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 static int
in6p_join_group(struct inpcb *inp, struct sockopt *sopt) in6p_join_group(struct inpcb *inp, struct sockopt *sopt)
{ {
struct in6_multi_head inmh; struct in6_multi_head inmh;
struct group_source_req gsr; struct group_source_req gsr;
struct epoch_tracker et;
sockunion_t *gsa, *ssa; sockunion_t *gsa, *ssa;
struct ifnet *ifp; struct ifnet *ifp;
struct in6_mfilter *imf; struct in6_mfilter *imf;
@ -1905,9 +1921,11 @@ in6p_join_group(struct inpcb *inp, struct sockopt *sopt)
if (mreq.ipv6mr_interface == 0) { if (mreq.ipv6mr_interface == 0) {
ifp = in6p_lookup_mcast_ifp(inp, &gsa->sin6); ifp = in6p_lookup_mcast_ifp(inp, &gsa->sin6);
} else { } else {
if (V_if_index < mreq.ipv6mr_interface) NET_EPOCH_ENTER(et);
return (EADDRNOTAVAIL);
ifp = ifnet_byindex(mreq.ipv6mr_interface); ifp = ifnet_byindex(mreq.ipv6mr_interface);
NET_EPOCH_EXIT(et);
if (ifp == NULL)
return (EADDRNOTAVAIL);
} }
CTR3(KTR_MLD, "%s: ipv6mr_interface = %d, ifp = %p", CTR3(KTR_MLD, "%s: ipv6mr_interface = %d, ifp = %p",
__func__, mreq.ipv6mr_interface, ifp); __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_port = 0;
ssa->sin6.sin6_scope_id = 0; ssa->sin6.sin6_scope_id = 0;
} }
NET_EPOCH_ENTER(et);
if (gsr.gsr_interface == 0 || V_if_index < gsr.gsr_interface)
return (EADDRNOTAVAIL);
ifp = ifnet_byindex(gsr.gsr_interface); ifp = ifnet_byindex(gsr.gsr_interface);
NET_EPOCH_EXIT(et);
if (ifp == NULL)
return (EADDRNOTAVAIL);
break; break;
default: default:
@ -2168,6 +2187,7 @@ in6p_leave_group(struct inpcb *inp, struct sockopt *sopt)
{ {
struct ipv6_mreq mreq; struct ipv6_mreq mreq;
struct group_source_req gsr; struct group_source_req gsr;
struct epoch_tracker et;
sockunion_t *gsa, *ssa; sockunion_t *gsa, *ssa;
struct ifnet *ifp; struct ifnet *ifp;
struct in6_mfilter *imf; struct in6_mfilter *imf;
@ -2266,9 +2286,9 @@ in6p_leave_group(struct inpcb *inp, struct sockopt *sopt)
* XXX SCOPE6 lock potentially taken here. * XXX SCOPE6 lock potentially taken here.
*/ */
if (ifindex != 0) { if (ifindex != 0) {
if (V_if_index < ifindex) NET_EPOCH_ENTER(et);
return (EADDRNOTAVAIL);
ifp = ifnet_byindex(ifindex); ifp = ifnet_byindex(ifindex);
NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */
if (ifp == NULL) if (ifp == NULL)
return (EADDRNOTAVAIL); return (EADDRNOTAVAIL);
(void)in6_setscope(&gsa->sin6.sin6_addr, ifp, NULL); (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)); ip6_sprintf(ip6tbuf, &gsa->sin6.sin6_addr));
ifp = in6p_lookup_mcast_ifp(inp, &gsa->sin6); ifp = in6p_lookup_mcast_ifp(inp, &gsa->sin6);
} else { } else {
NET_EPOCH_ENTER(et);
ifp = ifnet_byindex(ifindex); ifp = ifnet_byindex(ifindex);
NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */
} }
if (ifp == NULL) if (ifp == NULL)
return (EADDRNOTAVAIL); return (EADDRNOTAVAIL);
@ -2410,6 +2432,7 @@ out_in6p_locked:
static int static int
in6p_set_multicast_if(struct inpcb *inp, struct sockopt *sopt) in6p_set_multicast_if(struct inpcb *inp, struct sockopt *sopt)
{ {
struct epoch_tracker et;
struct ifnet *ifp; struct ifnet *ifp;
struct ip6_moptions *imo; struct ip6_moptions *imo;
u_int ifindex; 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)); error = sooptcopyin(sopt, &ifindex, sizeof(u_int), sizeof(u_int));
if (error) if (error)
return (error); return (error);
if (V_if_index < ifindex) NET_EPOCH_ENTER(et);
return (EINVAL);
if (ifindex == 0) if (ifindex == 0)
ifp = NULL; ifp = NULL;
else { else {
ifp = ifnet_byindex(ifindex); ifp = ifnet_byindex(ifindex);
if (ifp == NULL) if (ifp == NULL || (ifp->if_flags & IFF_MULTICAST) == 0) {
return (EINVAL); NET_EPOCH_EXIT(et);
if ((ifp->if_flags & IFF_MULTICAST) == 0)
return (EADDRNOTAVAIL); return (EADDRNOTAVAIL);
} }
}
imo = in6p_findmoptions(inp); imo = in6p_findmoptions(inp);
imo->im6o_multicast_ifp = ifp; imo->im6o_multicast_ifp = ifp; /* XXXGL: reference?! */
NET_EPOCH_EXIT(et);
INP_WUNLOCK(inp); INP_WUNLOCK(inp);
return (0); 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. * 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 static int
in6p_set_source_filters(struct inpcb *inp, struct sockopt *sopt) in6p_set_source_filters(struct inpcb *inp, struct sockopt *sopt)
{ {
struct __msfilterreq msfr; struct __msfilterreq msfr;
struct epoch_tracker et;
sockunion_t *gsa; sockunion_t *gsa;
struct ifnet *ifp; struct ifnet *ifp;
struct in6_mfilter *imf; 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 */ gsa->sin6.sin6_port = 0; /* ignore port */
if (msfr.msfr_ifindex == 0 || V_if_index < msfr.msfr_ifindex) NET_EPOCH_ENTER(et);
return (EADDRNOTAVAIL);
ifp = ifnet_byindex(msfr.msfr_ifindex); ifp = ifnet_byindex(msfr.msfr_ifindex);
NET_EPOCH_EXIT(et);
if (ifp == NULL) if (ifp == NULL)
return (EADDRNOTAVAIL); return (EADDRNOTAVAIL);
(void)in6_setscope(&gsa->sin6.sin6_addr, ifp, NULL); (void)in6_setscope(&gsa->sin6.sin6_addr, ifp, NULL);
@ -2758,13 +2782,6 @@ sysctl_ip6_mcast_filters(SYSCTL_HANDLER_ARGS)
if (namelen != 5) if (namelen != 5)
return (EINVAL); 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)); memcpy(&mcaddr, &name[1], sizeof(struct in6_addr));
if (!IN6_IS_ADDR_MULTICAST(&mcaddr)) { if (!IN6_IS_ADDR_MULTICAST(&mcaddr)) {
CTR2(KTR_MLD, "%s: group %s is not multicast", CTR2(KTR_MLD, "%s: group %s is not multicast",
@ -2772,6 +2789,7 @@ sysctl_ip6_mcast_filters(SYSCTL_HANDLER_ARGS)
return (EINVAL); return (EINVAL);
} }
ifindex = name[0];
NET_EPOCH_ENTER(et); NET_EPOCH_ENTER(et);
ifp = ifnet_byindex(ifindex); ifp = ifnet_byindex(ifindex);
if (ifp == NULL) { if (ifp == NULL) {

View File

@ -677,6 +677,7 @@ static struct sockaddr_in6 sin6 = { sizeof(sin6), AF_INET6 };
static int static int
add_m6if(struct mif6ctl *mifcp) add_m6if(struct mif6ctl *mifcp)
{ {
struct epoch_tracker et;
struct mif6 *mifp; struct mif6 *mifp;
struct ifnet *ifp; struct ifnet *ifp;
int error; int error;
@ -692,12 +693,14 @@ add_m6if(struct mif6ctl *mifcp)
MIF6_UNLOCK(); MIF6_UNLOCK();
return (EADDRINUSE); /* XXX: is it appropriate? */ 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(); MIF6_UNLOCK();
return (ENXIO); return (ENXIO);
} }
NET_EPOCH_EXIT(et); /* XXXGL: unsafe ifp */
ifp = ifnet_byindex(mifcp->mif6c_pifi);
if (mifcp->mif6c_flags & MIFF_REGISTER) { if (mifcp->mif6c_flags & MIFF_REGISTER) {
if (reg_mif_num == (mifi_t)-1) { if (reg_mif_num == (mifi_t)-1) {

View File

@ -2819,8 +2819,8 @@ ip6_setpktopts(struct mbuf *control, struct ip6_pktopts *opt,
return (EINVAL); return (EINVAL);
/* /*
* ip6_setpktopt can call ifnet_by_index(), so it's imperative that we are * ip6_setpktopt can call ifnet_byindex(), so it's imperative that we
* in the net epoch here. * are in the network epoch here.
*/ */
NET_EPOCH_ASSERT(); 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)) if (IN6_IS_ADDR_MULTICAST(&pktinfo->ipi6_addr))
return (EINVAL); return (EINVAL);
/* validate the interface index if specified. */ /* validate the interface index if specified. */
if (pktinfo->ipi6_ifindex > V_if_index)
return (ENXIO);
if (pktinfo->ipi6_ifindex) { if (pktinfo->ipi6_ifindex) {
ifp = ifnet_byindex(pktinfo->ipi6_ifindex); ifp = ifnet_byindex(pktinfo->ipi6_ifindex);
if (ifp == NULL) if (ifp == NULL)

View File

@ -356,13 +356,13 @@ out_locked:
* Expose struct mld_ifsoftc to userland, keyed by ifindex. * Expose struct mld_ifsoftc to userland, keyed by ifindex.
* For use by ifmcstat(8). * For use by ifmcstat(8).
* *
* SMPng: NOTE: Does an unlocked ifindex space read.
* VIMAGE: Assume curvnet set by caller. The node handler itself * VIMAGE: Assume curvnet set by caller. The node handler itself
* is not directly virtualized. * is not directly virtualized.
*/ */
static int static int
sysctl_mld_ifinfo(SYSCTL_HANDLER_ARGS) sysctl_mld_ifinfo(SYSCTL_HANDLER_ARGS)
{ {
struct epoch_tracker et;
int *name; int *name;
int error; int error;
u_int namelen; u_int namelen;
@ -385,14 +385,9 @@ sysctl_mld_ifinfo(SYSCTL_HANDLER_ARGS)
IN6_MULTI_LOCK(); IN6_MULTI_LOCK();
IN6_MULTI_LIST_LOCK(); IN6_MULTI_LIST_LOCK();
MLD_LOCK(); MLD_LOCK();
NET_EPOCH_ENTER(et);
if (name[0] <= 0 || name[0] > V_if_index) {
error = ENOENT;
goto out_locked;
}
error = ENOENT; error = ENOENT;
ifp = ifnet_byindex(name[0]); ifp = ifnet_byindex(name[0]);
if (ifp == NULL) if (ifp == NULL)
goto out_locked; goto out_locked;
@ -415,6 +410,7 @@ sysctl_mld_ifinfo(SYSCTL_HANDLER_ARGS)
} }
out_locked: out_locked:
NET_EPOCH_EXIT(et);
MLD_UNLOCK(); MLD_UNLOCK();
IN6_MULTI_LIST_UNLOCK(); IN6_MULTI_LIST_UNLOCK();
IN6_MULTI_UNLOCK(); IN6_MULTI_UNLOCK();

View File

@ -2425,18 +2425,21 @@ rt6_flush(struct in6_addr *gateway, struct ifnet *ifp)
int int
nd6_setdefaultiface(int ifindex) 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) { if (V_nd6_defifindex != ifindex) {
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); 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; V_nd6_defifp = NULL;
/* /*
@ -2447,7 +2450,7 @@ nd6_setdefaultiface(int ifindex)
scope6_setdefault(V_nd6_defifp); scope6_setdefault(V_nd6_defifp);
} }
return (error); return (0);
} }
bool bool

View File

@ -177,17 +177,23 @@ scope6_set(struct ifnet *ifp, struct scope6_id *idlist)
return (EINVAL); return (EINVAL);
} }
if (i == IPV6_ADDR_SCOPE_LINKLOCAL && if (i == IPV6_ADDR_SCOPE_LINKLOCAL) {
idlist->s6id_list[i] > V_if_index) { struct epoch_tracker et;
NET_EPOCH_ENTER(et);
if (!ifnet_byindex(idlist->s6id_list[i])) {
/* /*
* XXX: theoretically, there should be no * XXX: theoretically, there should be
* relationship between link IDs and interface * no relationship between link IDs and
* IDs, but we check the consistency for * interface IDs, but we check the
* safety in later use. * consistency for safety in later use.
*/ */
NET_EPOCH_EXIT(et);
IF_AFDATA_WUNLOCK(ifp); IF_AFDATA_WUNLOCK(ifp);
return (EINVAL); return (EINVAL);
} }
NET_EPOCH_EXIT(et);
}
/* /*
* XXX: we must need lots of work in this case, * XXX: we must need lots of work in this case,
@ -325,14 +331,20 @@ sa6_embedscope(struct sockaddr_in6 *sin6, int defaultok)
if (zoneid != 0 && if (zoneid != 0 &&
(IN6_IS_SCOPE_LINKLOCAL(&sin6->sin6_addr) || (IN6_IS_SCOPE_LINKLOCAL(&sin6->sin6_addr) ||
IN6_IS_ADDR_MC_INTFACELOCAL(&sin6->sin6_addr))) { IN6_IS_ADDR_MC_INTFACELOCAL(&sin6->sin6_addr))) {
struct epoch_tracker et;
/* /*
* At this moment, we only check interface-local and * At this moment, we only check interface-local and
* link-local scope IDs, and use interface indices as the * link-local scope IDs, and use interface indices as the
* zone IDs assuming a one-to-one mapping between interfaces * zone IDs assuming a one-to-one mapping between interfaces
* and links. * 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); return (ENXIO);
}
NET_EPOCH_EXIT(et);
/* XXX assignment to 16bit from 32bit variable */ /* XXX assignment to 16bit from 32bit variable */
sin6->sin6_addr.s6_addr16[1] = htons(zoneid & 0xffff); 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]); zoneid = ntohs(sin6->sin6_addr.s6_addr16[1]);
if (zoneid) { if (zoneid) {
struct epoch_tracker et;
NET_EPOCH_ENTER(et);
/* sanity check */ /* sanity check */
if (V_if_index < zoneid) if (!ifnet_byindex(zoneid)) {
NET_EPOCH_EXIT(et);
return (ENXIO); return (ENXIO);
#if 0 }
/* XXX: Disabled due to possible deadlock. */ NET_EPOCH_EXIT(et);
if (!ifnet_byindex(zoneid))
return (ENXIO);
#endif
if (sin6->sin6_scope_id != 0 && if (sin6->sin6_scope_id != 0 &&
zoneid != sin6->sin6_scope_id) { zoneid != sin6->sin6_scope_id) {
log(LOG_NOTICE, log(LOG_NOTICE,