From c17f32a11c94daafe48a608e66db44df24c43779 Mon Sep 17 00:00:00 2001 From: Yi Yang Date: Wed, 14 Oct 2020 15:22:48 +0800 Subject: [PATCH] netdev-dpdk: Fix incorrect shinfo initialization. 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); After this, pkt is freed, but it still contains shinfo, which is referenced by pkt2. This sequence of operations is possible inside DPDK e.g., while performing TSO operations for 'net_tap' PMD. 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 Signed-off-by: Olivier Matz Signed-off-by: Yi Yang Acked-by: Flavio Leitner Signed-off-by: Ilya Maximets --- lib/netdev-dpdk.c | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 2640a421a..9d8096668 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -2683,12 +2683,8 @@ dpdk_pktmbuf_attach_extbuf(struct rte_mbuf *pkt, uint32_t data_len) uint16_t buf_len; void *buf; - if (rte_pktmbuf_tailroom(pkt) >= sizeof *shinfo) { - shinfo = rte_pktmbuf_mtod(pkt, struct rte_mbuf_ext_shared_info *); - } else { - total_len += sizeof *shinfo + sizeof(uintptr_t); - total_len = RTE_ALIGN_CEIL(total_len, sizeof(uintptr_t)); - } + total_len += sizeof *shinfo + sizeof(uintptr_t); + total_len = RTE_ALIGN_CEIL(total_len, sizeof(uintptr_t)); if (OVS_UNLIKELY(total_len > UINT16_MAX)) { VLOG_ERR("Can't copy packet: too big %u", total_len); @@ -2703,20 +2699,14 @@ dpdk_pktmbuf_attach_extbuf(struct rte_mbuf *pkt, uint32_t data_len) } /* Initialize shinfo. */ - if (shinfo) { - shinfo->free_cb = netdev_dpdk_extbuf_free; - shinfo->fcb_opaque = buf; - rte_mbuf_ext_refcnt_set(shinfo, 1); - } else { - shinfo = rte_pktmbuf_ext_shinfo_init_helper(buf, &buf_len, - netdev_dpdk_extbuf_free, - buf); - if (OVS_UNLIKELY(shinfo == NULL)) { - rte_free(buf); - VLOG_ERR("Failed to initialize shared info for mbuf while " - "attempting to attach an external buffer."); - return NULL; - } + shinfo = rte_pktmbuf_ext_shinfo_init_helper(buf, &buf_len, + netdev_dpdk_extbuf_free, + buf); + if (OVS_UNLIKELY(shinfo == NULL)) { + rte_free(buf); + VLOG_ERR("Failed to initialize shared info for mbuf while " + "attempting to attach an external buffer."); + return NULL; } rte_pktmbuf_attach_extbuf(pkt, buf, rte_malloc_virt2iova(buf), buf_len,