From 22856fffdda15ca1dbc045287173479e79f40767 Mon Sep 17 00:00:00 2001 From: Anatol Belski Date: Wed, 6 May 2026 10:14:50 +0200 Subject: [PATCH] virtio-devices: Factor out NEEDS_RESET helpers Both block and net implement the same DEVICE_NEEDS_RESET bookkeeping when a corrupted virtqueue request is detected. They set the bit, trigger a config change interrupt and log a warning. Move that logic into shared device_needs_reset and mark_device_needs_reset helpers in lib.rs and update both call sites to use them. No functional change. Signed-off-by: Anatol Belski --- virtio-devices/src/block.rs | 31 +++++++++++-------------------- virtio-devices/src/lib.rs | 28 ++++++++++++++++++++++++++++ virtio-devices/src/net.rs | 31 +++++++++++-------------------- 3 files changed, 50 insertions(+), 40 deletions(-) diff --git a/virtio-devices/src/block.rs b/virtio-devices/src/block.rs index 528946026..f72370443 100644 --- a/virtio-devices/src/block.rs +++ b/virtio-devices/src/block.rs @@ -50,7 +50,7 @@ use super::{ }; use crate::seccomp_filters::Thread; use crate::thread_helper::spawn_virtio_thread; -use crate::{GuestMemoryMmap, VirtioInterrupt}; +use crate::{GuestMemoryMmap, VirtioInterrupt, device_needs_reset, mark_device_needs_reset}; const SECTOR_SHIFT: u8 = 9; pub const SECTOR_SIZE: u64 = 0x01 << SECTOR_SHIFT; @@ -174,7 +174,7 @@ fn has_feature(features: u64, feature_flag: u64) -> bool { impl BlockEpollHandler { fn needs_reset(&self) -> bool { - (self.device_status.load(Ordering::Acquire) & crate::DEVICE_NEEDS_RESET as u8) != 0 + device_needs_reset(&self.device_status) } fn check_request( @@ -209,25 +209,16 @@ impl BlockEpollHandler { fn handle_queue_iterator_error(&mut self, err: &virtio_queue::Error) { // The guest submitted a corrupted VirtQ request, and the error - // was logged during queue processing. We cannot just ignore the - // error, as the guest could continue spamming the VMM with bad - // requests, triggering excessive error logging. So we mark - // the device "NEEDS_RESET", effectively stopping all request - // processing (see self.needs_reset() usage) until the guest - // resets and reactivates the device. - - warn!( - "Corrupted request detected (virtqueue error: {err:?}). \ -Setting device status to 'NEEDS_RESET' and stopping processing queues until reset." + // was logged during queue processing. Ignoring it would let the + // guest keep spamming the VMM with bad requests and trigger + // excessive error logging, so mark the device as NEEDS_RESET to + // stop request processing (see self.needs_reset() usage) until + // the guest resets and reactivates the device. + mark_device_needs_reset( + &self.device_status, + self.interrupt_cb.as_ref(), + format_args!("virtqueue error: {err:?}"), ); - - self.device_status - .fetch_or(crate::DEVICE_NEEDS_RESET as u8, Ordering::SeqCst); - - // Let the guest know that the device status has changed. - if let Err(e) = self.interrupt_cb.trigger(VirtioInterruptType::Config) { - error!("Failed to signal config interrupt: {e:?}"); - } } fn process_queue_submit(&mut self) -> Result<()> { diff --git a/virtio-devices/src/lib.rs b/virtio-devices/src/lib.rs index 767284e85..3d6296dff 100644 --- a/virtio-devices/src/lib.rs +++ b/virtio-devices/src/lib.rs @@ -11,6 +11,7 @@ //! Implements virtio devices, queues, and transport mechanisms. use std::io; +use std::sync::atomic::{AtomicU8, Ordering}; use serde::{Deserialize, Serialize}; use thiserror::Error; @@ -69,6 +70,33 @@ const DEVICE_FEATURES_OK: u32 = 0x08; const DEVICE_NEEDS_RESET: u32 = 0x40; const DEVICE_FAILED: u32 = 0x80; +/// Returns true if `device_status` has the `DEVICE_NEEDS_RESET` bit set. +pub(crate) fn device_needs_reset(device_status: &AtomicU8) -> bool { + (device_status.load(Ordering::Acquire) & DEVICE_NEEDS_RESET as u8) != 0 +} + +/// Marks a virtio device as `NEEDS_RESET` and notifies the guest via a config +/// change interrupt. Used when a guest induced error (corrupted virtqueue, +/// malformed descriptor chain or similar) is detected and the device wants +/// to stop further queue processing without killing the worker thread. +/// +/// `context` is included verbatim in the warning log to identify the cause. +pub(crate) fn mark_device_needs_reset( + device_status: &AtomicU8, + interrupt_cb: &dyn self::VirtioInterrupt, + context: std::fmt::Arguments<'_>, +) { + log::warn!( + "Corrupted request detected ({context}). Setting device status to 'NEEDS_RESET' and stopping processing queues until reset." + ); + + device_status.fetch_or(DEVICE_NEEDS_RESET as u8, Ordering::SeqCst); + + if let Err(e) = interrupt_cb.trigger(self::VirtioInterruptType::Config) { + log::error!("Failed to signal config interrupt: {e:?}"); + } +} + const VIRTIO_F_RING_INDIRECT_DESC: u32 = 28; const VIRTIO_F_RING_EVENT_IDX: u32 = 29; const VIRTIO_F_VERSION_1: u32 = 32; diff --git a/virtio-devices/src/net.rs b/virtio-devices/src/net.rs index 30fa25b6d..f85192e27 100644 --- a/virtio-devices/src/net.rs +++ b/virtio-devices/src/net.rs @@ -42,7 +42,7 @@ use super::{ }; use crate::seccomp_filters::Thread; use crate::thread_helper::spawn_virtio_thread; -use crate::{GuestMemoryMmap, VirtioInterrupt}; +use crate::{GuestMemoryMmap, VirtioInterrupt, device_needs_reset, mark_device_needs_reset}; /// Control queue // Event available on the control queue. @@ -221,25 +221,16 @@ impl NetEpollHandler { fn handle_queue_iterator_error(&mut self, err: &virtio_queue::Error) { // The guest submitted a corrupted VirtQ request, and the error - // was logged during queue processing. We cannot just ignore the - // error, as the guest could continue spamming the VMM with bad - // requests, triggering excessive error logging. So we mark - // the device "NEEDS_RESET", effectively stopping all request - // processing (see self.needs_reset() usage) until the guest - // resets and reactivates the device. - - warn!( - "Corrupted request detected (virtqueue error: {err:?}). \ -Setting device status to 'NEEDS_RESET' and stopping processing queues until reset." + // was logged during queue processing. Ignoring it would let the + // guest keep spamming the VMM with bad requests and trigger + // excessive error logging, so mark the device as NEEDS_RESET to + // stop request processing (see self.needs_reset() usage) until + // the guest resets and reactivates the device. + mark_device_needs_reset( + &self.device_status, + self.interrupt_cb.as_ref(), + format_args!("virtqueue error: {err:?}"), ); - - self.device_status - .fetch_or(crate::DEVICE_NEEDS_RESET as u8, Ordering::SeqCst); - - // Let the guest know that the device status has changed. - if let Err(e) = self.interrupt_cb.trigger(VirtioInterruptType::Config) { - error!("Failed to signal config interrupt: {e:?}"); - } } fn process_tx(&mut self) -> result::Result<(), DeviceError> { @@ -343,7 +334,7 @@ Setting device status to 'NEEDS_RESET' and stopping processing queues until rese } fn needs_reset(&self) -> bool { - (self.device_status.load(Ordering::Acquire) & crate::DEVICE_NEEDS_RESET as u8) != 0 + device_needs_reset(&self.device_status) } }