vhost: fix null dereference in async packed dequeue
PATCH 1/1 vhost: fix null dereference in async packed dequeue
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.
--- .mailmap | 1 + lib/vhost/virtio_net.c | 26 +++++++++++++++++--------- 2 files changed, 18 insertions(+), 9 deletions(-)
.mailmap +1 -0
@@ -141,6 +141,7 @@ Antara Ganesh Kolar <[email protected]> Anthony Fee <[email protected]> Anthony Harivel <[email protected]> Antonio Fischetti <[email protected]> +Anton Vanda <[email protected]> Anup Prabhu <[email protected]> Anupam Kapoor <[email protected]> Anurag Mandal <[email protected]>
lib/vhost/virtio_net.c +17 -9
@@ -4039,6 +4039,23 @@ virtio_dev_tx_async_packed_batch(struct virtio_net *dev,vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE )rte_prefetch0 ((void *)(uintptr_t )desc_addrs[i]); +if (virtio_net_with_host_offload (dev)) { +vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) { + desc_vva =vhost_iova_to_vva (dev, vq, desc_addrs[i], + &lens[i],VHOST_ACCESS_RO ); +/* +* A malformed or unmapped guest descriptor makes the +* IOVA translation fail (returns 0). Bail out of the +* batch fast path so the single-packet path handles the +* error, instead of dereferencing a NULL header. +*/ +if (unlikely (!desc_vva)) +return -1 ; + hdr = (struct virtio_net_hdr *)(uintptr_t )desc_vva; + pkts_info[slot_idx + i].nethdr = *hdr; + } + } +vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) { host_iova[i] = (void *)(uintptr_t )gpa_to_first_hpa (dev, desc_addrs[i] + buf_offset, pkts[i]->pkt_len, &mapped_len[i]);
@@ -4053,15 +4070,6 @@ virtio_dev_tx_async_packed_batch(struct virtio_net *dev, async->iter_idx++; } -if (virtio_net_with_host_offload (dev)) { -vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) { - desc_vva =vhost_iova_to_vva (dev, vq, desc_addrs[i], - &lens[i],VHOST_ACCESS_RO ); - hdr = (struct virtio_net_hdr *)(uintptr_t )desc_vva; - pkts_info[slot_idx + i].nethdr = *hdr; - } - } -vq_inc_last_avail_packed (vq,PACKED_BATCH_SIZE );vhost_async_shadow_dequeue_packed_batch (vq, ids);
Signature · 1 line
2.51.0
Quoted 12 lines · On Tue, 7 Jul 2026 16:50:44 +0300 Anton Vanda <[email protected]> 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.
Quoted 8 lines
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")Signed-off-by Anton Vanda <[email protected]>---
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".
Quoted 19 lines · Stephen Hemminger
Quoted 15 lines · On Tue, 7 Jul 2026 16:50:44 +0300 Anton Vanda <[email protected]> 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.
Quoted 8 lines
Quoted 5 lines
Fixes c2fa52bf1e5d ("vhost: add batch dequeue in async vhost packed ring")Signed-off-by Anton Vanda <[email protected]>---
Since vhost is so security sensitive asked for more detailed AI review (Claude Fable). It found some issues that need addressing before merge.
vhost async is unmaintained, and I don't know a user of it. I would rather drop it and reduce vhost library attack surface.
I have this idea in the back of my mind for quite some time... I'll post a RFC for removal.
Signature · 1 line
David Marchand
Hello Anton,
Quoted 16 lines · Anton Vanda
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.
Quoted 9 lines
Fixes c2fa52bf1e5d ("vhost: add batch dequeue in async vhost packed ring")Signed-off-by Anton Vanda <[email protected]>--- .mailmap | 1 + lib/vhost/virtio_net.c | 26 +++++++++++++++++--------- 2 files changed, 18 insertions(+), 9 deletions(-)
Thanks for your contribution, but since async support is being removed in v26.11,
your patch is no longer necessary.
Maxime