[ovs-dev] [PATCH] netdev-dpdk: fix incorrect shinfo initialization

yang_y_yi at 163.com yang_y_yi at 163.com
Wed Oct 14 07:22:48 UTC 2020


From: Yi Yang <yangyi01 at inspur.com>

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.

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 <olivier.matz at 6wind.com>
Signed-off-by: Olivier Matz <olivier.matz at 6wind.com>
Signed-off-by: Yi Yang <yangyi01 at inspur.com>
---
 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 0b830be..c7f9326 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2654,12 +2654,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);
@@ -2674,20 +2670,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,
-- 
1.8.3.1



More information about the dev mailing list