[DPDK/ethdev Bug 2018] memif: zero copy design issues

[bugzilla@dpdk.org]

https://bugs.dpdk.org/show_bug.cgi?id=2018

            Bug ID: 2018
           Summary: memif: zero copy design issues
           Product: DPDK
           Version: 26.11
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: major
          Priority: Normal
         Component: ethdev
          Assignee: dev@dpdk.org
          Reporter: stephen@networkplumber.org
  Target Milestone: ---
             Group: security

Both VPP and DPDK have this bug found by AI assisted review of memif.

In eth_memif_rx_zc() in drivers/net/memif/rte_eth_memif.c the peer's
descriptor length is written straight into the mbuf:

    rte_pktmbuf_data_len(mbuf) = d0->length;
    rte_pktmbuf_pkt_len(mbuf) = rte_pktmbuf_data_len(mbuf);

with no check against the mbuf data room. Chained segments accumulate
the same unchecked value into the head mbuf's pkt_len through
memif_pktmbuf_chain(). The copy-mode path has the same problem with the
same field, filed separately.

The mbufs here belong to the local side, so the driver itself does not
read out of bounds. The consequence is downstream: every consumer of
data_len and pkt_len treats them as valid. An application that does
rte_pktmbuf_mtod() and reads pkt_len bytes, or transmits the mbuf on a
real NIC that DMAs pkt_len bytes, reads past the end of the mbuf data
area and into adjacent objects in the same mempool. Those adjacent
objects hold other packets, so the result can be an information leak
onto the wire.

Zero-copy is client-role only, so the peer supplying these lengths is
the server. Under the memif trust model the client trusts the server,
so this is not a trust boundary violation in a correct deployment. It
is still worth fixing:

  - the trust model is not documented, so users cannot currently know
    which deployments are correct;
  - a zero-copy client exposes considerably more than its packet pool.
    memif_init_regions_and_queues() walks every memseg list and shares
    each one as a region ("Zero-copy exposes dpdk memory"), so the peer
    sees the whole of the local DPDK memory, not just the mbufs it is
    meant to fill. That raises the value of getting the receive path
    right even against a peer that is only buggy rather than hostile;
  - this path has a history of memory-safety defects, including a
    zero-copy Rx overflow fixed in 2024.

Suggested fix
-------------

Bound the length by rte_pktmbuf_data_room_size(mq->mempool) minus the
headroom before assigning it, taking the value from a private snapshot
of the descriptor so the peer cannot change it between check and use.

Prefer dropping the buffer chain and counting an xstat over silently
clamping. A clamp keeps a malformed packet in flight with a plausible
length, which is harder to diagnose than a counted drop, and it would
leave the zero-copy path behaving differently from the copy path for
the same peer. Whichever is chosen, the two paths should agree.

-- 
You are receiving this mail because:
You are the assignee for the bug.