Skip to content

Commit 15a5aba

Browse files
committed
feat(virtq): do not swallow errors
Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
1 parent 83c21fd commit 15a5aba

4 files changed

Lines changed: 47 additions & 23 deletions

File tree

src/hyperlight_common/src/virtq/consumer.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -329,10 +329,14 @@ impl<M: MemOps + Clone, N: Notifier> VirtqConsumer<M, N> {
329329
self.next_token = self.next_token.wrapping_add(1);
330330

331331
// Copy entry data from shared memory
332-
let data = entry_elem
333-
.map(|elem| self.read_element(&elem))
334-
.transpose()?
335-
.unwrap_or_default();
332+
let data = match entry_elem.map(|elem| self.read_element(&elem)).transpose() {
333+
Ok(d) => d.unwrap_or_default(),
334+
Err(e) => {
335+
// Read failed - clear inflight before propagating
336+
self.inflight.set(id_idx, false);
337+
return Err(e);
338+
}
339+
};
336340

337341
let entry = RecvEntry { token, data };
338342

src/hyperlight_common/src/virtq/ring.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1251,7 +1251,8 @@ fn should_notify(evt: EventSuppression, ring_len: u16, old: RingCursor, new: Rin
12511251

12521252
ring_need_event(off, new.head(), old.head())
12531253
}
1254-
_ => unreachable!(),
1254+
// treat as disabled if invalid
1255+
_ => false,
12551256
}
12561257
}
12571258

src/hyperlight_host/src/mem/mgr.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,8 +1014,11 @@ impl SandboxMemoryManager<HostSharedMemory> {
10141014
return Err(new_error!("G2H: result entry too short"));
10151015
}
10161016

1017-
let hdr: &VirtqMsgHeader = bytemuck::from_bytes(&entry_data[..VirtqMsgHeader::SIZE]);
1018-
let payload = &entry_data[VirtqMsgHeader::SIZE..];
1017+
let hdr_size = VirtqMsgHeader::SIZE;
1018+
let hdr: &VirtqMsgHeader = bytemuck::from_bytes(&entry_data[..hdr_size]);
1019+
let available = entry_data.len() - hdr_size;
1020+
let payload_len = (hdr.payload_len as usize).min(available);
1021+
let payload = &entry_data[hdr_size..hdr_size + payload_len];
10191022

10201023
match hdr.msg_kind() {
10211024
Ok(MsgKind::Response) => {

src/hyperlight_host/src/sandbox/outb.rs

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -199,27 +199,40 @@ fn outb_virtq_call(
199199

200200
// Drain entries, processing Log messages, until we find a Request.
201201
let (entry, completion) = loop {
202-
let Some((entry, completion)) = consumer
203-
.poll(g2h_pool_size)
204-
.map_err(|e| HandleOutbError::ReadHostFunctionCall(format!("G2H poll: {e}")))?
205-
else {
202+
let Ok(maybe_next) = consumer.poll(g2h_pool_size) else {
203+
return Err(HandleOutbError::ReadHostFunctionCall(
204+
"G2H poll failed".into(),
205+
));
206+
};
207+
208+
let Some((entry, completion)) = maybe_next else {
206209
// No G2H entry - backpressure-only notify or prefill notify.
207210
return Ok(());
208211
};
209212

213+
let hdr_size = VirtqMsgHeader::SIZE;
210214
let entry_data = entry.data();
211-
if entry_data.len() < VirtqMsgHeader::SIZE {
215+
216+
if entry_data.len() < hdr_size {
212217
return Err(HandleOutbError::ReadHostFunctionCall(
213218
"G2H entry too short".into(),
214219
));
215220
}
216-
let hdr: VirtqMsgHeader = *bytemuck::from_bytes(&entry_data[..VirtqMsgHeader::SIZE]);
221+
222+
let hdr: VirtqMsgHeader = *bytemuck::from_bytes(&entry_data[..hdr_size]);
217223

218224
match hdr.msg_kind() {
219225
Ok(MsgKind::Log) => {
220-
let payload = &entry_data[VirtqMsgHeader::SIZE..];
226+
let available = entry_data.len() - hdr_size;
227+
let log_len = (hdr.payload_len as usize).min(available);
228+
let payload = &entry_data[hdr_size..hdr_size + log_len];
229+
221230
emit_guest_log(payload);
222-
let _ = consumer.complete(completion);
231+
232+
consumer.complete(completion).map_err(|e| {
233+
HandleOutbError::ReadHostFunctionCall(format!("G2H complete log: {e}"))
234+
})?;
235+
223236
continue;
224237
}
225238
Ok(MsgKind::Request) => break (entry, completion),
@@ -237,8 +250,18 @@ fn outb_virtq_call(
237250
}
238251
};
239252

253+
// Validate completion buffer before calling the host function
254+
let virtq::SendCompletion::Writable(mut wc) = completion else {
255+
return Err(HandleOutbError::WriteHostFunctionResponse(
256+
"G2H: expected writable completion, got ack (ring corruption)".into(),
257+
));
258+
};
259+
240260
let entry_data = entry.data();
241-
let payload = &entry_data[VirtqMsgHeader::SIZE..];
261+
let hdr: VirtqMsgHeader = *bytemuck::from_bytes(&entry_data[..VirtqMsgHeader::SIZE]);
262+
let available = entry_data.len() - VirtqMsgHeader::SIZE;
263+
let payload_len = (hdr.payload_len as usize).min(available);
264+
let payload = &entry_data[VirtqMsgHeader::SIZE..VirtqMsgHeader::SIZE + payload_len];
242265

243266
let call = FunctionCall::try_from(payload)
244267
.map_err(|e| HandleOutbError::ReadHostFunctionCall(e.to_string()))?;
@@ -259,13 +282,6 @@ fn outb_virtq_call(
259282
let resp_header = VirtqMsgHeader::new(MsgKind::Response, 0, result_payload.len() as u32);
260283
let resp_header_bytes = bytemuck::bytes_of(&resp_header);
261284

262-
// Write response into the completion buffer
263-
let virtq::SendCompletion::Writable(mut wc) = completion else {
264-
return Err(HandleOutbError::WriteHostFunctionResponse(
265-
"G2H: expected writable completion, got ack (ring corruption)".into(),
266-
));
267-
};
268-
269285
wc.write_all(resp_header_bytes)
270286
.map_err(|e| HandleOutbError::WriteHostFunctionResponse(format!("{e}")))?;
271287
wc.write_all(result_payload)

0 commit comments

Comments
 (0)