When deciding whether to call m_pullup() even though there is adequate
data in an mbuf, use M_WRITABLE() instead of a direct test of M_EXT; the latter both unnecessarily exposes mbuf-allocator internals in the protocol stack and is also insufficient to catch all cases of non-writability. (NB: m_pullup() does not actually guarantee that a writable mbuf is returned, so further refinement of all of these code paths continues to be required.) Reviewed by: bz MFC after: 3 days Sponsored by: EMC / Isilon Storage Division Differential Revision: https://reviews.freebsd.org/D900
This commit is contained in:
parent
75e1716807
commit
f0cace5d94
Notes:
svn2git
2020-12-20 02:59:44 +00:00
svn path=/head/; revision=272984
@ -1466,7 +1466,7 @@ igmp_input(struct mbuf **mp, int *offp, int proto)
|
||||
minlen += IGMP_V3_QUERY_MINLEN;
|
||||
else
|
||||
minlen += IGMP_MINLEN;
|
||||
if ((m->m_flags & M_EXT || m->m_len < minlen) &&
|
||||
if ((!M_WRITABLE(m) || m->m_len < minlen) &&
|
||||
(m = m_pullup(m, minlen)) == 0) {
|
||||
IGMPSTAT_INC(igps_rcv_tooshort);
|
||||
return (IPPROTO_DONE);
|
||||
@ -1557,7 +1557,7 @@ igmp_input(struct mbuf **mp, int *offp, int proto)
|
||||
*/
|
||||
igmpv3len = iphlen + IGMP_V3_QUERY_MINLEN +
|
||||
srclen;
|
||||
if ((m->m_flags & M_EXT ||
|
||||
if ((!M_WRITABLE(m) ||
|
||||
m->m_len < igmpv3len) &&
|
||||
(m = m_pullup(m, igmpv3len)) == NULL) {
|
||||
IGMPSTAT_INC(igps_rcv_tooshort);
|
||||
|
@ -121,7 +121,6 @@ __FBSDID("$FreeBSD$");
|
||||
#endif
|
||||
|
||||
#define VIFI_INVALID ((vifi_t) -1)
|
||||
#define M_HASCL(m) ((m)->m_flags & M_EXT)
|
||||
|
||||
static VNET_DEFINE(uint32_t, last_tv_sec); /* last time we processed this */
|
||||
#define V_last_tv_sec VNET(last_tv_sec)
|
||||
@ -1304,7 +1303,7 @@ X_ip_mforward(struct ip *ip, struct ifnet *ifp, struct mbuf *m,
|
||||
}
|
||||
|
||||
mb0 = m_copypacket(m, M_NOWAIT);
|
||||
if (mb0 && (M_HASCL(mb0) || mb0->m_len < hlen))
|
||||
if (mb0 && (!M_WRITABLE(mb0) || mb0->m_len < hlen))
|
||||
mb0 = m_pullup(mb0, hlen);
|
||||
if (mb0 == NULL) {
|
||||
free(rte, M_MRTABLE);
|
||||
@ -1544,7 +1543,7 @@ ip_mdq(struct mbuf *m, struct ifnet *ifp, struct mfc *rt, vifi_t xmt_vif)
|
||||
int hlen = ip->ip_hl << 2;
|
||||
struct mbuf *mm = m_copy(m, 0, hlen);
|
||||
|
||||
if (mm && (M_HASCL(mm) || mm->m_len < hlen))
|
||||
if (mm && (!M_WRITABLE(mm) || mm->m_len < hlen))
|
||||
mm = m_pullup(mm, hlen);
|
||||
if (mm == NULL)
|
||||
return ENOBUFS;
|
||||
@ -1665,7 +1664,7 @@ phyint_send(struct ip *ip, struct vif *vifp, struct mbuf *m)
|
||||
* so that ip_output() only scribbles on the copy.
|
||||
*/
|
||||
mb_copy = m_copypacket(m, M_NOWAIT);
|
||||
if (mb_copy && (M_HASCL(mb_copy) || mb_copy->m_len < hlen))
|
||||
if (mb_copy && (!M_WRITABLE(mb_copy) || mb_copy->m_len < hlen))
|
||||
mb_copy = m_pullup(mb_copy, hlen);
|
||||
if (mb_copy == NULL)
|
||||
return;
|
||||
|
@ -1365,7 +1365,7 @@ ip_mloopback(struct ifnet *ifp, struct mbuf *m, struct sockaddr_in *dst,
|
||||
* modify the pack in order to generate checksums.
|
||||
*/
|
||||
copym = m_dup(m, M_NOWAIT);
|
||||
if (copym != NULL && (copym->m_flags & M_EXT || copym->m_len < hlen))
|
||||
if (copym != NULL && (!M_WRITABLE(copym) || copym->m_len < hlen))
|
||||
copym = m_pullup(copym, hlen);
|
||||
if (copym != NULL) {
|
||||
/* If needed, compute the checksum and mark it as valid. */
|
||||
|
@ -63,6 +63,8 @@
|
||||
#include <sys/cdefs.h>
|
||||
__FBSDID("$FreeBSD$");
|
||||
|
||||
#define MBUF_PRIVATE /* XXXRW: Optimisation tries to avoid M_EXT mbufs */
|
||||
|
||||
#include "opt_inet.h"
|
||||
#include "opt_inet6.h"
|
||||
#include "opt_ipsec.h"
|
||||
@ -581,7 +583,7 @@ icmp6_input(struct mbuf **mp, int *offp, int proto)
|
||||
/* Give up remote */
|
||||
break;
|
||||
}
|
||||
if ((n->m_flags & M_EXT) != 0
|
||||
if (!M_WRITABLE(n)
|
||||
|| n->m_len < off + sizeof(struct icmp6_hdr)) {
|
||||
struct mbuf *n0 = n;
|
||||
int n0len;
|
||||
|
@ -126,9 +126,6 @@ __FBSDID("$FreeBSD$");
|
||||
|
||||
static MALLOC_DEFINE(M_MRTABLE6, "mf6c", "multicast forwarding cache entry");
|
||||
|
||||
/* XXX: this is a very common idiom; move to <sys/mbuf.h> ? */
|
||||
#define M_HASCL(m) ((m)->m_flags & M_EXT)
|
||||
|
||||
static int ip6_mdq(struct mbuf *, struct ifnet *, struct mf6c *);
|
||||
static void phyint_send(struct ip6_hdr *, struct mif6 *, struct mbuf *);
|
||||
static int register_send(struct ip6_hdr *, struct mif6 *, struct mbuf *);
|
||||
@ -1128,7 +1125,7 @@ X_ip6_mforward(struct ip6_hdr *ip6, struct ifnet *ifp, struct mbuf *m)
|
||||
* Pullup packet header if needed before storing it,
|
||||
* as other references may modify it in the meantime.
|
||||
*/
|
||||
if (mb0 && (M_HASCL(mb0) || mb0->m_len < sizeof(struct ip6_hdr)))
|
||||
if (mb0 && (!M_WRITABLE(mb0) || mb0->m_len < sizeof(struct ip6_hdr)))
|
||||
mb0 = m_pullup(mb0, sizeof(struct ip6_hdr));
|
||||
if (mb0 == NULL) {
|
||||
free(rte, M_MRTABLE6);
|
||||
@ -1397,7 +1394,7 @@ ip6_mdq(struct mbuf *m, struct ifnet *ifp, struct mf6c *rt)
|
||||
|
||||
mm = m_copy(m, 0, sizeof(struct ip6_hdr));
|
||||
if (mm &&
|
||||
(M_HASCL(mm) ||
|
||||
(!M_WRITABLE(mm) ||
|
||||
mm->m_len < sizeof(struct ip6_hdr)))
|
||||
mm = m_pullup(mm, sizeof(struct ip6_hdr));
|
||||
if (mm == NULL)
|
||||
@ -1527,7 +1524,7 @@ phyint_send(struct ip6_hdr *ip6, struct mif6 *mifp, struct mbuf *m)
|
||||
*/
|
||||
mb_copy = m_copy(m, 0, M_COPYALL);
|
||||
if (mb_copy &&
|
||||
(M_HASCL(mb_copy) || mb_copy->m_len < sizeof(struct ip6_hdr)))
|
||||
(!M_WRITABLE(mb_copy) || mb_copy->m_len < sizeof(struct ip6_hdr)))
|
||||
mb_copy = m_pullup(mb_copy, sizeof(struct ip6_hdr));
|
||||
if (mb_copy == NULL) {
|
||||
return;
|
||||
|
@ -1196,7 +1196,7 @@ ip6_insertfraghdr(struct mbuf *m0, struct mbuf *m, int hlen,
|
||||
for (mlast = n; mlast->m_next; mlast = mlast->m_next)
|
||||
;
|
||||
|
||||
if ((mlast->m_flags & M_EXT) == 0 &&
|
||||
if (M_WRITABLE(mlast) &&
|
||||
M_TRAILINGSPACE(mlast) >= sizeof(struct ip6_frag)) {
|
||||
/* use the trailing space of the last mbuf for the fragment hdr */
|
||||
*frghdrp = (struct ip6_frag *)(mtod(mlast, caddr_t) +
|
||||
@ -2918,7 +2918,7 @@ ip6_mloopback(struct ifnet *ifp, struct mbuf *m, struct sockaddr_in6 *dst)
|
||||
* is in an mbuf cluster, so that we can safely override the IPv6
|
||||
* header portion later.
|
||||
*/
|
||||
if ((copym->m_flags & M_EXT) != 0 ||
|
||||
if (!M_WRITABLE(copym) ||
|
||||
copym->m_len < sizeof(struct ip6_hdr)) {
|
||||
copym = m_pullup(copym, sizeof(struct ip6_hdr));
|
||||
if (copym == NULL)
|
||||
|
Loading…
Reference in New Issue
Block a user