Hi, On Tue, Jan 19, 2016 at 04:31:56PM +0100, Mike Belopuhov wrote: > Hi, > > We've just run into a vmx panic and code inspection revealed > that my previous diff contained a mistake, the pullup operation > is called on a wrong mbuf chain. > > I apologize for overlooking this issue. > > We're not 100% certain that this fixes our exact problem yet > since we can't reproduce it at will, but the diff appears > correct to us. Please test and report any problems. > > Thanks! > > diff --git sys/dev/pci/if_vmx.c sys/dev/pci/if_vmx.c > index 2a3367c..7710987 100644 > --- sys/dev/pci/if_vmx.c > +++ sys/dev/pci/if_vmx.c > @@ -1121,11 +1121,11 @@ vmxnet3_load_mbuf(struct vmxnet3_softc *sc, struct > vmxnet3_txring *ring, > return (-1); > > ip = (struct ip *)(n->m_data + offp); > hlen += ip->ip_hl << 2; > > - *mp = m_pullup(m, hlen + csum_off + 2); > + *mp = m_pullup(n, hlen + csum_off + 2); > if (*mp == NULL) > return (-1); > m = *mp; > } > >
This doesn't look correct. As discussed with mikeb@ already, - n, as returned by m_pulldown(), can be a mbuf further down the chain. - *mp, as returned by m_pullup(), can be a new mbuf with n as a m_next pointer. - replacing the m pointer with *mp will a) leak the original m and b) seek to a new mbuf further down the chain, skipping the headers, and putting an mbuf with missing ethernet headers on the ring. The manpage doesn't explain the return values of m_pulldown() very well and it doesn't explain the return value of m_pullup() at all. We had to consult itojun's paper to verify what it does. Reyk
