[PATCH v1 4/4] rust: hrtimer: Make HrTimer repr(transparent)
FUJITA Tomonori <[email protected]>
| Newsgroups | org.kernel.vger.rust-for-linux |
|---|---|
| Message-ID | <[email protected]> |
From: FUJITA Tomonori <[email protected]> HrTimerCallbackContext acquires a &HrTimer<T> from a NonNull<HrTimer<T>> while a &mut HrTimer<T> can exist at the same time. This is sound only because HrTimer's sole field is Opaque<bindings::hrtimer>, which puts every byte behind an UnsafeCell. Adding a field to HrTimer that is not Opaque would make acquiring that shared reference unsound. Make HrTimer repr(transparent), which prevents multiple fields, so that such a refactor fails to compile instead of silently introducing unsoundness. This does not guarantee the remaining field stays behind Opaque, but it rules out the likely way of getting there. repr(transparent) cannot be combined with repr(C), so drop the latter. Suggested-by: Miguel Ojeda <[email protected]> Reviewed-by: Andreas Hindborg <[email protected]> Signed-off-by: FUJITA Tomonori <[email protected]> --- rust/kernel/time/hrtimer.rs | 6 +++++- rust/kernel/time/hrtimer/arc.rs | 2 +- rust/kernel/time/hrtimer/pin.rs | 2 +- rust/kernel/time/hrtimer/pin_mut.rs | 2 +- rust/kernel/time/hrtimer/tbox.rs | 2 +- 5 files changed, 9 insertions(+), 5 deletions(-) diff --git a/rust/kernel/time/hrtimer.rs b/rust/kernel/time/hrtimer.rs index 59e9559e7099..2130dd24cccb 100644 --- a/rust/kernel/time/hrtimer.rs +++ b/rust/kernel/time/hrtimer.rs @@ -415,8 +415,12 @@ /// # Invariants /// /// * `self.timer` is initialized by `bindings::hrtimer_setup`. +// `repr(transparent)` is not merely about layout. `HrTimerCallbackContext` acquires a +// `&HrTimer<T>` while a `&mut HrTimer<T>` may exist, which is sound only because every byte of +// this type sits inside `Opaque`. Being transparent rejects a second field at compile time, +// but it does not enforce that the remaining field stays `Opaque`. #[pin_data] -#[repr(C)] +#[repr(transparent)] pub struct HrTimer<T> { #[pin] timer: Opaque<bindings::hrtimer>, diff --git a/rust/kernel/time/hrtimer/arc.rs b/rust/kernel/time/hrtimer/arc.rs index 2134d12d558c..ce7cff7efe29 100644 --- a/rust/kernel/time/hrtimer/arc.rs +++ b/rust/kernel/time/hrtimer/arc.rs @@ -143,7 +143,7 @@ impl<T> RawHrTimerCallback for HrTimerArc<T> type CallbackTarget<'a> = ArcBorrow<'a, T>; unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart { - // `HrTimer` is `repr(C)` + // `HrTimer` is `repr(transparent)` let timer_ptr = ptr.cast::<super::HrTimer<T>>(); // SAFETY: By C API contract `ptr` is the pointer we passed when diff --git a/rust/kernel/time/hrtimer/pin.rs b/rust/kernel/time/hrtimer/pin.rs index f44ac07cb722..6a0ac4d7dedf 100644 --- a/rust/kernel/time/hrtimer/pin.rs +++ b/rust/kernel/time/hrtimer/pin.rs @@ -128,7 +128,7 @@ impl<'a, T> RawHrTimerCallback for HrTimerPin<'a, T> type CallbackTarget<'b> = Pin<&'a T>; unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart { - // `HrTimer` is `repr(C)` + // `HrTimer` is `repr(transparent)` let timer_ptr = ptr.cast::<HrTimer<T>>(); // SAFETY: By the safety requirement of this function, `timer_ptr` diff --git a/rust/kernel/time/hrtimer/pin_mut.rs b/rust/kernel/time/hrtimer/pin_mut.rs index 9d9447d4d57e..65172c9e55e9 100644 --- a/rust/kernel/time/hrtimer/pin_mut.rs +++ b/rust/kernel/time/hrtimer/pin_mut.rs @@ -86,7 +86,7 @@ impl<'a, T> RawHrTimerCallback for Pin<&'a mut T> type CallbackTarget<'b> = Self; unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart { - // `HrTimer` is `repr(C)` + // `HrTimer` is `repr(transparent)` let timer_ptr = ptr.cast::<HrTimer<T>>(); // SAFETY: By the safety requirement of this function, `timer_ptr` diff --git a/rust/kernel/time/hrtimer/tbox.rs b/rust/kernel/time/hrtimer/tbox.rs index aa1ee31a7195..1dd68fcf2bd6 100644 --- a/rust/kernel/time/hrtimer/tbox.rs +++ b/rust/kernel/time/hrtimer/tbox.rs @@ -103,7 +103,7 @@ impl<T, A> RawHrTimerCallback for Pin<Box<T, A>> type CallbackTarget<'a> = Pin<&'a mut T>; unsafe extern "C" fn run(ptr: *mut bindings::hrtimer) -> bindings::hrtimer_restart { - // `HrTimer` is `repr(C)` + // `HrTimer` is `repr(transparent)` let timer_ptr = ptr.cast::<super::HrTimer<T>>(); // SAFETY: By C API contract `ptr` is the pointer we passed when -- 2.43.0