Repository: hannobraun/inotify-rs (language: rust)

Issue:
Consider addressing buffer alignment in a more robust way
See #155 and and #171 (specifically https://github.com/hannobraun/inotify-rs/pull/171#pullrequestreview-655632788) for context.


Discussion / hints:

Fix patch:
diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml
index 76355ff..c84931c 100644
--- a/.github/workflows/rust.yml
+++ b/.github/workflows/rust.yml
@@ -29,3 +29,16 @@ jobs:
       run: cargo test --verbose --no-default-features
     - name: Run Clippy lints (no default features)
       run: cargo clippy --verbose --no-default-features -- -D warnings
+
+  miri:
+    name: "Miri"
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - name: Install Miri
+        run: |
+          rustup toolchain install nightly --component miri
+          rustup override set nightly
+          cargo miri setup
+      - name: Test with Miri tests with "from_buffer" in name
+        run: cargo miri test from_buffer
diff --git a/src/events.rs b/src/events.rs
index 9ea1ccc..1cd508c 100644
--- a/src/events.rs
+++ b/src/events.rs
@@ -12,7 +12,6 @@ use inotify_sys as ffi;
 
 use crate::fd_guard::FdGuard;
 use crate::watches::WatchDescriptor;
-use crate::util::align_buffer;
 
 
 /// Iterator over inotify events
@@ -150,37 +149,32 @@ impl<'a> Event<&'a OsStr> {
         -> (usize, Self)
     {
         let event_size = mem::size_of::<ffi::inotify_event>();
-        let event_align = mem::align_of::<ffi::inotify_event>();
 
-        // Make sure that the buffer can satisfy the alignment requirements for `inotify_event`
-        assert!(buffer.len() >= event_align);
-
-        // Discard the unaligned portion, if any, of the supplied buffer
-        let buffer = align_buffer(buffer);
-
-        // Make sure that the aligned buffer is big enough to contain an event, without
+        // Make sure that the buffer is big enough to contain an event, without
         // the name. Otherwise we can't safely convert it to an `inotify_event`.
         assert!(buffer.len() >= event_size);
 
-
-        let event = buffer.as_ptr() as *const ffi::inotify_event;
+        let ffi_event_ptr = buffer.as_ptr() as *const ffi::inotify_event;
 
         // We have a pointer to an `inotify_event`, pointing to the beginning of
         // `buffer`. Since we know, as per the assertion above, that there are
         // enough bytes in the buffer for at least one event, we can safely
-        // convert that pointer into a reference.
-        let event = unsafe { &*event };
+        // read that `inotify_event`.
+        // We call `read_unaligned()` since the byte buffer has alignment 1
+        // and `inotify_event` has a higher alignment, so `*` cannot be used to dereference
+        // the unaligned pointer (undefined behavior).
+        let ffi_event = unsafe { ffi_event_ptr.read_unaligned() };
 
         // The name's length is given by `event.len`. There should always be
         // enough bytes left in the buffer to fit the name. Let's make sure that
         // is the case.
         let bytes_left_in_buffer = buffer.len() - event_size;
-        assert!(bytes_left_in_buffer >= event.len as usize);
+        assert!(bytes_left_in_buffer >= ffi_event.len as usize);
 
         // Directly after the event struct should be a name, if there's one
         // associated with the event. Let's make a new slice that starts with
         // that name. If there's no name, this slice might have a length of `0`.
-        let bytes_consumed = event_size + event.len as usize;
+        let bytes_consumed = event_size + ffi_event.len as usize;
         let name = &buffer[event_size..bytes_consumed];
 
         // Remove trailing '\0' bytes
@@ -198,7 +192,7 @@ impl<'a> Event<&'a OsStr> {
 
         let event = Event::new(
             fd,
-            event,
+            &ffi_event,
             OsStr::from_bytes(name),
         );
 
@@ -408,8 +402,6 @@ mod tests {
         sync,
     };
 
-    use crate::util;
-
     use inotify_sys as ffi;
 
     use super::Event;
@@ -419,9 +411,6 @@ mod tests {
     fn from_buffer_should_not_mistake_next_event_for_name_of_previous_event() {
         let mut buffer = [0u8; 1024];
 
-        // Make sure the buffer is properly aligned before writing raw events into it
-        let buffer = util::align_buffer_mut(&mut buffer);
-
         // First, put a normal event into the buffer
         let event = ffi::inotify_event {
             wd:     0,
diff --git a/src/util.rs b/src/util.rs
index 25ddb03..24f8eed 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -15,9 +15,6 @@ const INOTIFY_EVENT_SIZE: usize = mem::size_of::<ffi::inotify_event>() + 257;
 
 pub fn read_into_buffer(fd: RawFd, buffer: &mut [u8]) -> isize {
     unsafe {
-        // Discard the unaligned portion, if any, of the supplied buffer
-        let buffer = align_buffer_mut(buffer);
-
         ffi::read(
             fd,
             buffer.as_mut_ptr() as *mut c_void,
@@ -26,26 +23,6 @@ pub fn read_into_buffer(fd: RawFd, buffer: &mut [u8]) -> isize {
     }
 }
 
-pub fn align_buffer(buffer: &[u8]) -> &[u8] {
-    if buffer.len() >= mem::align_of::<ffi::inotify_event>() {
-        let ptr = buffer.as_ptr();
-        let offset = ptr.align_offset(mem::align_of::<ffi::inotify_event>());
-        &buffer[offset..]
-    } else {
-        &buffer[0..0]
-    }
-}
-
-pub fn align_buffer_mut(buffer: &mut [u8]) -> &mut [u8] {
-   if buffer.len() >= mem::align_of::<ffi::inotify_event>() {
-        let ptr = buffer.as_mut_ptr();
-        let offset = ptr.align_offset(mem::align_of::<ffi::inotify_event>());
-        &mut buffer[offset..]
-   } else {
-       &mut buffer[0..0]
-   }
-}
-
 /// Get the inotify event buffer size
 ///
 /// The maximum size of an inotify event and thus the buffer size to hold it


Test patch: