Merge pull request #348 from ojeda/warnings

Clean `unsafe` in `unsafe fn`s warnings
Rust-for-Linux · Jun 4, 2021 · cae4454 · cae4454
2 parents 0b9bcdf + a0ea20f
commit cae4454
Show file tree

Hide file tree

Showing 21 changed files with 114 additions and 98 deletions.
diff --git a/Makefile b/Makefile
@@ -516,7 +516,7 @@ KBUILD_RUSTCFLAGS := --emit=dep-info,obj,metadata --edition=2018 \
 		     -Cpanic=abort -Cembed-bitcode=n -Clto=n -Crpath=n \
 		     -Cforce-unwind-tables=n -Ccodegen-units=1 \
 		     -Zbinary_dep_depinfo=y -Zsymbol-mangling-version=v0 \
-		     -W unsafe_op_in_unsafe_fn
+		     -D unsafe_op_in_unsafe_fn
 KBUILD_AFLAGS_KERNEL :=
 KBUILD_CFLAGS_KERNEL :=
 KBUILD_RUSTCFLAGS_KERNEL :=

diff --git a/drivers/android/node.rs b/drivers/android/node.rs
@@ -83,12 +83,14 @@ impl NodeDeath {
             cookie,
             work_links: Links::new(),
             death_links: Links::new(),
-            inner: SpinLock::new(NodeDeathInner {
-                dead: false,
-                cleared: false,
-                notification_done: false,
-                aborted: false,
-            }),
+            inner: unsafe {
+                SpinLock::new(NodeDeathInner {
+                    dead: false,
+                    cleared: false,
+                    notification_done: false,
+                    aborted: false,
+                })
+            },
         }
     }
 

diff --git a/rust/kernel/allocator.rs b/rust/kernel/allocator.rs
@@ -14,11 +14,13 @@ unsafe impl GlobalAlloc for KernelAllocator {
     unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
         // `krealloc()` is used instead of `kmalloc()` because the latter is
         // an inline function and cannot be bound to as a result.
-        bindings::krealloc(ptr::null(), layout.size(), bindings::GFP_KERNEL) as *mut u8
+        unsafe { bindings::krealloc(ptr::null(), layout.size(), bindings::GFP_KERNEL) as *mut u8 }
     }
 
     unsafe fn dealloc(&self, ptr: *mut u8, _layout: Layout) {
-        bindings::kfree(ptr as *const c_types::c_void);
+        unsafe {
+            bindings::kfree(ptr as *const c_types::c_void);
+        }
     }
 }
 

diff --git a/rust/kernel/bindings.rs b/rust/kernel/bindings.rs
@@ -9,7 +9,8 @@
     non_camel_case_types,
     non_upper_case_globals,
     non_snake_case,
-    improper_ctypes
+    improper_ctypes,
+    unsafe_op_in_unsafe_fn
 )]
 mod bindings_raw {
     use crate::c_types;

diff --git a/rust/kernel/file_operations.rs b/rust/kernel/file_operations.rs
@@ -57,10 +57,10 @@ impl PollTable {
 
         // SAFETY: `PollTable::ptr` is guaranteed to be valid by the type invariants and the null
         // check above.
-        let table = &*self.ptr;
+        let table = unsafe { &*self.ptr };
         if let Some(proc) = table._qproc {
             // SAFETY: All pointers are known to be valid.
-            proc(file.ptr as _, cv.wait_list.get(), self.ptr)
+            unsafe { proc(file.ptr as _, cv.wait_list.get(), self.ptr) }
         }
     }
 }
@@ -84,9 +84,9 @@ unsafe extern "C" fn open_callback<A: FileOpenAdapter, T: FileOpener<A::Arg>>(
     file: *mut bindings::file,
 ) -> c_types::c_int {
     from_kernel_result! {
-        let arg = A::convert(inode, file);
-        let ptr = T::open(&*arg)?.into_pointer();
-        (*file).private_data = ptr as *mut c_types::c_void;
+        let arg = unsafe { A::convert(inode, file) };
+        let ptr = T::open(unsafe { &*arg })?.into_pointer();
+        unsafe { (*file).private_data = ptr as *mut c_types::c_void };
         Ok(0)
     }
 }
