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
