Skip to content

Commit a8d7363

Browse files
committed
feat(virtio-mem): mprotect unplugged memory ranges
This prevents the device emulation to be tricked into accessing unplugged memory ranges. If a malicious driver tries to do so, the VMM will crash with a memory error. Signed-off-by: Riccardo Mancini <[email protected]>
1 parent 2f16b1d commit a8d7363

File tree

4 files changed

+50
-4
lines changed

4 files changed

+50
-4
lines changed

resources/seccomp/aarch64-unknown-linux-musl.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,10 @@
472472
{
473473
"syscall": "restart_syscall",
474474
"comment": "automatically issued by the kernel when specific timing-related syscalls (e.g. nanosleep) get interrupted by SIGSTOP"
475+
},
476+
{
477+
"syscall": "mprotect",
478+
"comment": "Used by memory hotplug to protect access to underlying host memory"
475479
}
476480
]
477481
},

resources/seccomp/x86_64-unknown-linux-musl.json

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,10 @@
484484
{
485485
"syscall": "restart_syscall",
486486
"comment": "automatically issued by the kernel when specific timing-related syscalls (e.g. nanosleep) get interrupted by SIGSTOP"
487+
},
488+
{
489+
"syscall": "mprotect",
490+
"comment": "Used by memory hotplug to protect access to underlying host memory"
487491
}
488492
]
489493
},

src/vmm/src/vstate/memory.rs

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,8 @@ pub enum MemoryError {
5757
FileMetadata(std::io::Error),
5858
/// Memory region is not aligned
5959
Unaligned,
60+
/// Error protecting memory slot: {0}
61+
Mprotect(std::io::Error),
6062
}
6163

6264
/// Type of the guest region
@@ -161,6 +163,28 @@ impl<'a> GuestMemorySlot<'a> {
161163

162164
Ok(())
163165
}
166+
167+
/// Makes the slot host memory PROT_NONE (true) or PROT_READ|PROT_WRITE (false)
168+
pub(crate) fn protect(&self, protected: bool) -> Result<(), MemoryError> {
169+
let prot = if protected {
170+
libc::PROT_NONE
171+
} else {
172+
libc::PROT_READ | libc::PROT_WRITE
173+
};
174+
// SAFETY: Parameters refer to an existing host memory region
175+
let ret = unsafe {
176+
libc::mprotect(
177+
self.slice.ptr_guard_mut().as_ptr().cast(),
178+
self.slice.len(),
179+
prot,
180+
)
181+
};
182+
if ret != 0 {
183+
Err(MemoryError::Mprotect(std::io::Error::last_os_error()))
184+
} else {
185+
Ok(())
186+
}
187+
}
164188
}
165189

166190
fn addr_in_range(addr: GuestAddress, start: GuestAddress, len: usize) -> bool {
@@ -297,11 +321,20 @@ impl GuestRegionMmapExt {
297321
if prev == plug {
298322
return Ok(());
299323
}
324+
300325
let mut kvm_region = kvm_userspace_memory_region::from(mem_slot);
301-
if !plug {
326+
if plug {
327+
// make it accessible _before_ adding it to KVM
328+
mem_slot.protect(false)?;
329+
vm.set_user_memory_region(kvm_region)?;
330+
} else {
331+
// to remove it we need to pass a size of zero
302332
kvm_region.memory_size = 0;
333+
vm.set_user_memory_region(kvm_region)?;
334+
// make it protected _after_ removing it from KVM
335+
mem_slot.protect(true)?;
303336
}
304-
vm.set_user_memory_region(kvm_region)
337+
Ok(())
305338
}
306339

307340
pub(crate) fn discard_range(

src/vmm/src/vstate/vm.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,13 @@ impl Vm {
196196
.insert_region(Arc::clone(&region))?;
197197

198198
region
199-
.plugged_slots()
200-
.try_for_each(|ref slot| self.set_user_memory_region(slot.into()))?;
199+
.slots()
200+
.try_for_each(|(ref slot, plugged)| match plugged {
201+
// if the slot is plugged, add it to kvm user memory regions
202+
true => self.set_user_memory_region(slot.into()),
203+
// if the slot is not plugged, protect accesses to it
204+
false => slot.protect(true).map_err(VmError::MemoryError),
205+
})?;
201206

202207
self.common.guest_memory = new_guest_memory;
203208

0 commit comments

Comments
 (0)