Skip to content

Commit 82d783c

Browse files
authored
fix(queue): close virtqueue notification races (#11)
Signed-off-by: LoGin <longjin@DragonOS.org>
1 parent 89f6a40 commit 82d783c

2 files changed

Lines changed: 270 additions & 10 deletions

File tree

src/device/net/dev_raw.rs

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ use crate::queue::VirtQueue;
55
use crate::transport::Transport;
66
use crate::volatile::volread;
77
use crate::{Error, Result};
8+
#[cfg(feature = "alloc")]
9+
use alloc::boxed::Box;
810
use log::{debug, info, warn};
911
use zerocopy::AsBytes;
1012

@@ -19,10 +21,26 @@ use zerocopy::AsBytes;
1921
pub struct VirtIONetRaw<H: Hal, T: Transport, const QUEUE_SIZE: usize> {
2022
transport: T,
2123
mac: EthernetAddress,
24+
#[cfg(feature = "alloc")]
25+
recv_queue: Box<VirtQueue<H, QUEUE_SIZE>>,
26+
#[cfg(not(feature = "alloc"))]
2227
recv_queue: VirtQueue<H, QUEUE_SIZE>,
28+
#[cfg(feature = "alloc")]
29+
send_queue: Box<VirtQueue<H, QUEUE_SIZE>>,
30+
#[cfg(not(feature = "alloc"))]
2331
send_queue: VirtQueue<H, QUEUE_SIZE>,
2432
}
2533

34+
/// Queue positions captured while enabling device-to-driver notifications.
35+
///
36+
/// Pass this value to [`VirtIONetRaw::interrupt_pending`] immediately after
37+
/// enabling notifications to close the disable/poll/enable race.
38+
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
39+
pub struct InterruptState {
40+
receive_used_idx: u16,
41+
transmit_used_idx: u16,
42+
}
43+
2644
impl<H: Hal, T: Transport, const QUEUE_SIZE: usize> VirtIONetRaw<H, T, QUEUE_SIZE> {
2745
/// Create a new VirtIO-Net driver.
2846
pub fn new(mut transport: T) -> Result<Self> {
@@ -40,12 +58,28 @@ impl<H: Hal, T: Transport, const QUEUE_SIZE: usize> VirtIONetRaw<H, T, QUEUE_SIZ
4058
volread!(config, status)
4159
);
4260
}
61+
#[cfg(feature = "alloc")]
62+
let send_queue = VirtQueue::new_boxed(
63+
&mut transport,
64+
QUEUE_TRANSMIT,
65+
false,
66+
negotiated_features.contains(Features::RING_EVENT_IDX),
67+
)?;
68+
#[cfg(not(feature = "alloc"))]
4369
let send_queue = VirtQueue::new(
4470
&mut transport,
4571
QUEUE_TRANSMIT,
4672
false,
4773
negotiated_features.contains(Features::RING_EVENT_IDX),
4874
)?;
75+
#[cfg(feature = "alloc")]
76+
let recv_queue = VirtQueue::new_boxed(
77+
&mut transport,
78+
QUEUE_RECEIVE,
79+
false,
80+
negotiated_features.contains(Features::RING_EVENT_IDX),
81+
)?;
82+
#[cfg(not(feature = "alloc"))]
4983
let recv_queue = VirtQueue::new(
5084
&mut transport,
5185
QUEUE_RECEIVE,
@@ -76,8 +110,26 @@ impl<H: Hal, T: Transport, const QUEUE_SIZE: usize> VirtIONetRaw<H, T, QUEUE_SIZ
76110

77111
/// Enable interrupts.
78112
pub fn enable_interrupts(&mut self) {
79-
self.send_queue.set_dev_notify(true);
80-
self.recv_queue.set_dev_notify(true);
113+
let _ = self.enable_interrupts_prepare();
114+
}
115+
116+
/// Enable interrupts and capture the queue positions used for the
117+
/// subsequent race-closing check.
118+
pub fn enable_interrupts_prepare(&mut self) -> InterruptState {
119+
InterruptState {
120+
receive_used_idx: self.recv_queue.enable_dev_notify_prepare(),
121+
transmit_used_idx: self.send_queue.enable_dev_notify_prepare(),
122+
}
123+
}
124+
125+
/// Return whether either queue gained a used buffer while interrupts were
126+
/// being enabled.
127+
///
128+
/// This method includes the full memory barrier required between publishing
129+
/// the notification request and rereading the device-owned used indices.
130+
pub fn interrupt_pending(&self, state: InterruptState) -> bool {
131+
self.recv_queue.dev_notify_pending(state.receive_used_idx)
132+
|| self.send_queue.dev_notify_pending(state.transmit_used_idx)
81133
}
82134

83135
/// Get MAC address.
@@ -162,6 +214,12 @@ impl<H: Hal, T: Transport, const QUEUE_SIZE: usize> VirtIONetRaw<H, T, QUEUE_SIZ
162214
self.send_queue.peek_used()
163215
}
164216

217+
/// Like [`Self::poll_transmit`], but rejects a device-provided descriptor
218+
/// ID which is outside this queue instead of narrowing it.
219+
pub fn poll_transmit_checked(&self) -> Result<Option<u16>> {
220+
Self::checked_used_token(self.send_queue.peek_used_id())
221+
}
222+
165223
/// Completes a transmission operation which was started by [`transmit_begin`].
166224
/// Returns number of bytes transmitted.
167225
///
@@ -214,6 +272,20 @@ impl<H: Hal, T: Transport, const QUEUE_SIZE: usize> VirtIONetRaw<H, T, QUEUE_SIZ
214272
self.recv_queue.peek_used()
215273
}
216274

275+
/// Like [`Self::poll_receive`], but rejects a device-provided descriptor ID
276+
/// which is outside this queue instead of narrowing it.
277+
pub fn poll_receive_checked(&self) -> Result<Option<u16>> {
278+
Self::checked_used_token(self.recv_queue.peek_used_id())
279+
}
280+
281+
fn checked_used_token(id: Option<u32>) -> Result<Option<u16>> {
282+
match id {
283+
None => Ok(None),
284+
Some(id) if id < QUEUE_SIZE as u32 => Ok(Some(id as u16)),
285+
Some(_) => Err(Error::IoError),
286+
}
287+
}
288+
217289
/// Completes a transmission operation which was started by [`receive_begin`].
218290
///
219291
/// After completion, the `rx_buf` will contain a header followed by the

0 commit comments

Comments
 (0)