vhost: fix external mbuf creation

In virtio_dev_extbuf_alloc(), the shinfo structure used to store
the reference counter and the free callback of the external buffer
is by default stored inside the mbuf data.

This is wrong because the mbuf (and its data) can be freed before
the external buffer, for instance in the following situation:

  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 the shinfo beside the external buffer.

Fixes: c3ff0ac70acb ("vhost: improve performance by supporting large buffer")
Cc: stable@dpdk.org
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 0a0bea1..008f5ce 100644 (file)
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -2098,16 +2098,8 @@ virtio_dev_extbuf_alloc(struct rte_mbuf *pkt, uint32_t size)
        rte_iova_t iova;
        void *buf;
 
-       /* Try to use pkt buffer to store shinfo to reduce the amount of memory
-        * required, otherwise store shinfo in the new buffer.
-        */
-       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 (unlikely(total_len > UINT16_MAX))
                return -ENOSPC;
@@ -2118,18 +2110,12 @@ virtio_dev_extbuf_alloc(struct rte_mbuf *pkt, uint32_t size)
                return -ENOMEM;
 
        /* Initialize shinfo */
-       if (shinfo) {
-               shinfo->free_cb = virtio_dev_extbuf_free;
-               shinfo->fcb_opaque = buf;
-               rte_mbuf_ext_refcnt_set(shinfo, 1);
-       } else {
-               shinfo = rte_pktmbuf_ext_shinfo_init_helper(buf, &buf_len,
-                                             virtio_dev_extbuf_free, buf);
-               if (unlikely(shinfo == NULL)) {
-                       rte_free(buf);
-                       VHOST_LOG_DATA(ERR, "Failed to init shinfo\n");
-                       return -1;
-               }
+       shinfo = rte_pktmbuf_ext_shinfo_init_helper(buf, &buf_len,
+                                               virtio_dev_extbuf_free, buf);
+       if (unlikely(shinfo == NULL)) {
+               rte_free(buf);
+               VHOST_LOG_DATA(ERR, "Failed to init shinfo\n");
+               return -1;
        }
 
        iova = rte_malloc_virt2iova(buf);