@@ -98,12 +98,12 @@ unsafe extern "C" fn read_callback<T: FileOperations>(
     offset: *mut bindings::loff_t,
 ) -> c_types::c_ssize_t {
     from_kernel_result! {
-        let mut data = UserSlicePtr::new(buf as *mut c_types::c_void, len).writer();
-        let f = &*((*file).private_data as *const T);
+        let mut data = unsafe { UserSlicePtr::new(buf as *mut c_types::c_void, len).writer() };
+        let f = unsafe { &*((*file).private_data as *const T) };
         // No `FMODE_UNSIGNED_OFFSET` support, so `offset` must be in [0, 2^63).
         // See discussion in https://github.com/fishinabarrel/linux-kernel-module-rust/pull/113
-        let read = f.read(&FileRef::from_ptr(file), &mut data, (*offset).try_into()?)?;
-        (*offset) += bindings::loff_t::try_from(read).unwrap();
+        let read = f.read(unsafe { &FileRef::from_ptr(file) }, &mut data, unsafe { *offset }.try_into()?)?;
+        unsafe { (*offset) += bindings::loff_t::try_from(read).unwrap() };
         Ok(read as _)
     }
 }
@@ -113,12 +113,12 @@ unsafe extern "C" fn read_iter_callback<T: FileOperations>(
     raw_iter: *mut bindings::iov_iter,
 ) -> isize {
     from_kernel_result! {
-        let mut iter = IovIter::from_ptr(raw_iter);
-        let file = (*iocb).ki_filp;
-        let offset = (*iocb).ki_pos;
-        let f = &*((*file).private_data as *const T);
-        let read = f.read(&FileRef::from_ptr(file), &mut iter, offset.try_into()?)?;
-        (*iocb).ki_pos += bindings::loff_t::try_from(read).unwrap();
+        let mut iter = unsafe { IovIter::from_ptr(raw_iter) };
+        let file = unsafe { (*iocb).ki_filp };
+        let offset = unsafe { (*iocb).ki_pos };
+        let f = unsafe { &*((*file).private_data as *const T) };
+        let read = f.read(unsafe { &FileRef::from_ptr(file) }, &mut iter, offset.try_into()?)?;
+        unsafe { (*iocb).ki_pos += bindings::loff_t::try_from(read).unwrap() };
         Ok(read as _)
     }
 }
@@ -130,12 +130,12 @@ unsafe extern "C" fn write_callback<T: FileOperations>(
     offset: *mut bindings::loff_t,
 ) -> c_types::c_ssize_t {
     from_kernel_result! {
-        let mut data = UserSlicePtr::new(buf as *mut c_types::c_void, len).reader();
-        let f = &*((*file).private_data as *const T);
+        let mut data = unsafe { UserSlicePtr::new(buf as *mut c_types::c_void, len).reader() };
+        let f = unsafe { &*((*file).private_data as *const T) };
         // No `FMODE_UNSIGNED_OFFSET` support, so `offset` must be in [0, 2^63).
         // See discussion in https://github.com/fishinabarrel/linux-kernel-module-rust/pull/113
-        let written = f.write(&FileRef::from_ptr(file), &mut data, (*offset).try_into()?)?;
-        (*offset) += bindings::loff_t::try_from(written).unwrap();
+        let written = f.write(unsafe { &FileRef::from_ptr(file) }, &mut data, unsafe { *offset }.try_into()?)?;
+        unsafe { (*offset) += bindings::loff_t::try_from(written).unwrap() };
         Ok(written as _)
     }
 }
@@ -145,12 +145,12 @@ unsafe extern "C" fn write_iter_callback<T: FileOperations>(
     raw_iter: *mut bindings::iov_iter,
 ) -> isize {
     from_kernel_result! {
-        let mut iter = IovIter::from_ptr(raw_iter);
-        let file = (*iocb).ki_filp;
-        let offset = (*iocb).ki_pos;
-        let f = &*((*file).private_data as *const T);
-        let written = f.write(&FileRef::from_ptr(file), &mut iter, offset.try_into()?)?;
-        (*iocb).ki_pos += bindings::loff_t::try_from(written).unwrap();
+        let mut iter = unsafe { IovIter::from_ptr(raw_iter) };
+        let file = unsafe { (*iocb).ki_filp };
+        let offset = unsafe { (*iocb).ki_pos };
+        let f = unsafe { &*((*file).private_data as *const T) };
+        let written = f.write(unsafe { &FileRef::from_ptr(file) }, &mut iter, offset.try_into()?)?;
+        unsafe { (*iocb).ki_pos += bindings::loff_t::try_from(written).unwrap() };
         Ok(written as _)
     }
 }
@@ -159,8 +159,10 @@ unsafe extern "C" fn release_callback<T: FileOperations>(
     _inode: *mut bindings::inode,
     file: *mut bindings::file,
 ) -> c_types::c_int {
-    let ptr = mem::replace(&mut (*file).private_data, ptr::null_mut());
-    T::release(T::Wrapper::from_pointer(ptr as _), &FileRef::from_ptr(file));
+    let ptr = mem::replace(unsafe { &mut (*file).private_data }, ptr::null_mut());
+    T::release(unsafe { T::Wrapper::from_pointer(ptr as _) }, unsafe {
+        &FileRef::from_ptr(file)
+    });
     0
 }
 
@@ -176,8 +178,8 @@ unsafe extern "C" fn llseek_callback<T: FileOperations>(
             bindings::SEEK_END => SeekFrom::End(offset),
             _ => return Err(Error::EINVAL),
         };
-        let f = &*((*file).private_data as *const T);
-        let off = f.seek(&FileRef::from_ptr(file), off)?;
+        let f = unsafe { &*((*file).private_data as *const T) };
+        let off = f.seek(unsafe { &FileRef::from_ptr(file) }, off)?;
         Ok(off as bindings::loff_t)
     }
 }
@@ -188,10 +190,10 @@ unsafe extern "C" fn unlocked_ioctl_callback<T: FileOperations>(
     arg: c_types::c_ulong,
 ) -> c_types::c_long {
     from_kernel_result! {
-        let f = &*((*file).private_data as *const T);
+        let f = unsafe { &*((*file).private_data as *const T) };
         // SAFETY: This function is called by the kernel, so it must set `fs` appropriately.
         let mut cmd = IoctlCommand::new(cmd as _, arg as _);
-        let ret = f.ioctl(&FileRef::from_ptr(file), &mut cmd)?;
+        let ret = f.ioctl(unsafe { &FileRef::from_ptr(file) }, &mut cmd)?;
         Ok(ret as _)
     }
 }
@@ -202,10 +204,10 @@ unsafe extern "C" fn compat_ioctl_callback<T: FileOperations>(
     arg: c_types::c_ulong,
 ) -> c_types::c_long {
     from_kernel_result! {
-        let f = &*((*file).private_data as *const T);
+        let f = unsafe { &*((*file).private_data as *const T) };
         // SAFETY: This function is called by the kernel, so it must set `fs` appropriately.
         let mut cmd = IoctlCommand::new(cmd as _, arg as _);
-        let ret = f.compat_ioctl(&FileRef::from_ptr(file), &mut cmd)?;
+        let ret = f.compat_ioctl(unsafe { &FileRef::from_ptr(file) }, &mut cmd)?;
         Ok(ret as _)
     }
 }
@@ -215,8 +217,8 @@ unsafe extern "C" fn mmap_callback<T: FileOperations>(
     vma: *mut bindings::vm_area_struct,
 ) -> c_types::c_int {
     from_kernel_result! {
-        let f = &*((*file).private_data as *const T);
-        f.mmap(&FileRef::from_ptr(file), &mut *vma)?;
+        let f = unsafe { &*((*file).private_data as *const T) };
+        f.mmap(unsafe { &FileRef::from_ptr(file) }, unsafe { &mut *vma })?;
         Ok(0)
     }
 }
@@ -231,8 +233,8 @@ unsafe extern "C" fn fsync_callback<T: FileOperations>(
         let start = start.try_into()?;
         let end = end.try_into()?;
         let datasync = datasync != 0;
-        let f = &*((*file).private_data as *const T);
-        let res = f.fsync(&FileRef::from_ptr(file), start, end, datasync)?;
+        let f = unsafe { &*((*file).private_data as *const T) };
+        let res = f.fsync(unsafe { &FileRef::from_ptr(file) }, start, end, datasync)?;
         Ok(res.try_into().unwrap())
     }
 }
