Skip to content

Commit cb08c1f

Browse files
committed
optimize cstring reading
Reading cstrings byte by byte is very inefficient.
1 parent 9117e0b commit cb08c1f

File tree

4 files changed

+99
-3
lines changed

4 files changed

+99
-3
lines changed

tee/kernel/src/memory/pagetable.rs

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ use constants::{
3030
use flush::{FlushGuard, GlobalFlushGuard};
3131
use log::trace;
3232
use static_page_tables::{StaticPageTable, StaticPd, StaticPdp, StaticPml4, flags};
33+
use usize_conversions::usize_from;
3334
use x86_64::{
3435
PhysAddr, VirtAddr,
3536
instructions::tlb::Pcid,
@@ -264,6 +265,44 @@ fn try_read_fast(src: VirtAddr, dest: NonNull<[u8]>) -> Result<(), ()> {
264265
if failed == 0 { Ok(()) } else { Err(()) }
265266
}
266267

268+
/// Try to copy memory from `src` into `dest` until a null-byte is encountered.
269+
///
270+
/// Returns the number of read bytes.
271+
///
272+
/// This function is not unsafe. If the read fails for some reason `Err(())` is
273+
/// returned.
274+
#[inline(always)]
275+
fn try_read_cstring_fast(src: VirtAddr, dest: NonNull<[u8]>) -> Result<usize, usize> {
276+
let remaining: u64;
277+
let failed: u64;
278+
unsafe {
279+
asm!(
280+
"test rcx, rcx",
281+
"je 67f",
282+
"66:",
283+
"mov {scratch:l}, byte ptr [{src}]",
284+
"mov byte ptr [{dest}], {scratch:l}",
285+
"inc {src}",
286+
"inc {dest}",
287+
"test {scratch:l}, {scratch:l}",
288+
"loopne 66b",
289+
"67:",
290+
".pushsection .recoverable",
291+
".quad 66b",
292+
".quad 67b",
293+
".popsection",
294+
src = inout(reg) src.as_u64() => _,
295+
dest = inout(reg) dest.as_mut_ptr() => _,
296+
inout("rcx") dest.len() => remaining,
297+
scratch = out(reg) _,
298+
inout("rdx") 0u64 => failed,
299+
);
300+
}
301+
302+
let read = dest.len() - usize_from(remaining);
303+
if failed == 0 { Ok(read) } else { Err(read) }
304+
}
305+
267306
/// Try to copy memory from `src` into `dest`.
268307
///
269308
/// If the write fails for some reason `Err(())` is returned.
@@ -677,6 +716,23 @@ impl Pagetables {
677716
without_smap(|| try_read_fast(src, dest))
678717
}
679718

719+
/// Try to copy memory from `src` into `dest` until a null-byte is encountered.
720+
///
721+
/// This function is not unsafe. If the read fails for some reason `Err(())` is
722+
/// returned. If `src` isn't user memory `Err(())` is returned.
723+
#[inline(always)]
724+
pub fn try_read_cstring_user_fast(
725+
&self,
726+
src: VirtAddr,
727+
dest: NonNull<[u8]>,
728+
) -> Result<usize, usize> {
729+
check_user_address(src, dest.len()).map_err(|_| 0usize)?;
730+
731+
let _guard = self.activate();
732+
733+
without_smap(|| try_read_cstring_fast(src, dest))
734+
}
735+
680736
/// Try to copy memory from `src` into `dest`.
681737
///
682738
/// If the write fails for some reason `Err(())` is returned. If `src` isn't

tee/kernel/src/user/process/memory.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,46 @@ impl VirtualMemory {
297297
Ok(ret)
298298
}
299299

300+
/// Read a string from userspace.
301+
pub fn read_small_cstring(
302+
&self,
303+
pointer: Pointer<CString>,
304+
max_length: usize,
305+
) -> Result<CString> {
306+
let capacity = max_length + 2;
307+
let mut buf = Vec::with_capacity(capacity);
308+
let dest = unsafe { NonNull::new_unchecked(buf.as_mut_ptr()) };
309+
let dest = NonNull::from_raw_parts(dest, capacity);
310+
311+
let addr = pointer.get();
312+
check_user_address(addr, capacity)?;
313+
314+
let len = 'read: {
315+
for _ in 0..Self::ACCESS_RETRIES {
316+
match self.pagetables.try_read_cstring_user_fast(addr, dest) {
317+
Ok(len) => break 'read len,
318+
Err(read) => self.map_addrs(
319+
addr + u64::from_usize(read) + 1,
320+
1,
321+
PageTableFlags::empty(),
322+
)?,
323+
}
324+
}
325+
326+
self.pagetables
327+
.try_read_cstring_user_fast(addr, dest)
328+
.unwrap()
329+
};
330+
331+
ensure!(len < capacity, NameTooLong);
332+
unsafe {
333+
buf.set_len(len);
334+
}
335+
336+
let cstr = unsafe { CString::from_vec_with_nul_unchecked(buf) };
337+
Ok(cstr)
338+
}
339+
300340
pub fn set_bytes(&self, addr: VirtAddr, count: usize, byte: u8) -> Result<()> {
301341
if count == 0 {
302342
return Ok(());

tee/kernel/src/user/process/syscall.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3672,7 +3672,7 @@ fn mount(
36723672
) -> SyscallResult {
36733673
let _dev_name = virtual_memory.read(dev_name)?;
36743674
let dir_name = virtual_memory.read(dir_name)?;
3675-
let r#type = virtual_memory.read_cstring(r#type, 0x10)?;
3675+
let r#type = virtual_memory.read_small_cstring(r#type, 0x10)?;
36763676

36773677
let create_node = match r#type.as_bytes() {
36783678
b"devtmpfs" => devtmpfs::new,

tee/kernel/src/user/process/syscall/args/pointee.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ impl Pointee for CStr {
307307
fn display(f: &mut dyn fmt::Write, addr: VirtAddr, thread: &ThreadGuard) -> fmt::Result {
308308
let res = thread
309309
.virtual_memory()
310-
.read_cstring(Pointer::from(addr), 1024);
310+
.read_small_cstring(Pointer::from(addr), 1024);
311311
match res {
312312
Ok(value) => write!(f, "{value:?}"),
313313
Err(_) => write!(f, "{:#x} (invalid ptr)", addr.as_u64()),
@@ -343,7 +343,7 @@ impl AbiAgnosticPointee for Path {}
343343

344344
impl ReadablePointee for Path {
345345
fn read(addr: VirtAddr, vm: &VirtualMemory, _abi: Abi) -> Result<(usize, Self)> {
346-
let pathname = vm.read_cstring(Pointer::from(addr), PATH_MAX)?;
346+
let pathname = vm.read_small_cstring(Pointer::from(addr), PATH_MAX)?;
347347
let len = pathname.to_bytes_with_nul().len();
348348
let value = Path::new(pathname.into_bytes())?;
349349
Ok((len, value))

0 commit comments

Comments
 (0)