[PATCH] vhost: fix null dereference in async packed dequeue

[Stephen Hemminger]

On Tue, 7 Jul 2026 16:50:44 +0300
Anton Vanda <avanda at ptsecurity.com> wrote:

> In the batch path of the asynchronous packed ring dequeue, the address
> of the virtio net header is obtained from vhost_iova_to_vva(), which
> returns 0 when a guest-provided descriptor address cannot be fully
> translated.  The batch check only validates that the descriptor address
> is non-zero and that the length is consistent. A malicious or buggy
> guest could therefore trigger a NULL pointer dereference and crash the
> vhost application (denial of service).
> 
> Check the translation result and leave the batch fast path with an error
> on failure, so the single-packet path handles the invalid descriptor, as
> is already done for the non-batch async dequeue path.
> 
> Perform the header translation before the DMA iovec setup so that the
> early return cannot leave the async iterator state partially updated.
> 
> Fixes: c2fa52bf1e5d ("vhost: add batch dequeue in async vhost packed ring")
> Cc: stable at dpdk.org
> 
> Signed-off-by: Anton Vanda <avanda at ptsecurity.com>
> ---

Since vhost is so security sensitive asked for more detailed AI review (Claude Fable).
It found some issues that need addressing before merge.


Review: [PATCH] vhost: fix null dereference in async packed dequeue

Errors
------

1. The check is incomplete: a non-zero return from vhost_iova_to_vva()
   does not mean the header is fully mapped. The function shrinks *len
   to the contiguously mapped length and returns a valid VVA whenever
   the start address falls inside a region (rte_vhost_va_from_guest_pa
   caps *len at region end; the IOTLB path does the same per entry).
   A guest that places a descriptor in the last 1-9 bytes of a memory
   region gets desc_vva != 0 with lens[i] < sizeof(struct virtio_net_hdr),
   and *hdr then reads past the end of the region mmap -- the same
   guest-triggerable crash class this patch is closing.

   Compare the other two paths:
   - single-packet: copy_vnet_hdr_from_desc() assembles a header that
     spans mappings from buf_vec chunks (virtio_net.c:2922).
   - sync batch: virtio_dev_tx_batch_packed_check() rejects partial
     mappings via lens[i] != descs[...].len after translation.

   Suggested fix -- translate only the header and verify coverage:

	uint64_t hdr_len = sizeof(struct virtio_net_hdr);

	desc_vva = vhost_iova_to_vva(dev, vq, desc_addrs[i],
				&hdr_len, VHOST_ACCESS_RO);
	if (unlikely(!desc_vva ||
		     hdr_len < sizeof(struct virtio_net_hdr)))
		return -1;

   Using a local length also stops clobbering lens[i]. That is harmless
   today (lens[] is dead after this point in the function) but fragile
   against future reordering.

TOCTOU assessment (requested)
-----------------------------

No TOCTOU introduced by this patch:

- No re-reads of the descriptor ring: the translation uses the
  desc_addrs[]/lens[] snapshots taken in
  vhost_async_tx_batch_packed_check(); guest-shared memory is not
  consulted again for validation.
- The header is consumed via a single struct copy into
  pkts_info[slot_idx + i].nethdr; the completion path parses that
  snapshot, so concurrent guest writes to the header cannot produce
  inconsistent offload parsing. The store escapes to the heap, so the
  compiler cannot elide or delay the copy -- the rte_compiler_barrier()
  that desc_to_mbuf() needs for its stack-local tmp_hdr is not needed
  here.
- Moving the block before the iovec setup does not open a check/use
  window: the same snapshot address feeds both the header translation
  and gpa_to_first_hpa().
- The early return leaves no partial state: async->iter_idx and the
  iovec array are untouched, last_avail_idx and the shadow ring are not
  advanced, and the caller falls through to the single-packet path,
  whose desc_to_mbuf() rewrites nethdr for any lanes the failed batch
  already filled. The reordering rationale in the commit message is
  correct.

Info
----

1. Same function, same threat model, pre-existing: host_iova[i] from
   gpa_to_first_hpa() is never checked; on translation failure
   rte_dma_copy() is issued with source IOVA 0, and a short mapped_len[i]
   silently truncates the copy. Worth a follow-up patch in this series
   or after.

2. .mailmap uses C-locale ordering ("Anup Prabhu" before
   "Anupam Kapoor"), so "Anton Vanda" sorts before "Antonio Fischetti"
   -- the new entry belongs one line up, after "Anthony Harivel".