http://bugs.dpdk.org/show_bug.cgi?id=1884
Bug ID: 1884
Summary: vhost: fix tx_burst return value after VLAN insertion
failure
Product: DPDK
Version: 25.11
Hardware: All
OS: All
Status: UNCONFIRMED
Severity: normal
Priority: Normal
Component: vhost/virtio
Assignee: [email protected]
Reporter: [email protected]
Target Milestone: ---
Found while auditing tx_burst semantics of drivers.
The eth_vhost_tx() function violates the rte_eth_tx_burst() ownership
contract when VLAN insertion fails for some packets.
The VLAN pre-processing loop compacts surviving packets into bufs[]
starting at index 0:
for (i = 0; i < nb_bufs; i++) {
struct rte_mbuf *m = bufs[i];
if (m->ol_flags & RTE_MBUF_F_TX_VLAN) {
int error = rte_vlan_insert(&m);
if (unlikely(error)) {
rte_pktmbuf_free(m);
continue; /* <-- skip, don't store */
}
}
bufs[nb_send] = m; /* <-- compacts in-place */
++nb_send;
}
When VLAN insertion fails, the packet is freed and skipped, so
nb_send < nb_bufs and the bufs[] array is rewritten as a compacted
version. After rte_vhost_enqueue_burst() the function returns nb_tx
(the number enqueued to the guest).
This causes two problems:
1. The return value does not correspond to a prefix of the original
array. The caller expects bufs[0..n-1] to be consumed and
bufs[n..nb_bufs-1] to be untouched. But the compaction has
shuffled entries so bufs[n..nb_bufs-1] now contains stale
pointers from before compaction.
For example, with nb_bufs=4 where bufs[1] fails VLAN insertion:
Original: bufs[] = {A, B, C, D}
After compaction: bufs[] = {A, C, D, D}
^
stale duplicate
If vhost enqueues 2 (nb_tx=2), the driver frees A and C (which
the driver enqueued - those are freed at line 515-516). The
caller sees nb_tx=2, then tries to free bufs[2] and bufs[3],
which are D and D -- double free.
2. Packets that passed VLAN pre-processing but were not enqueued
by rte_vhost_enqueue_burst() are not freed by the driver.
The caller cannot reliably free them either because the array
has been rewritten.
The missed_pkts statistic (nb_bufs - nb_tx) is also inflated
because it counts VLAN failures (already freed) as missed.
--
You are receiving this mail because:
You are the assignee for the bug.
