From 3aa26c19a3ade3c6c0559f168f0a85f89654ffee Mon Sep 17 00:00:00 2001
From: R1kaB3rN <100738684+R1kaB3rN@users.noreply.github.com>
Date: Thu, 12 Dec 2024 13:52:36 -0800
Subject: [PATCH] lib: document all usages of unsafe

---
 src/lib.rs | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/src/lib.rs b/src/lib.rs
index f70eecb2c..3d22b2eb4 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -15,26 +15,41 @@ use std::os::unix::io::FromRawFd;
 fn bspatch_rs(py: Python<'_>, source: i32, patch: &[u8]) -> io::Result<Vec<u8>> {
     py.allow_threads(|| {
         let patcher = Bspatch::new(patch)?;
+
+        // We pass the file descriptor from Python, which is foreign to Rust
+        // and handling them is currently always unsafe. While we could pass
+        // a string representing the absolute file path instead, it'll come at
+        // higher conversion and path resolution costs
+        // See https://pyo3.rs/v0.23.1/conversions/tables.html#using-rust-library-types-vs-python-native-types
         let file = unsafe { File::from_raw_fd(source) };
         let original_size = file.metadata()?.len();
+
         // Ensure file is resized to accommodate the patch
         if original_size < patcher.hint_target_size() {
             file.set_len(patcher.hint_target_size())?;
         }
+
+        // See https://docs.rs/memmap2/0.9.5/memmap2/struct.MmapMut.html#safety
+        // In context of applying partial updates, umu-launcher mitigates this
+        // risk by holding a lock in Python's context before applying the
+        // partial update in Rust and ensures mutual exclusivity of file access
         let mut mmap = unsafe {
             MmapMut::map_mut(&file).map_err(|e| {
                 io::Error::new(io::ErrorKind::Other, format!("Failed to map source: {}", e))
             })?
         };
+
         // Don't run the destructor. We'll manage the file descriptor in Python
         std::mem::forget(file);
         let mut target = Vec::with_capacity(patcher.hint_target_size() as usize);
+
         // Optimization. Let the kernel know the specific ranges we're
         // accessing. Here, we only need to access up to the original's
         // length
         mmap.advise_range(Advice::Sequential, 0, original_size as usize)
             .unwrap();
         patcher.apply(&mmap[..original_size as usize], &mut target)?;
+
         // Validate target size before writing to mmap
         if target.len() > mmap.len() {
             return Err(io::Error::new(
@@ -46,6 +61,7 @@ fn bspatch_rs(py: Python<'_>, source: i32, patch: &[u8]) -> io::Result<Vec<u8>>
         mmap.advise_range(Advice::Sequential, 0, target.len())
             .unwrap();
         mmap[..target.len()].copy_from_slice(&target[..]);
+
         // Handle small file case
         if target.len() < original_size as usize {
             let file = unsafe { File::from_raw_fd(source) };
@@ -60,6 +76,7 @@ fn bspatch_rs(py: Python<'_>, source: i32, patch: &[u8]) -> io::Result<Vec<u8>>
 #[pyfunction]
 fn bz2_decompress_rs(py: Python<'_>, source: &[u8], target: i32, size: u64) -> io::Result<u64> {
     py.allow_threads(|| {
+        // See bspatch_rs for rationale on the use of unsafe
         let mut file = unsafe { File::from_raw_fd(target) };
         file.set_len(size)?;
         let mut decoder = BzDecoder::new(source);
@@ -72,6 +89,7 @@ fn bz2_decompress_rs(py: Python<'_>, source: &[u8], target: i32, size: u64) -> i
 #[pyfunction]
 fn crc32_rs(py: Python<'_>, source: i32) -> io::Result<u32> {
     py.allow_threads(|| {
+        // See bspatch_rs for rationale on the use of unsafe
         let mut file = unsafe { File::from_raw_fd(source) };
         let mut hasher = Hasher::new();
         let mut buffer = Vec::new();
@@ -85,8 +103,10 @@ fn crc32_rs(py: Python<'_>, source: i32) -> io::Result<u32> {
 #[pyfunction]
 fn crc32_mmap_rs(py: Python<'_>, source: i32) -> u32 {
     py.allow_threads(|| {
+        // See bspatch_rs for rationale on the use of unsafe
         let file = unsafe { File::from_raw_fd(source) };
         let mmap = unsafe { Mmap::map(&file).ok() };
+
         let mut hasher = Hasher::new();
         if let Some(mmap) = mmap {
             hasher.update(&mmap[..]);