On Tue, Oct 27, 2020 at 01:47:22PM +0100, Ilya Maximets wrote:
> On 10/27/20 12:34 PM, Flavio Leitner wrote:
> > On Wed, Oct 14, 2020 at 03:22:48PM +0800, [email protected] wrote:
> >> From: Yi Yang <[email protected]>
> >>
> >> shinfo is used to store reference counter and free callback
> >> of an external buffer, but it is stored in mbuf if the mbuf
> >> has tailroom for it.
> >>
> >> This is wrong because the mbuf (and its data) can be freed
> >> before the external buffer, for example:
> >>
> >>   pkt2 = rte_pktmbuf_alloc(mp);
> >>   rte_pktmbuf_attach(pkt2, pkt);
> >>   rte_pktmbuf_free(pkt);
> 
> How is that possible with OVS?  Right now OVS doesn't support multi-segement
> mbufs and will, likely, not support them in a near future because it requires
> changes all other the codebase.
> 
> Is there any other scenario that could lead to issues with current OVS
> implementation?

This is copying packets. The shinfo is allocated in the mbuf of
the first packet which could be deleted without any references
to the external buffer still using it.

fbl


> 
> >>
> >> After this, pkt is freed, but it still contains shinfo, which
> >> is referenced by pkt2.
> >>
> >> Fix this by always storing shinfo at the tail of external buffer.
> >>
> >> Fixes: 29cf9c1b3b9c ("userspace: Add TCP Segmentation Offload support")
> >> Co-authored-by: Olivier Matz <[email protected]>
> >> Signed-off-by: Olivier Matz <[email protected]>
> >> Signed-off-by: Yi Yang <[email protected]>
> >> ---
> > 
> > Acked-by: Flavio Leitner <[email protected]>
> > 
> > Thanks Yi,
> > fbl
> > 
> 

-- 
fbl
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to