commit 95e854e053733cf905e7bc713470b09b4a6d74bc Author: Ryota Ozaki Date: Fri Feb 3 12:10:27 2017 +0900 Fix m_get_rcvif and m_put_rcvif diff --git a/sys/net/if_pppoe.c b/sys/net/if_pppoe.c index ab89469..51a9577 100644 --- a/sys/net/if_pppoe.c +++ b/sys/net/if_pppoe.c @@ -55,6 +55,7 @@ __KERNEL_RCSID(0, "$NetBSD: if_pppoe.c,v 1.124 2017/02/01 17:58:47 maxv Exp $"); #include #include #include +#include #include #include @@ -640,7 +641,8 @@ pppoe_dispatch_disc_pkt(struct mbuf *m, int off) break; /* ignored */ case PPPOE_TAG_HUNIQUE: { struct ifnet *rcvif; - int s; + struct psref psref; + if (sc != NULL) break; n = m_pulldown(m, off + sizeof(*pt), len, &noff); @@ -653,10 +655,12 @@ pppoe_dispatch_disc_pkt(struct mbuf *m, int off) hunique = mtod(n, uint8_t *) + noff; hunique_len = len; #endif - rcvif = m_get_rcvif(m, &s); - sc = pppoe_find_softc_by_hunique(mtod(n, char *) + noff, - len, rcvif, RW_READER); - m_put_rcvif(rcvif, &s); + rcvif = m_get_rcvif_psref(m, &psref); + if (rcvif != NULL) { + sc = pppoe_find_softc_by_hunique(mtod(n, char *) + noff, + len, rcvif, RW_READER); + } + m_put_rcvif_psref(rcvif, &psref); if (sc != NULL) { strlcpy(devname, sc->sc_sppp.pp_if.if_xname, sizeof(devname)); @@ -786,6 +790,9 @@ breakbreak:; #endif /* PPPOE_SERVER */ case PPPOE_CODE_PADR: #ifdef PPPOE_SERVER + { + struct ifnet *rcvif; + struct psref psref; /* * get sc from ac_cookie if IFF_PASSIVE */ @@ -794,10 +801,15 @@ breakbreak:; printf("pppoe: received PADR but not includes ac_cookie\n"); goto done; } - sc = pppoe_find_softc_by_hunique(ac_cookie, - ac_cookie_len, - m_get_rcvif_NOMPSAFE(m), - RW_WRITER); + + rcvif = m_get_rcvif_psref(m, &psref); + if (__predict_true(rcvif != NULL)) { + sc = pppoe_find_softc_by_hunique(ac_cookie, + ac_cookie_len, + rcvif, + RW_WRITER); + } + m_put_rcvif_psref(rcvif, &psref); if (sc == NULL) { /* be quiet if there is not a single pppoe instance */ if (!LIST_EMPTY(&pppoe_softc_list)) @@ -835,6 +847,7 @@ breakbreak:; sc->sc_sppp.pp_up(&sc->sc_sppp); sppp_lock_exit(&sc->sc_sppp); break; + } #else /* ignore, we are no access concentrator */ goto done; @@ -935,11 +948,14 @@ breakbreak:; break; case PPPOE_CODE_PADT: { struct ifnet *rcvif; - int s; + struct psref psref; - rcvif = m_get_rcvif(m, &s); - sc = pppoe_find_softc_by_session(session, rcvif, RW_WRITER); - m_put_rcvif(rcvif, &s); + rcvif = m_get_rcvif_psref(m, &psref); + if (__predict_true(rcvif != NULL)) { + sc = pppoe_find_softc_by_session(session, rcvif, + RW_WRITER); + } + m_put_rcvif_psref(rcvif, &psref); if (sc == NULL) goto done; diff --git a/sys/netinet/if_arp.c b/sys/netinet/if_arp.c index 8409648..29473e0 100644 --- a/sys/netinet/if_arp.c +++ b/sys/netinet/if_arp.c @@ -941,6 +941,10 @@ arpintr(void) goto badlen; rcvif = m_get_rcvif(m, &s); + if (__predict_false(rcvif == NULL)) { + ARP_STATINC(ARP_STAT_RCVNOINT); + goto free; + } switch (rcvif->if_type) { case IFT_IEEE1394: arplen = sizeof(struct arphdr) + @@ -967,6 +971,7 @@ arpintr(void) badlen: ARP_STATINC(ARP_STAT_RCVBADLEN); } +free: m_freem(m); } out: @@ -1312,10 +1317,15 @@ reply: struct llentry *lle = NULL; struct sockaddr_in sin; #if NCARP > 0 - struct ifnet *_rcvif = m_get_rcvif(m, &s); - if (ifp->if_type == IFT_CARP && _rcvif->if_type != IFT_CARP) - goto out; - m_put_rcvif(_rcvif, &s); + if (ifp->if_type == IFT_CARP) { + struct ifnet *_rcvif = m_get_rcvif(m, &s); + int iftype = 0; + if (__predict_true(_rcvif != NULL)) + iftype = _rcvif->if_type; + m_put_rcvif(_rcvif, &s); + if (iftype != IFT_CARP) + goto out; + } #endif tha = ar_tha(ah); @@ -1877,6 +1887,8 @@ in_revarpinput(struct mbuf *m) op = ntohs(ah->ar_op); rcvif = m_get_rcvif(m, &s); + if (__predict_false(rcvif == NULL)) + goto out; switch (rcvif->if_type) { case IFT_IEEE1394: /* ARP without target hardware address is not supported */ diff --git a/sys/netinet/ip_flow.c b/sys/netinet/ip_flow.c index a0ba0e2..cb8a055 100644 --- a/sys/netinet/ip_flow.c +++ b/sys/netinet/ip_flow.c @@ -247,6 +247,8 @@ ipflow_fastforward(struct mbuf *m) goto out; ifp = m_get_rcvif(m, &s); + if (__predict_false(ifp == NULL)) + goto out_unref; /* * Verify the IP header checksum. */ diff --git a/sys/netinet/ip_icmp.c b/sys/netinet/ip_icmp.c index fe12e32..79ec31c 100644 --- a/sys/netinet/ip_icmp.c +++ b/sys/netinet/ip_icmp.c @@ -562,7 +562,7 @@ _icmp_input(struct mbuf *m, int hlen, int proto) case ICMP_MASKREQ: { struct ifnet *rcvif; int s, ss; - struct ifaddr *ifa; + struct ifaddr *ifa = NULL; if (icmpmaskrepl == 0) break; @@ -581,7 +581,8 @@ _icmp_input(struct mbuf *m, int hlen, int proto) icmpdst.sin_addr = ip->ip_dst; ss = pserialize_read_enter(); rcvif = m_get_rcvif(m, &s); - ifa = ifaof_ifpforaddr(sintosa(&icmpdst), rcvif); + if (__predict_true(rcvif != NULL)) + ifa = ifaof_ifpforaddr(sintosa(&icmpdst), rcvif); m_put_rcvif(rcvif, &s); if (ifa == NULL) { pserialize_read_exit(ss); diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c index 4e9231d..07e0a53 100644 --- a/sys/netinet/ip_input.c +++ b/sys/netinet/ip_input.c @@ -908,7 +908,7 @@ ip_dooptions(struct mbuf *m) int opt, optlen, cnt, off, code, type = ICMP_PARAMPROB, forward = 0; struct in_addr dst; n_time ntime; - struct ifaddr *ifa; + struct ifaddr *ifa = NULL; int s; dst = ip->ip_dst; @@ -1098,7 +1098,10 @@ ip_dooptions(struct mbuf *m) ipaddr.sin_addr = dst; _ss = pserialize_read_enter(); rcvif = m_get_rcvif(m, &_s); - ifa = ifaof_ifpforaddr(sintosa(&ipaddr), rcvif); + if (__predict_true(rcvif != NULL)) { + ifa = ifaof_ifpforaddr(sintosa(&ipaddr), + rcvif); + } m_put_rcvif(rcvif, &_s); if (ifa == NULL) { pserialize_read_exit(_ss); diff --git a/sys/netinet/tcp_input.c b/sys/netinet/tcp_input.c index 9771214..49ce77d 100644 --- a/sys/netinet/tcp_input.c +++ b/sys/netinet/tcp_input.c @@ -904,6 +904,8 @@ tcp_input_checksum(int af, struct mbuf *m, const struct tcphdr *th, */ rcvif = m_get_rcvif(m, &s); + if (__predict_false(rcvif == NULL)) + goto badcsum; /* XXX */ switch (af) { #ifdef INET diff --git a/sys/netinet6/icmp6.c b/sys/netinet6/icmp6.c index f274dfd..b589311 100644 --- a/sys/netinet6/icmp6.c +++ b/sys/netinet6/icmp6.c @@ -1067,6 +1067,8 @@ icmp6_notify_error(struct mbuf *m, int off, int icmp6len, int code) eip6 = (struct ip6_hdr *)(icmp6 + 1); rcvif = m_get_rcvif(m, &s); + if (__predict_false(rcvif == NULL)) + goto freeit; sockaddr_in6_init(&icmp6dst, (finaldst == NULL) ? &eip6->ip6_dst : finaldst, 0, 0, 0); if (in6_setscope(&icmp6dst.sin6_addr, rcvif, NULL)) { @@ -1164,6 +1166,8 @@ icmp6_mtudisc_update(struct ip6ctlparam *ip6cp, int validated) sin6.sin6_len = sizeof(struct sockaddr_in6); sin6.sin6_addr = *dst; rcvif = m_get_rcvif(m, &s); + if (__predict_false(rcvif == NULL)) + return; if (in6_setscope(&sin6.sin6_addr, rcvif, NULL)) { m_put_rcvif(rcvif, &s); return; @@ -1307,6 +1311,8 @@ ni6_input(struct mbuf *m, int off) m_copydata(m, off + sizeof(struct icmp6_nodeinfo), subjlen, (void *)&in6_subj); rcvif = m_get_rcvif(m, &s); + if (__predict_false(rcvif == NULL)) + goto bad; if (in6_setscope(&in6_subj, rcvif, NULL)) { m_put_rcvif(rcvif, &s); goto bad; diff --git a/sys/netinet6/mld6.c b/sys/netinet6/mld6.c index f76ee20..d2ff7a5 100644 --- a/sys/netinet6/mld6.c +++ b/sys/netinet6/mld6.c @@ -345,6 +345,8 @@ mld_input(struct mbuf *m, int off) int s; ifp = m_get_rcvif(m, &s); + if (__predict_false(ifp == NULL)) + goto out; IP6_EXTHDR_GET(mldh, struct mld_hdr *, m, off, sizeof(*mldh)); if (mldh == NULL) { ICMP6_STATINC(ICMP6_STAT_TOOSHORT); diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h index e085c27..2517b04 100644 --- a/sys/sys/mbuf.h +++ b/sys/sys/mbuf.h @@ -1006,16 +1006,23 @@ void m_print(const struct mbuf *, const char *, void (*)(const char *, ...) /* * Get rcvif of a mbuf. * - * The caller must call m_put_rcvif after using rcvif. The caller cannot - * block or sleep during using rcvif. Insofar as the constraint is satisfied, - * the API ensures a got rcvif isn't be freed until m_put_rcvif is called. + * The caller must call m_put_rcvif after using rcvif if the returned rcvif + * isn't NULL. If the return rcvif is NULL, the caller doesn't need to call + * m_put_rcvif (calling it is safe). The caller cannot block or sleep during + * using rcvif. Insofar as the constraint is satisfied, the API ensures a got + * rcvif isn't be freed until m_put_rcvif is called. */ static __inline struct ifnet * m_get_rcvif(const struct mbuf *m, int *s) { + struct ifnet *ifp; *s = pserialize_read_enter(); - return if_byindex(m->m_pkthdr.rcvif_index); + ifp = if_byindex(m->m_pkthdr.rcvif_index); + if (__predict_false(ifp == NULL)) + pserialize_read_exit(*s); + + return ifp; } static __inline void