From aceb9baee8aec6844125bd6612f92e9a281373df Mon Sep 17 00:00:00 2001 From: Prabhpreet Dua <615318+prabhpreet@users.noreply.github.com> Date: Wed, 12 Jun 2024 11:29:42 +0530 Subject: [PATCH 1/6] feat: Improve memory usage- nest memfd_secret in anonymous mmap guard page Co-authored-by: Prabhpreet Dua <615318+prabhpreet@users.noreply.github.com> Co-authored-by: Karolin Varner --- src/alloc/allocext/linux.rs | 104 ++++++++++++++++++++++++++++-------- 1 file changed, 83 insertions(+), 21 deletions(-) diff --git a/src/alloc/allocext/linux.rs b/src/alloc/allocext/linux.rs index 154ef62..6f5e278 100644 --- a/src/alloc/allocext/linux.rs +++ b/src/alloc/allocext/linux.rs @@ -1,18 +1,25 @@ extern crate std; +use libc::{MAP_ANONYMOUS, MAP_FIXED, MAP_NORESERVE, MAP_SHARED, PROT_NONE}; + use self::std::process::abort; use crate::{alloc::*, Prot}; use core::mem::{self, size_of}; -use core::ptr::{self, NonNull}; +use core::ptr::{self, null_mut, NonNull}; use core::slice; use self::memfd_secret_alloc::*; mod memfd_secret_alloc { + use libc::{MAP_LOCKED, MAP_POPULATE}; + use super::*; use core::convert::TryInto; #[inline] - pub unsafe fn alloc_memfd_secret(size: usize) -> Option<(NonNull, libc::c_int)> { + pub unsafe fn alloc_memfd_secret( + size: usize, + ptr: Option<*mut libc::c_void>, + ) -> Option<(NonNull, libc::c_int)> { let fd: Result = libc::syscall(libc::SYS_memfd_secret, 0).try_into(); let fd = fd.ok().filter(|&fd| fd >= 0)?; @@ -21,10 +28,17 @@ mod memfd_secret_alloc { let _ = libc::ftruncate(fd, size as libc::off_t); let ptr = libc::mmap( - ptr::null_mut(), + if ptr.is_some() { + ptr.unwrap() + } else { + ptr::null_mut() + }, size, Prot::ReadWrite, - libc::MAP_SHARED, + libc::MAP_SHARED + | if ptr.is_some() { libc::MAP_FIXED } else { 0 } + | MAP_LOCKED + | MAP_POPULATE, fd, 0, ); @@ -47,29 +61,60 @@ unsafe fn _memfd_secret(size: usize) -> Option<*mut u8> { return None; } - // aligned alloc ptr let size_with_canary = CANARY_SIZE + size; + let front_guard_size = PAGE_SIZE + PAGE_SIZE; let unprotected_size = page_round(size_with_canary); - let total_size = PAGE_SIZE + PAGE_SIZE + unprotected_size + PAGE_SIZE; - let (base_ptr, fd) = alloc_memfd_secret(total_size)?; - let base_ptr = base_ptr.as_ptr(); - let fd_ptr = base_ptr.add(size_of::()); - let unprotected_ptr = base_ptr.add(PAGE_SIZE * 2); + let back_guard_size = PAGE_SIZE; + let total_size = front_guard_size + unprotected_size + back_guard_size; + + let base_ptr = libc::mmap( + null_mut(), + total_size, + PROT_NONE, + MAP_SHARED | MAP_ANONYMOUS | MAP_NORESERVE, + -1, + 0, + ); - // mprotect can be used to change protection flag after mmap setup - // https://www.gnu.org/software/libc/manual/html_node/Memory-Protection.html#index-mprotect - _mprotect(base_ptr.add(PAGE_SIZE), PAGE_SIZE, Prot::NoAccess); - _mprotect( - unprotected_ptr.add(unprotected_size), + if base_ptr == libc::MAP_FAILED { + return None; + } + let base_ptr = base_ptr as *mut u8; + + let base_ptr_stored = libc::mmap( + base_ptr as *mut libc::c_void, PAGE_SIZE, - Prot::NoAccess, - ); + Prot::ReadWrite, + MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, + -1, + 0, + ) as *mut u8; + + if base_ptr_stored == libc::MAP_FAILED as *mut u8 || base_ptr_stored != base_ptr { + libc::munmap(base_ptr as *mut libc::c_void, total_size); + return None; + } + + let unprotected_ptr = base_ptr.add(front_guard_size); + let Some((unprotected_ptr_val, fd)) = + alloc_memfd_secret(unprotected_size, Some(unprotected_ptr as *mut libc::c_void)) + else { + libc::munmap(base_ptr_stored as *mut libc::c_void, PAGE_SIZE); + libc::munmap(base_ptr as *mut libc::c_void, total_size); + return None; + }; + assert_eq!(unprotected_ptr_val.as_ptr(), unprotected_ptr as *mut u8); + + let fd_ptr = base_ptr.add(size_of::()); let canary_ptr = unprotected_ptr.add(unprotected_size - size_with_canary); let user_ptr = canary_ptr.add(CANARY_SIZE); ptr::copy_nonoverlapping(CANARY.as_ptr(), canary_ptr, CANARY_SIZE); ptr::write_unaligned(base_ptr as *mut usize, unprotected_size); ptr::write_unaligned(fd_ptr as *mut libc::c_int, fd); + + // mprotect can be used to change protection flag after mmap setup + // https://www.gnu.org/software/libc/manual/html_node/Memory-Protection.html#index-mprotect _mprotect(base_ptr, PAGE_SIZE, Prot::ReadOnly); assert_eq!(unprotected_ptr_from_user_ptr(user_ptr), unprotected_ptr); @@ -109,20 +154,37 @@ pub unsafe fn free_memfd_secret(memptr: NonNull) { let base_ptr = unprotected_ptr.sub(PAGE_SIZE * 2); let fd_ptr = base_ptr.add(size_of::()) as *mut libc::c_int; let unprotected_size = ptr::read(base_ptr as *const usize); + let back_guard_ptr = unprotected_ptr.add(unprotected_size); let fd = ptr::read(fd_ptr); - // check + // check canary value is same if !crate::memeq(canary_ptr as *const u8, CANARY.as_ptr(), CANARY_SIZE) { abort(); } // free - let total_size = PAGE_SIZE + PAGE_SIZE + unprotected_size + PAGE_SIZE; - _mprotect(base_ptr, total_size, Prot::ReadWrite); + let front_guard_size = PAGE_SIZE + PAGE_SIZE; + let back_guard_size = PAGE_SIZE; + + _mprotect(base_ptr.add(PAGE_SIZE), front_guard_size, Prot::ReadWrite); + _mprotect(back_guard_ptr, back_guard_size, Prot::ReadWrite); crate::memzero(unprotected_ptr, unprotected_size); - let res = libc::munmap(base_ptr as *mut c_void, total_size); + // Unmap memfd_secret mapping + let res = libc::munmap(unprotected_ptr as *mut c_void, unprotected_size); + if res < 0 { + abort(); + } + + // Unmap header mapping + let res = libc::munmap(base_ptr as *mut c_void, PAGE_SIZE); + if res < 0 { + abort(); + } + + // Unmap reserved space mapping + let res = libc::munmap(base_ptr as *mut c_void, PAGE_SIZE); if res < 0 { abort(); } From e4584e9b17f66da78140945aaeb45364547a1af2 Mon Sep 17 00:00:00 2001 From: Prabhpreet Dua <615318+prabhpreet@users.noreply.github.com> Date: Thu, 13 Jun 2024 09:18:07 +0530 Subject: [PATCH 2/6] Increment version number, fix readme link spacing --- Cargo.toml | 2 +- README.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index f2b69e3..f6f8323 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "memsec" -version = "0.7.0" +version = "0.7.1" authors = ["quininer kel "] description = "Rust implementation `libsodium/utils`." repository = "https://github.com/quininer/memsec" diff --git a/README.md b/README.md index 256ef5f..3b5bf6c 100644 --- a/README.md +++ b/README.md @@ -20,4 +20,4 @@ ref * [rlibc](https://github.com/alexcrichton/rlibc) * [aligned\_alloc.rs](https://github.com/jonas-schievink/aligned_alloc.rs) * [cst\_time\_memcmp](https://github.com/chmike/cst_time_memcmp) -* [memfd_secret] (https://man.archlinux.org/man/memfd_secret.2.en) +* [memfd_secret](https://man.archlinux.org/man/memfd_secret.2.en) From 0f0bc74af15df0120b08a640f02926579b1616fb Mon Sep 17 00:00:00 2001 From: Prabhpreet Dua <615318+prabhpreet@users.noreply.github.com> Date: Thu, 13 Jun 2024 11:44:42 +0530 Subject: [PATCH 3/6] Review changes --- src/alloc/allocext/linux.rs | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/alloc/allocext/linux.rs b/src/alloc/allocext/linux.rs index 6f5e278..b39225c 100644 --- a/src/alloc/allocext/linux.rs +++ b/src/alloc/allocext/linux.rs @@ -15,8 +15,9 @@ mod memfd_secret_alloc { use super::*; use core::convert::TryInto; + /// Allocate memfd_secret with given size and optionally at given address ptr #[inline] - pub unsafe fn alloc_memfd_secret( + pub unsafe fn alloc_memfd_secret_at_ptr( size: usize, ptr: Option<*mut libc::c_void>, ) -> Option<(NonNull, libc::c_int)> { @@ -28,11 +29,7 @@ mod memfd_secret_alloc { let _ = libc::ftruncate(fd, size as libc::off_t); let ptr = libc::mmap( - if ptr.is_some() { - ptr.unwrap() - } else { - ptr::null_mut() - }, + ptr.unwrap_or_else(ptr::null_mut), size, Prot::ReadWrite, libc::MAP_SHARED @@ -68,7 +65,7 @@ unsafe fn _memfd_secret(size: usize) -> Option<*mut u8> { let total_size = front_guard_size + unprotected_size + back_guard_size; let base_ptr = libc::mmap( - null_mut(), + ptr::null_mut(), total_size, PROT_NONE, MAP_SHARED | MAP_ANONYMOUS | MAP_NORESERVE, @@ -97,7 +94,7 @@ unsafe fn _memfd_secret(size: usize) -> Option<*mut u8> { let unprotected_ptr = base_ptr.add(front_guard_size); let Some((unprotected_ptr_val, fd)) = - alloc_memfd_secret(unprotected_size, Some(unprotected_ptr as *mut libc::c_void)) + alloc_memfd_secret_at_ptr(unprotected_size, Some(unprotected_ptr as *mut libc::c_void)) else { libc::munmap(base_ptr_stored as *mut libc::c_void, PAGE_SIZE); libc::munmap(base_ptr as *mut libc::c_void, total_size); From 24eb2d8c4917fab4e0271660625f28d35c3d0814 Mon Sep 17 00:00:00 2001 From: Prabhpreet Dua <615318+prabhpreet@users.noreply.github.com> Date: Thu, 13 Jun 2024 11:46:01 +0530 Subject: [PATCH 4/6] Cargo clippy changes --- src/alloc/allocext/linux.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/alloc/allocext/linux.rs b/src/alloc/allocext/linux.rs index b39225c..3320f6e 100644 --- a/src/alloc/allocext/linux.rs +++ b/src/alloc/allocext/linux.rs @@ -4,7 +4,7 @@ use libc::{MAP_ANONYMOUS, MAP_FIXED, MAP_NORESERVE, MAP_SHARED, PROT_NONE}; use self::std::process::abort; use crate::{alloc::*, Prot}; use core::mem::{self, size_of}; -use core::ptr::{self, null_mut, NonNull}; +use core::ptr::{self, NonNull}; use core::slice; use self::memfd_secret_alloc::*; From e6ffa13df2045e2b396c4b499351f6f54408eaca Mon Sep 17 00:00:00 2001 From: Prabhpreet Dua <615318+prabhpreet@users.noreply.github.com> Date: Thu, 13 Jun 2024 11:52:20 +0530 Subject: [PATCH 5/6] Additional doc comment --- src/alloc/allocext/linux.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/alloc/allocext/linux.rs b/src/alloc/allocext/linux.rs index 3320f6e..bcc5512 100644 --- a/src/alloc/allocext/linux.rs +++ b/src/alloc/allocext/linux.rs @@ -16,6 +16,7 @@ mod memfd_secret_alloc { use core::convert::TryInto; /// Allocate memfd_secret with given size and optionally at given address ptr + /// Returns tuple of ptr to memory and file descriptor of memfd_secret #[inline] pub unsafe fn alloc_memfd_secret_at_ptr( size: usize, From f03957067048c7371f4007c27494295f004bee83 Mon Sep 17 00:00:00 2001 From: Prabhpreet Dua <615318+prabhpreet@users.noreply.github.com> Date: Thu, 13 Jun 2024 12:56:44 +0530 Subject: [PATCH 6/6] Remove optional ptr argument in alloc_memfd_secret_at_ptr --- src/alloc/allocext/linux.rs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/alloc/allocext/linux.rs b/src/alloc/allocext/linux.rs index bcc5512..9ff23d3 100644 --- a/src/alloc/allocext/linux.rs +++ b/src/alloc/allocext/linux.rs @@ -15,12 +15,12 @@ mod memfd_secret_alloc { use super::*; use core::convert::TryInto; - /// Allocate memfd_secret with given size and optionally at given address ptr + /// Allocate memfd_secret with given size at given address ptr /// Returns tuple of ptr to memory and file descriptor of memfd_secret #[inline] pub unsafe fn alloc_memfd_secret_at_ptr( size: usize, - ptr: Option<*mut libc::c_void>, + ptr: *mut libc::c_void, ) -> Option<(NonNull, libc::c_int)> { let fd: Result = libc::syscall(libc::SYS_memfd_secret, 0).try_into(); @@ -29,23 +29,25 @@ mod memfd_secret_alloc { // File size is set using ftruncate let _ = libc::ftruncate(fd, size as libc::off_t); - let ptr = libc::mmap( - ptr.unwrap_or_else(ptr::null_mut), + let ptr_out = libc::mmap( + ptr, size, Prot::ReadWrite, - libc::MAP_SHARED - | if ptr.is_some() { libc::MAP_FIXED } else { 0 } - | MAP_LOCKED - | MAP_POPULATE, + libc::MAP_SHARED | libc::MAP_FIXED | MAP_LOCKED | MAP_POPULATE, fd, 0, ); - if ptr == libc::MAP_FAILED { + if ptr_out == libc::MAP_FAILED { return None; } - NonNull::new(ptr as *mut u8).map(|ptr| (ptr, fd)) + if ptr_out != ptr { + libc::munmap(ptr_out, size); + return None; + } + + NonNull::new(ptr_out as *mut u8).map(|ptr| (ptr, fd)) } } @@ -95,7 +97,7 @@ unsafe fn _memfd_secret(size: usize) -> Option<*mut u8> { let unprotected_ptr = base_ptr.add(front_guard_size); let Some((unprotected_ptr_val, fd)) = - alloc_memfd_secret_at_ptr(unprotected_size, Some(unprotected_ptr as *mut libc::c_void)) + alloc_memfd_secret_at_ptr(unprotected_size, unprotected_ptr as *mut libc::c_void) else { libc::munmap(base_ptr_stored as *mut libc::c_void, PAGE_SIZE); libc::munmap(base_ptr as *mut libc::c_void, total_size);