rust: merge Formatter and Buffer

Also fix soundness problems with original `Formatter` when the buffer
was `null` and with construction.

Signed-off-by: Wedson Almeida Filho <[email protected]>

Author: wedsonaf

rust/kernel/buffer.rs

@@ -43,7 +43,6 @@ pub mod bindings;
 
 #[cfg(CONFIG_ARM_AMBA)]
 pub mod amba;
-pub mod buffer;
 pub mod c_types;
 pub mod chrdev;
 #[cfg(CONFIG_COMMON_CLK)]

rust/kernel/lib.rs

@@ -4,7 +4,8 @@
 //!
 //! C header: [`include/linux/moduleparam.h`](../../../include/linux/moduleparam.h)
 
-use crate::str::CStr;
+use crate::error::from_kernel_result;
+use crate::str::{CStr, Formatter};
 use core::fmt::Write;
 
 /// Types that can be used for module parameters.
@@ -95,11 +96,11 @@ pub trait ModuleParam: core::fmt::Display + core::marker::Sized {
         buf: *mut crate::c_types::c_char,
         param: *const crate::bindings::kernel_param,
     ) -> crate::c_types::c_int {
-        let slice = unsafe { core::slice::from_raw_parts_mut(buf as *mut u8, crate::PAGE_SIZE) };
-        let mut buf = crate::buffer::Buffer::new(slice);
-        match unsafe { write!(buf, "{}\0", *((*param).__bindgen_anon_1.arg as *mut Self)) } {
-            Err(_) => crate::error::Error::EINVAL.to_kernel_errno(),
-            Ok(()) => buf.bytes_written() as crate::c_types::c_int,
+        from_kernel_result! {
+            // SAFETY: The C contracts guarantees that the buffer is at least `PAGE_SIZE` bytes.
+            let mut f = unsafe { Formatter::from_buffer(buf.cast(), crate::PAGE_SIZE) };
+            unsafe { write!(f, "{}\0", *((*param).__bindgen_anon_1.arg as *mut Self)) }?;
+            Ok(f.bytes_written().try_into()?)
         }
     }
 

rust/kernel/module_param.rs

@@ -10,7 +10,7 @@ use core::fmt;
 
 use crate::{
     c_types::{c_char, c_void},
-    str::Formatter,
+    str::RawFormatter,
 };
 
 #[cfg(CONFIG_PRINTK)]
@@ -20,13 +20,10 @@ use crate::bindings;
 #[no_mangle]
 unsafe fn rust_fmt_argument(buf: *mut c_char, end: *mut c_char, ptr: *const c_void) -> *mut c_char {
     use fmt::Write;
-
-    let mut w = Formatter {
-        buf: buf as _,
-        end: end as _,
-    };
+    // SAFETY: The C contract guarantees that `buf` is valid if it's less than `end`.
+    let mut w = unsafe { RawFormatter::from_ptrs(buf.cast(), end.cast()) };
     let _ = w.write_fmt(unsafe { *(ptr as *const fmt::Arguments<'_>) });
-    w.buf as _
+    w.pos().cast()
 }
 
 /// Format strings.

rust/kernel/print.rs

@@ -374,28 +374,129 @@ mod tests {
     }
 }
 
-// Use `usize` to use `saturating_*` functions.
-pub(crate) struct Formatter {
-    pub(crate) buf: usize,
-    pub(crate) end: usize,
+/// Allows formatting of [`fmt::Arguments`] into a raw buffer.
+///
+/// It does not fail if callers write past the end of the buffer so that they can calculate the
+/// size required to fit everything.
+///
+/// # Invariants
+///
+/// The memory region between `pos` (inclusive) and `end` (exclusive) is valid for writes if `pos`
+/// is less than `end`.
+pub(crate) struct RawFormatter {
+    // Use `usize` to use `saturating_*` functions.
+    beg: usize,
+    pos: usize,
+    end: usize,
 }
 
-impl fmt::Write for Formatter {
+impl RawFormatter {
+    /// Creates a new instance of [`RawFormatter`] with the given buffer pointers.
+    ///
+    /// # Safety
+    ///
+    /// If `pos` is less than `end`, then the region between `pos` (inclusive) and `end`
+    /// (exclusive) must be valid for writes for the lifetime of the returned [`RawFormatter`].
+    pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
+        // INVARIANT: The safety requierments guarantee the type invariants.
+        Self {
+            beg: pos as _,
+            pos: pos as _,
+            end: end as _,
+        }
+    }
+
+    /// Creates a new instance of [`RawFormatter`] with the given buffer.
+    ///
+    /// # Safety
+    ///
+    /// The memory region starting at `buf` and extending for `len` bytes must be valid for writes
+    /// for the lifetime of the returned [`RawFormatter`].
+    pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
+        let pos = buf as usize;
+        // INVARIANT: We ensure that `end` is never less then `buf`, and the safety requirements
+        // guarantees that the memory region is valid for writes.
+        Self {
+            pos,
+            beg: pos,
+            end: pos.saturating_add(len),
+        }
+    }
+
+    /// Returns the current insert position.
+    ///
+    /// N.B. It may point to invalid memory.
+    pub(crate) fn pos(&self) -> *mut u8 {
+        self.pos as _
+    }
+
+    /// Return the number of bytes written to the formatter.
+    pub(crate) fn bytes_written(&self) -> usize {
+        self.pos - self.beg
+    }
+}
+
+impl fmt::Write for RawFormatter {
     fn write_str(&mut self, s: &str) -> fmt::Result {
-        // `buf` value after writing `len` bytes. This does not have to be bounded by `end`, but we
+        // `pos` value after writing `len` bytes. This does not have to be bounded by `end`, but we
         // don't want it to wrap around to 0.
-        let buf_new = self.buf.saturating_add(s.len());
+        let pos_new = self.pos.saturating_add(s.len());
+
+        // Amount that we can copy. `saturating_sub` ensures we get 0 if `pos` goes past `end`.
+        let len_to_copy = core::cmp::min(pos_new, self.end).saturating_sub(self.pos);
+
+        if len_to_copy > 0 {
+            // SAFETY: If `len_to_copy` is non-zero, then we know `pos` has not gone past `end`
+            // yet, so it is valid for write per the type invariants.
+            unsafe {
+                core::ptr::copy_nonoverlapping(
+                    s.as_bytes().as_ptr(),
+                    self.pos as *mut u8,
+                    len_to_copy,
+                )
+            };
+        }
 
-        // Amount that we can copy. `saturating_sub` ensures we get 0 if `buf` goes past `end`.
-        let len_to_copy = core::cmp::min(buf_new, self.end).saturating_sub(self.buf);
+        self.pos = pos_new;
+        Ok(())
+    }
+}
 
-        // SAFETY: In any case, `buf` is non-null and properly aligned.  If `len_to_copy` is
-        // non-zero, then we know `buf` has not past `end` yet and so is valid.
-        unsafe {
-            core::ptr::copy_nonoverlapping(s.as_bytes().as_ptr(), self.buf as *mut u8, len_to_copy)
-        };
+/// Allows formatting of [`fmt::Arguments`] into a raw buffer.
+///
+/// Fails if callers attempt to write more than will fit in the buffer.
+pub(crate) struct Formatter(RawFormatter);
 
-        self.buf = buf_new;
-        Ok(())
+impl Formatter {
+    /// Creates a new instance of [`Formatter`] with the given buffer.
+    ///
+    /// # Safety
+    ///
+    /// The memory region starting at `buf` and extending for `len` bytes must be valid for writes
+    /// for the lifetime of the returned [`Formatter`].
+    pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
+        // SAFETY: The safety requirements of this function satisfy those of the callee.
+        Self(unsafe { RawFormatter::from_buffer(buf, len) })
+    }
+}
+
+impl Deref for Formatter {
+    type Target = RawFormatter;
+
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+
+impl fmt::Write for Formatter {
+    fn write_str(&mut self, s: &str) -> fmt::Result {
+        self.0.write_str(s)?;
+
+        // Fail the request if we go past the end of the buffer.
+        if self.0.pos > self.0.end {
+            Err(fmt::Error)
+        } else {
+            Ok(())
+        }
     }
 }