From 5d691e6da82888221a9e142fdbc3650ec9f17de4 Mon Sep 17 00:00:00 2001 From: Andre Oppermann Date: Wed, 18 Jan 2006 14:24:39 +0000 Subject: Return mbuf pointer or NULL from ip_fastforward() as the mbuf pointer may have changed by m_pullup() during fastforward processing. While this is a bug it is actually never triggered in real world situations and it is not remotely exploitable. Found by: Coverity Prevent(tm) Coverity ID: CID780 Sponsored by: TCP/IP Optimization Fundraise 2005 --- sys/net/if_arcsubr.c | 4 ++-- sys/net/if_ef.c | 2 +- sys/net/if_ethersubr.c | 2 +- sys/net/if_fddisubr.c | 2 +- sys/net/if_fwsubr.c | 2 +- sys/net/if_iso88025subr.c | 2 +- sys/net/if_ppp.c | 2 +- sys/netinet/in_var.h | 2 +- sys/netinet/ip_fastfwd.c | 36 ++++++++++++++++++------------------ 9 files changed, 27 insertions(+), 27 deletions(-) diff --git a/sys/net/if_arcsubr.c b/sys/net/if_arcsubr.c index 6cdb951fb627..f3c00cde1c15 100644 --- a/sys/net/if_arcsubr.c +++ b/sys/net/if_arcsubr.c @@ -559,14 +559,14 @@ arc_input(ifp, m) #ifdef INET case ARCTYPE_IP: m_adj(m, ARC_HDRNEWLEN); - if (ip_fastforward(m)) + if ((m = ip_fastforward(m)) == NULL) return; isr = NETISR_IP; break; case ARCTYPE_IP_OLD: m_adj(m, ARC_HDRLEN); - if (ip_fastforward(m)) + if ((m = ip_fastforward(m)) == NULL) return; isr = NETISR_IP; break; diff --git a/sys/net/if_ef.c b/sys/net/if_ef.c index 04262c6fa9a6..19c16874c28d 100644 --- a/sys/net/if_ef.c +++ b/sys/net/if_ef.c @@ -247,7 +247,7 @@ ef_inputEII(struct mbuf *m, struct ether_header *eh, u_short ether_type) #endif #ifdef INET case ETHERTYPE_IP: - if (ip_fastforward(m)) + if ((m = ip_fastforward(m)) == NULL) return (0); isr = NETISR_IP; break; diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c index d96b48bfa431..9a5ffc3055c2 100644 --- a/sys/net/if_ethersubr.c +++ b/sys/net/if_ethersubr.c @@ -741,7 +741,7 @@ post_stats: switch (ether_type) { #ifdef INET case ETHERTYPE_IP: - if (ip_fastforward(m)) + if ((m = ip_fastforward(m)) == NULL) return; isr = NETISR_IP; break; diff --git a/sys/net/if_fddisubr.c b/sys/net/if_fddisubr.c index d7c3e33b6afc..cc4b1fce46e9 100644 --- a/sys/net/if_fddisubr.c +++ b/sys/net/if_fddisubr.c @@ -495,7 +495,7 @@ fddi_input(ifp, m) switch (type) { #ifdef INET case ETHERTYPE_IP: - if (ip_fastforward(m)) + if ((m = ip_fastforward(m)) == NULL) return; isr = NETISR_IP; break; diff --git a/sys/net/if_fwsubr.c b/sys/net/if_fwsubr.c index 4846b27df87c..7cf1c220782d 100644 --- a/sys/net/if_fwsubr.c +++ b/sys/net/if_fwsubr.c @@ -601,7 +601,7 @@ firewire_input(struct ifnet *ifp, struct mbuf *m, uint16_t src) switch (type) { #ifdef INET case ETHERTYPE_IP: - if (ip_fastforward(m)) + if ((m = ip_fastforward(m)) == NULL) return; isr = NETISR_IP; break; diff --git a/sys/net/if_iso88025subr.c b/sys/net/if_iso88025subr.c index 5d5a3a284133..14c6f983ccd8 100644 --- a/sys/net/if_iso88025subr.c +++ b/sys/net/if_iso88025subr.c @@ -583,7 +583,7 @@ iso88025_input(ifp, m) #ifdef INET case ETHERTYPE_IP: th->iso88025_shost[0] &= ~(TR_RII); - if (ip_fastforward(m)) + if ((m = ip_fastforward(m)) == NULL) return; isr = NETISR_IP; break; diff --git a/sys/net/if_ppp.c b/sys/net/if_ppp.c index 7bf26f387f83..4d410d408bd3 100644 --- a/sys/net/if_ppp.c +++ b/sys/net/if_ppp.c @@ -1570,7 +1570,7 @@ ppp_inproc(sc, m) m->m_pkthdr.len -= PPP_HDRLEN; m->m_data += PPP_HDRLEN; m->m_len -= PPP_HDRLEN; - if (ip_fastforward(m)) + if ((m = ip_fastforward(m)) == NULL) return; isr = NETISR_IP; break; diff --git a/sys/netinet/in_var.h b/sys/netinet/in_var.h index 5792a0bd007a..319b0dc5519b 100644 --- a/sys/netinet/in_var.h +++ b/sys/netinet/in_var.h @@ -242,7 +242,7 @@ void in_rtqdrain(void); void ip_input(struct mbuf *); int in_ifadown(struct ifaddr *ifa, int); void in_ifscrub(struct ifnet *, struct in_ifaddr *); -int ip_fastforward(struct mbuf *); +struct mbuf *ip_fastforward(struct mbuf *); #endif /* _KERNEL */ diff --git a/sys/netinet/ip_fastfwd.c b/sys/netinet/ip_fastfwd.c index 43419f3c5fe2..6ad8e9d85fe6 100644 --- a/sys/netinet/ip_fastfwd.c +++ b/sys/netinet/ip_fastfwd.c @@ -151,7 +151,7 @@ ip_findroute(struct route *ro, struct in_addr dest, struct mbuf *m) * otherwise 0 is returned and the packet should be delivered * to ip_input for full processing. */ -int +struct mbuf * ip_fastforward(struct mbuf *m) { struct ip *ip; @@ -171,7 +171,7 @@ ip_fastforward(struct mbuf *m) * Are we active and forwarding packets? */ if (!ipfastforward_active || !ipforwarding) - return 0; + return m; M_ASSERTVALID(m); M_ASSERTPKTHDR(m); @@ -196,7 +196,7 @@ ip_fastforward(struct mbuf *m) if (m->m_len < sizeof (struct ip) && (m = m_pullup(m, sizeof (struct ip))) == NULL) { ipstat.ips_toosmall++; - return 1; /* mbuf already free'd */ + return NULL; /* mbuf already free'd */ } ip = mtod(m, struct ip *); @@ -218,9 +218,9 @@ ip_fastforward(struct mbuf *m) goto drop; } if (hlen > m->m_len) { - if ((m = m_pullup(m, hlen)) == 0) { + if ((m = m_pullup(m, hlen)) == NULL) { ipstat.ips_badhlen++; - return 1; + return NULL; /* mbuf already free'd */ } ip = mtod(m, struct ip *); } @@ -281,7 +281,7 @@ ip_fastforward(struct mbuf *m) * Is packet dropped by traffic conditioner? */ if (altq_input != NULL && (*altq_input)(m, AF_INET) == 0) - return 1; + goto drop; #endif /* @@ -293,11 +293,11 @@ ip_fastforward(struct mbuf *m) */ if (ip->ip_hl != (sizeof(struct ip) >> 2)) { if (ip_doopts == 1) - return 0; + return m; else if (ip_doopts == 2) { icmp_error(m, ICMP_UNREACH, ICMP_UNREACH_FILTER_PROHIB, 0, 0); - return 1; + return NULL; /* mbuf already free'd */ } /* else ignore IP options and continue */ } @@ -320,13 +320,13 @@ ip_fastforward(struct mbuf *m) IN_MULTICAST(ntohl(ip->ip_dst.s_addr)) || ip->ip_src.s_addr == INADDR_ANY || ip->ip_dst.s_addr == INADDR_ANY ) - return 0; + return m; /* * Is it for a local address on this host? */ if (in_localip(ip->ip_dst)) - return 0; + return m; ipstat.ips_total++; @@ -350,7 +350,7 @@ ip_fastforward(struct mbuf *m) if (pfil_run_hooks(&inet_pfil_hook, &m, m->m_pkthdr.rcvif, PFIL_IN, NULL) || m == NULL) - return 1; + goto drop; M_ASSERTVALID(m); M_ASSERTPKTHDR(m); @@ -393,7 +393,7 @@ passin: #endif if (ip->ip_ttl <= IPTTLDEC) { icmp_error(m, ICMP_TIMXCEED, ICMP_TIMXCEED_INTRANS, 0, 0); - return 1; + return NULL; /* mbuf already free'd */ } /* @@ -414,7 +414,7 @@ passin: * Find route to destination. */ if ((dst = ip_findroute(&ro, dest, m)) == NULL) - return 1; /* icmp unreach already sent */ + return NULL; /* icmp unreach already sent */ ifp = ro.ro_rt->rt_ifp; /* @@ -434,7 +434,7 @@ passin: goto passout; if (pfil_run_hooks(&inet_pfil_hook, &m, ifp, PFIL_OUT, NULL) || m == NULL) { - goto consumed; + goto drop; } M_ASSERTVALID(m); @@ -469,7 +469,7 @@ forwardlocal: m->m_flags |= M_FASTFWD_OURS; if (ro.ro_rt) RTFREE(ro.ro_rt); - return 0; + return m; } /* * Redo route lookup with new destination address @@ -483,7 +483,7 @@ forwardlocal: #endif /* IPFIREWALL_FORWARD */ RTFREE(ro.ro_rt); if ((dst = ip_findroute(&ro, dest, m)) == NULL) - return 1; /* icmp unreach already sent */ + return NULL; /* icmp unreach already sent */ ifp = ro.ro_rt->rt_ifp; } @@ -597,11 +597,11 @@ passout: } consumed: RTFREE(ro.ro_rt); - return 1; + return NULL; drop: if (m) m_freem(m); if (ro.ro_rt) RTFREE(ro.ro_rt); - return 1; + return NULL; } -- cgit v1.2.3