linux-kernel - Re: [PATCH v1 2/7] rust: add offset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87pm6i9pqh.fsf@metaspace.dk>
Date:   Tue, 30 May 2023 10:40:03 +0200
From:   Andreas Hindborg <nmi@...aspace.dk>
To:     Gary Guo <gary@...yguo.net>
Cc:     Alice Ryhl <aliceryhl@...gle.com>, rust-for-linux@...r.kernel.org,
        Miguel Ojeda <ojeda@...nel.org>,
        Wedson Almeida Filho <wedsonaf@...il.com>,
        Tejun Heo <tj@...nel.org>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        Alex Gaynor <alex.gaynor@...il.com>,
        Boqun Feng <boqun.feng@...il.com>,
        Björn Roy Baron <bjorn3_gh@...tonmail.com>,
        Benno Lossin <benno.lossin@...ton.me>,
        linux-kernel@...r.kernel.org, patches@...ts.linux.dev
Subject: Re: [PATCH v1 2/7] rust: add offset_of! macro


Gary Guo <gary@...yguo.net> writes:

> On Wed, 17 May 2023 20:31:14 +0000
> Alice Ryhl <aliceryhl@...gle.com> wrote:
>
>> From: Wedson Almeida Filho <walmeida@...rosoft.com>
>> 
>> This macro is used to compute the offset of a field in a struct.
>> 
>> This commit enables two unstable features that are necessary for using
>> the macro in a constant. However, this is not a problem as the macro
>> will become available from the Rust standard library soon [1]. The
>> unstable features can be disabled again once that happens.
>> 
>> The macro in this patch does not support sub-fields. That is, you cannot
>> write `offset_of!(MyStruct, field.sub_field)` to get the offset of
>> `sub_field` with `field`'s type being a struct with a field called
>> `sub_field`. This is because `field` might be a `Box<SubStruct>`, which
>> means that you would be trying to compute the offset to something in an
>> entirely different allocation. There's no easy way to fix the current
>> macro to support subfields, but the version being added to the standard
>> library should support it, so the limitation is temporary and not a big
>> deal.
>> 
>> Link: https://github.com/rust-lang/rust/issues/106655 [1]
>> Signed-off-by: Wedson Almeida Filho <walmeida@...rosoft.com>
>> Co-developed-by: Alice Ryhl <aliceryhl@...gle.com>
>> Signed-off-by: Alice Ryhl <aliceryhl@...gle.com>
>> ---
>>  rust/kernel/lib.rs     | 35 +++++++++++++++++++++++++++++++++++
>>  scripts/Makefile.build |  2 +-
>>  2 files changed, 36 insertions(+), 1 deletion(-)
>> 
>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index c718524056a6..cdf9fe999328 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -14,6 +14,8 @@
>>  #![no_std]
>>  #![feature(allocator_api)]
>>  #![feature(coerce_unsized)]
>> +#![feature(const_ptr_offset_from)]
>> +#![feature(const_refs_to_cell)]
>>  #![feature(core_ffi_c)]
>>  #![feature(dispatch_from_dyn)]
>>  #![feature(explicit_generic_args_with_impl_trait)]
>> @@ -102,3 +104,36 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
>>      // instead of `!`. See <https://github.com/rust-lang/rust-bindgen/issues/2094>.
>>      loop {}
>>  }
>> +
>> +/// Calculates the offset of a field from the beginning of the struct it belongs to.
>> +///
>> +/// # Examples
>> +///
>> +/// ```
>> +/// #[repr(C)]
>> +/// struct Test {
>> +///     a: u64,
>> +///     b: u32,
>> +/// }
>> +///
>> +/// assert_eq!(kernel::offset_of!(Test, b), 8);
>> +/// ```
>> +#[macro_export]
>> +macro_rules! offset_of {
>> +    ($type:ty, $field:ident) => {{
>> +        let tmp = ::core::mem::MaybeUninit::<$type>::uninit();
>> +        let outer = tmp.as_ptr();
>> +        // To avoid warnings when nesting `unsafe` blocks.
>> +        #[allow(unused_unsafe)]
>> +        // SAFETY: The pointer is valid and aligned, just not initialised; `addr_of` ensures that
>> +        // we don't actually read from `outer` (which would be UB) nor create an intermediate
>> +        // reference.
>> +        let inner = unsafe { ::core::ptr::addr_of!((*outer).$field) } as *const u8;
>> +        // To avoid warnings when nesting `unsafe` blocks.
>> +        #[allow(unused_unsafe)]
>> +        // SAFETY: The two pointers are within the same allocation block.
>> +        unsafe {
>> +            inner.offset_from(outer as *const u8) as usize
>> +        }
>
> This has no protection against using `Deref`. The memoffset crate has a 
>
> let $type { $field: _, .. };
>
> line to ensure that the field is a direct member of type and deref is
> not happening.

Good catch 👍

>
>> +    }};
>> +}
>> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
>> index 9f94fc83f086..ec583d13dde2 100644
>> --- a/scripts/Makefile.build
>> +++ b/scripts/Makefile.build
>> @@ -277,7 +277,7 @@ $(obj)/%.lst: $(src)/%.c FORCE
>>  # Compile Rust sources (.rs)
>>  # ---------------------------------------------------------------------------
>>  
>> -rust_allowed_features := core_ffi_c,explicit_generic_args_with_impl_trait,new_uninit,pin_macro
>> +rust_allowed_features := const_ptr_offset_from,const_refs_to_cell,core_ffi_c,explicit_generic_args_with_impl_trait,new_uninit,pin_macro
>
> Side note: once we bump our compiler to 1.71, we should switch to the
> built-in `offset_of!` macro and get rid of these unstable features.
>
> Best,
> Gary