@@ -241,8 +243,10 @@ unsafe extern "C" fn poll_callback<T: FileOperations>(
     file: *mut bindings::file,
     wait: *mut bindings::poll_table_struct,
 ) -> bindings::__poll_t {
-    let f = &*((*file).private_data as *const T);
-    match f.poll(&FileRef::from_ptr(file), &PollTable::from_ptr(wait)) {
+    let f = unsafe { &*((*file).private_data as *const T) };
+    match f.poll(unsafe { &FileRef::from_ptr(file) }, unsafe {
+        &PollTable::from_ptr(wait)
+    }) {
         Ok(v) => v,
         Err(_) => bindings::POLLERR,
     }

diff --git a/rust/kernel/iov_iter.rs b/rust/kernel/iov_iter.rs
@@ -70,7 +70,7 @@ impl IoBufferWriter for IovIter {
     }
 
     unsafe fn write_raw(&mut self, data: *const u8, len: usize) -> Result {
-        let res = rust_helper_copy_to_iter(data as _, len, self.ptr);
+        let res = unsafe { rust_helper_copy_to_iter(data as _, len, self.ptr) };
         if res != len {
             Err(Error::EFAULT)
         } else {
@@ -85,7 +85,7 @@ impl IoBufferReader for IovIter {
     }
 
     unsafe fn read_raw(&mut self, out: *mut u8, len: usize) -> Result {
-        let res = rust_helper_copy_from_iter(out as _, len, self.ptr);
+        let res = unsafe { rust_helper_copy_from_iter(out as _, len, self.ptr) };
         if res != len {
             Err(Error::EFAULT)
         } else {

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
@@ -210,7 +210,7 @@ macro_rules! offset_of {
 macro_rules! container_of {
     ($ptr:expr, $type:ty, $($f:tt)*) => {{
         let offset = $crate::offset_of!($type, $($f)*);
-        ($ptr as *const _ as *const u8).offset(-offset) as *const $type
+        unsafe { ($ptr as *const _ as *const u8).offset(-offset) as *const $type }
     }}
 }
 

diff --git a/rust/kernel/linked_list.rs b/rust/kernel/linked_list.rs
@@ -33,7 +33,7 @@ impl<T: ?Sized> Wrapper<T> for Box<T> {
     }
 
     unsafe fn from_pointer(ptr: NonNull<T>) -> Self {
-        Box::from_raw(ptr.as_ptr())
+        unsafe { Box::from_raw(ptr.as_ptr()) }
     }
 
     fn as_ref(&self) -> &T {
@@ -47,7 +47,7 @@ impl<T: ?Sized> Wrapper<T> for Arc<T> {
     }
 
     unsafe fn from_pointer(ptr: NonNull<T>) -> Self {
-        Arc::from_raw(ptr.as_ptr())
+        unsafe { Arc::from_raw(ptr.as_ptr()) }
     }
 
     fn as_ref(&self) -> &T {
@@ -61,7 +61,7 @@ impl<T: ?Sized> Wrapper<T> for &T {
     }
 
     unsafe fn from_pointer(ptr: NonNull<T>) -> Self {
-        &*ptr.as_ptr()
+        unsafe { &*ptr.as_ptr() }
     }
 
     fn as_ref(&self) -> &T {
@@ -149,10 +149,10 @@ impl<G: GetLinksWrapped> List<G> {
     /// Callers must ensure that `existing` points to a valid entry that is on the list.
     pub unsafe fn insert_after(&mut self, existing: NonNull<G::EntryType>, data: G::Wrapped) {
         let ptr = data.into_pointer();
-        let entry = &*existing.as_ptr();
-        if !self.list.insert_after(entry, ptr.as_ref()) {
+        let entry = unsafe { &*existing.as_ptr() };
+        if unsafe { !self.list.insert_after(entry, ptr.as_ref()) } {
             // If insertion failed, rebuild object so that it can be freed.
-            G::Wrapped::from_pointer(ptr);
+            unsafe { G::Wrapped::from_pointer(ptr) };
         }
     }
 
@@ -164,8 +164,8 @@ impl<G: GetLinksWrapped> List<G> {
     /// list leads to memory unsafety.
     pub unsafe fn remove(&mut self, data: &G::Wrapped) -> Option<G::Wrapped> {
         let entry_ref = Wrapper::as_ref(data);
-        if self.list.remove(entry_ref) {
-            Some(G::Wrapped::from_pointer(NonNull::from(entry_ref)))
+        if unsafe { self.list.remove(entry_ref) } {
+            Some(unsafe { G::Wrapped::from_pointer(NonNull::from(entry_ref)) })
         } else {
             None
         }

diff --git a/rust/kernel/miscdev.rs b/rust/kernel/miscdev.rs
@@ -85,8 +85,11 @@ impl<T: Sync> FileOpenAdapter for Registration<T> {
     type Arg = T;
 
     unsafe fn convert(_inode: *mut bindings::inode, file: *mut bindings::file) -> *const Self::Arg {
+        // TODO: `SAFETY` comment required here even if `unsafe` is not present,
+        // because `container_of!` hides it. Ideally we would not allow
+        // `unsafe` code as parameters to macros.
         let reg = crate::container_of!((*file).private_data, Self, mdev);
-        &(*reg).context
+        unsafe { &(*reg).context }
     }
 }
 

diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
@@ -71,12 +71,12 @@ pub trait ModuleParam: core::fmt::Display + core::marker::Sized {
         let arg = if val.is_null() {
             None
         } else {
-            Some(CStr::from_char_ptr(val).as_bytes())
+            Some(unsafe { CStr::from_char_ptr(val).as_bytes() })
         };
         match Self::try_from_param_arg(arg) {
             Some(new_value) => {
-                let old_value = (*param).__bindgen_anon_1.arg as *mut Self;
-                let _ = core::ptr::replace(old_value, new_value);
+                let old_value = unsafe { (*param).__bindgen_anon_1.arg as *mut Self };
+                let _ = unsafe { core::ptr::replace(old_value, new_value) };
                 0
             }
             None => crate::error::Error::EINVAL.to_kernel_errno(),
@@ -95,9 +95,9 @@ 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 = core::slice::from_raw_parts_mut(buf as *mut u8, crate::PAGE_SIZE);
+        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 write!(buf, "{}\0", *((*param).__bindgen_anon_1.arg as *mut Self)) {
+        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,
         }
@@ -111,7 +111,7 @@ pub trait ModuleParam: core::fmt::Display + core::marker::Sized {
     ///
     /// The `arg` field of `param` must be an instance of `Self`.
     unsafe extern "C" fn free(arg: *mut crate::c_types::c_void) {
-        core::ptr::drop_in_place(arg as *mut Self);
+        unsafe { core::ptr::drop_in_place(arg as *mut Self) };
     }
 }
 

diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
@@ -69,6 +69,6 @@ impl PointerWrapper for OfMatchTable {
     }
 
     unsafe fn from_pointer(p: *const c_types::c_void) -> Self {
-        Self(InnerTable::from_pointer(p))
+        Self(unsafe { InnerTable::from_pointer(p) })
     }
 }
diff --git a/rust/kernel/pages.rs b/rust/kernel/pages.rs
@@ -107,7 +107,7 @@ impl<const ORDER: u32> Pages<ORDER> {
         }
 
         let mapping = self.kmap(0).ok_or(Error::EINVAL)?;
-        ptr::copy((mapping.ptr as *mut u8).add(offset), dest, len);
+        unsafe { ptr::copy((mapping.ptr as *mut u8).add(offset), dest, len) };
         Ok(())
     }
 
@@ -127,7 +127,7 @@ impl<const ORDER: u32> Pages<ORDER> {
         }
 
         let mapping = self.kmap(0).ok_or(Error::EINVAL)?;
-        ptr::copy(src, (mapping.ptr as *mut u8).add(offset), len);
+        unsafe { ptr::copy(src, (mapping.ptr as *mut u8).add(offset), len) };
         Ok(())
     }