Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add safety comments to unsafe attributes #592

Merged
merged 4 commits into from
Jan 2, 2025
Merged

Add safety comments to unsafe attributes #592

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1c24991
Add safety comments to unsafe attributes
smalis-msft Dec 30, 2024
84d7bb0
Feedback
smalis-msft Dec 30, 2024
8ba4b28
Fmt and vmgs too
smalis-msft Dec 30, 2024
53373c2
feedback
smalis-msft Jan 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion openhcl/build_info/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,11 @@ impl BuildInfo {
// a debugger. With a debugger, the non-mangled name is easier
// to use.

// UNSAFETY: link_section and export_name are considered unsafe.
// UNSAFETY: link_section and export_name are unsafe.
#[expect(unsafe_code)]
// SAFETY: The build_info section is custom and carries no safety requirements.
#[unsafe(link_section = ".build_info")]
// SAFETY: The name "BUILD_INFO" does not collide with any other symbols.
smalis-msft marked this conversation as resolved.
Show resolved Hide resolved
#[unsafe(export_name = "BUILD_INFO")]
static BUILD_INFO: BuildInfo = BuildInfo::new();

Expand Down
4 changes: 4 additions & 0 deletions openhcl/minimal_rt/src/arch/aarch64/intrinsics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

/// Hand rolled implementation of memcpy.
#[cfg(minimal_rt)]
// SAFETY: The minimal_rt_build crate ensures that when this code is compiled
// there is no libc for this to conflict with.
#[unsafe(no_mangle)]
unsafe extern "C" fn memcpy(mut dest: *mut u8, src: *const u8, len: usize) -> *mut u8 {
// SAFETY: the caller guarantees the pointers and length are correct.
Expand All @@ -31,6 +33,8 @@ unsafe extern "C" fn memcpy(mut dest: *mut u8, src: *const u8, len: usize) -> *m

/// Hand rolled implementation of memset.
#[cfg(minimal_rt)]
// SAFETY: The minimal_rt_build crate ensures that when this code is compiled
// there is no libc for this to conflict with.
#[unsafe(no_mangle)]
unsafe extern "C" fn memset(mut ptr: *mut u8, val: i32, len: usize) -> *mut u8 {
// SAFETY: the caller guarantees the pointer and length are correct.
Expand Down
4 changes: 4 additions & 0 deletions openhcl/minimal_rt/src/arch/x86_64/intrinsics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

/// Hand rolled implementation of memset.
#[cfg(minimal_rt)]
// SAFETY: The minimal_rt_build crate ensures that when this code is compiled
// there is no libc for this to conflict with.
#[unsafe(no_mangle)]
unsafe extern "C" fn memset(mut ptr: *mut u8, val: i32, len: usize) -> *mut u8 {
// SAFETY: The caller guarantees that the pointer and length are correct.
Expand All @@ -22,6 +24,8 @@ unsafe extern "C" fn memset(mut ptr: *mut u8, val: i32, len: usize) -> *mut u8 {

/// Hand rolled implementation of memcpy.
#[cfg(minimal_rt)]
// SAFETY: The minimal_rt_build crate ensures that when this code is compiled
// there is no libc for this to conflict with.
#[unsafe(no_mangle)]
unsafe extern "C" fn memcpy(mut dest: *mut u8, src: *const u8, len: usize) -> *mut u8 {
// SAFETY: The caller guarantees that the pointers and length are correct.
Expand Down
8 changes: 6 additions & 2 deletions openhcl/minimal_rt/src/rt.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@ mod instead_of_builtins {
fn memcpy(dest: *mut u8, src: *const u8, n: usize) -> *mut u8;
}

/// Implementation cribbed from compiler_builtins.
// SAFETY: The minimal_rt_build crate ensures that when this code is compiled
// there is no libc for this to conflict with.
#[unsafe(no_mangle)]
/// Implementation cribbed from compiler_builtins.
smalis-msft marked this conversation as resolved.
Show resolved Hide resolved
unsafe extern "C" fn memmove(dest: *mut u8, src: *const u8, n: usize) -> *mut u8 {
let delta = (dest as usize).wrapping_sub(src as usize);
if delta >= n {
Expand All @@ -44,10 +46,12 @@ mod instead_of_builtins {
dest
}

// SAFETY: The minimal_rt_build crate ensures that when this code is compiled
// there is no libc for this to conflict with.
#[unsafe(no_mangle)]
/// This implementation is cribbed from compiler_builtins. It would be nice to
/// use those implementation for all the above functions, but those require
/// nightly as these are not yet stabilized.
#[unsafe(no_mangle)]
unsafe extern "C" fn bcmp(s1: *const u8, s2: *const u8, n: usize) -> i32 {
// SAFETY: The caller guarantees that the pointers and length are correct.
unsafe {
Expand Down
4 changes: 3 additions & 1 deletion support/openssl_crypto_only/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,12 @@ unsafe extern "C" {
#[macro_export]
macro_rules! openssl_crypto_only {
() => {
// SAFETY: We are purposefully overriding this symbol and we have made
// sure the definition is compatible with the original.
#[unsafe(no_mangle)]
/// # Safety
///
/// The caller must call as documented for `OPENSSL_init_ssl`.
#[unsafe(no_mangle)]
unsafe extern "C" fn OPENSSL_init_ssl(
opts: u64,
settings: *const ::core::ffi::c_void,
Expand Down
20 changes: 18 additions & 2 deletions support/win_prng_support/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,25 @@ macro_rules! use_win10_prng_apis {
$($crate::use_win10_prng_apis!(@x $lib);)*
};
(@x advapi32) => {
// SAFETY: We are purposefully overriding this symbol and we have made
smalis-msft marked this conversation as resolved.
Show resolved Hide resolved
// sure the definition is compatible with the original.
#[unsafe(no_mangle)]
pub unsafe extern "system" fn SystemFunction036(data: *mut u8, len: u32) -> u8 {
// SAFETY: passing through guarantees.
unsafe { $crate::private::SystemFunction036(data, len) }
}

// SAFETY: We are purposefully overriding this symbol and we have made
// sure the definition is compatible with the original.
#[unsafe(no_mangle)]
/// If a call to SystemFunction036 is marked as a dllimport, then it may be an indirect call
/// through __imp_SystemFunction036 instead.
#[unsafe(no_mangle)]
pub static __imp_SystemFunction036: unsafe extern "system" fn(*mut u8, u32) -> u8 =
SystemFunction036;
};
(@x bcrypt) => {
// SAFETY: We are purposefully overriding this symbol and we have made
// sure the definition is compatible with the original.
#[unsafe(no_mangle)]
pub unsafe extern "system" fn BCryptOpenAlgorithmProvider(
handle: *mut ::core::ffi::c_void,
Expand All @@ -56,6 +62,8 @@ macro_rules! use_win10_prng_apis {
}
}

// SAFETY: We are purposefully overriding this symbol and we have made
// sure the definition is compatible with the original.
#[unsafe(no_mangle)]
pub unsafe extern "system" fn BCryptCloseAlgorithmProvider(
handle: *mut ::core::ffi::c_void,
Expand All @@ -65,6 +73,8 @@ macro_rules! use_win10_prng_apis {
unsafe { $crate::private::BCryptCloseAlgorithmProvider(handle, flags) }
}

// SAFETY: We are purposefully overriding this symbol and we have made
// sure the definition is compatible with the original.
#[unsafe(no_mangle)]
pub unsafe extern "system" fn BCryptGenRandom(
algorithm: usize,
Expand All @@ -76,16 +86,20 @@ macro_rules! use_win10_prng_apis {
unsafe { $crate::private::BCryptGenRandom(algorithm, data, len, flags) }
}

// SAFETY: We are purposefully overriding this symbol and we have made
// sure the definition is compatible with the original.
#[unsafe(no_mangle)]
/// If a call to BCryptGenRandom is marked as a dllimport, then it may be an indirect call
/// through __imp_BCryptGenRandom instead.
#[unsafe(no_mangle)]
pub static __imp_BCryptGenRandom: unsafe extern "system" fn(
usize,
*mut u8,
u32,
u32,
) -> u32 = BCryptGenRandom;

// SAFETY: We are purposefully overriding this symbol and we have made
// sure the definition is compatible with the original.
#[unsafe(no_mangle)]
pub static __imp_BCryptOpenAlgorithmProvider: unsafe extern "system" fn(
*mut ::core::ffi::c_void,
Expand All @@ -94,6 +108,8 @@ macro_rules! use_win10_prng_apis {
u32,
) -> u32 = BCryptOpenAlgorithmProvider;

// SAFETY: We are purposefully overriding this symbol and we have made
// sure the definition is compatible with the original.
#[unsafe(no_mangle)]
pub static __imp_BCryptCloseAlgorithmProvider: unsafe extern "system" fn(
*mut ::core::ffi::c_void,
Expand Down
16 changes: 12 additions & 4 deletions vm/vmgs/vmgs_lib/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@ pub enum VmgsError {
FileExists = 14,
}

// SAFETY: We are exporting our own C-compatible API. In this library
// this function name is unique.
#[unsafe(no_mangle)]
/// Read the contents of a `FileId` in a VMGS file
///
/// # Safety
///
/// `file_path` must point to a valid null-terminated utf-8 string.
/// `in_len` must be the size of `in_buf` in bytes and match the value returned from query_size_vmgs
#[unsafe(no_mangle)]
pub unsafe extern "C" fn read_vmgs(
file_path: *const c_char,
file_id: FileId,
Expand Down Expand Up @@ -146,13 +148,15 @@ async fn do_read(
Ok(data)
}

// SAFETY: We are exporting our own C-compatible API. In this library
// this function name is unique.
#[unsafe(no_mangle)]
/// Write from a data file to a `FileId` in a VMGS file
///
/// # Safety
///
/// `file_path` and `data_path` must point to valid null-terminated utf-8 strings.
/// `encryption_key` must be null-terminated and nonnull if using encryption
#[unsafe(no_mangle)]
pub unsafe extern "C" fn write_vmgs(
file_path: *const c_char,
data_path: *const c_char,
Expand Down Expand Up @@ -229,6 +233,9 @@ async fn do_write(
Ok(())
}

// SAFETY: We are exporting our own C-compatible API. In this library
// this function name is unique.
#[unsafe(no_mangle)]
/// Create a VMGS file
///
/// If `file_size` is zero, default size of 4MB is used
Expand All @@ -239,7 +246,6 @@ async fn do_write(
/// # Safety
///
/// `path` must point to a valid null-terminated utf-8 string.
#[unsafe(no_mangle)]
pub unsafe extern "C" fn create_vmgs(
path: *const c_char,
file_size: u64,
Expand Down Expand Up @@ -329,13 +335,15 @@ async fn do_create(
Ok(())
}

// SAFETY: We are exporting our own C-compatible API. In this library
// this function name is unique.
#[unsafe(no_mangle)]
/// Get the size of a `FileId` in a VMGS file
///
/// # Safety
///
/// `path` pointer must point to a valid, null-terminated utf-8 string.
/// `out_size` pointer must be nonnull
#[unsafe(no_mangle)]
pub unsafe extern "C" fn query_size_vmgs(
path: *const c_char,
file_id: FileId,
Expand Down
Loading