Skip to contentSkip to conversation

memif: zero copy design issues

DPDK/ethdev Bug 2018 [email protected] 1 participant 1 message Aug 28

lore
Share & tools
lore
#0 lore raw

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: [email protected]
          Reporter: [email protected]

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.

Signature · 2 lines
You are receiving this mail because:
You are the assignee for the bug.