linux-kernel - Re: [PATCH 1/1] rust: bindgen: Add `alt

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: <37578649-c696-f3b5-a216-196e210929e5@gmail.com>
Date:   Thu, 2 Mar 2023 11:59:00 -0300
From:   Martin Rodriguez Reboredo <yakoyoku@...il.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Miguel Ojeda <ojeda@...nel.org>
Cc:     Alex Gaynor <alex.gaynor@...il.com>,
        Wedson Almeida Filho <wedsonaf@...il.com>,
        Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
        Björn Roy Baron <bjorn3_gh@...tonmail.com>,
        Derek Barbosa <debarbos@...hat.com>,
        rust-for-linux@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] rust: bindgen: Add `alt_instr` as opaque type

On 3/2/23 11:16, Arnaldo Carvalho de Melo wrote:
> To address this build error:
> 
>     BINDGEN rust/bindings/bindings_generated.rs
>     BINDGEN rust/bindings/bindings_helpers_generated.rs
>     EXPORTS rust/exports_core_generated.h
>     RUSTC P rust/libmacros.so
>     RUSTC L rust/compiler_builtins.o
>     RUSTC L rust/alloc.o
>     RUSTC L rust/bindings.o
>     RUSTC L rust/build_error.o
>     EXPORTS rust/exports_alloc_generated.h
>   error[E0588]: packed type cannot transitively contain a `#[repr(align)]` type
>        --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10094:1
>         |
>   10094 | / pub struct alt_instr {
>   10095 | |     pub instr_offset: s32,
>   10096 | |     pub repl_offset: s32,
>   10097 | |     pub __bindgen_anon_1: alt_instr__bindgen_ty_1,
>   10098 | |     pub instrlen: u8_,
>   10099 | |     pub replacementlen: u8_,
>   10100 | | }
>         | |_^
>         |
>   note: `alt_instr__bindgen_ty_1__bindgen_ty_1` has a `#[repr(align)]` attribute
>        --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10111:1
>         |
>   10111 | / pub struct alt_instr__bindgen_ty_1__bindgen_ty_1 {
>   10112 | |     pub _bitfield_1: __BindgenBitfieldUnit<[u8; 4usize], u16>,
>   10113 | | }
>         | |_^
>   note: `alt_instr` contains a field of type `alt_instr__bindgen_ty_1`
>        --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10097:9
>         |
>   10097 |     pub __bindgen_anon_1: alt_instr__bindgen_ty_1,
>         |         ^^^^^^^^^^^^^^^^
>   note: ...which contains a field of type `alt_instr__bindgen_ty_1__bindgen_ty_1`
>        --> /var/home/acme/git/linux/rust/bindings/bindings_generated.rs:10104:9
>         |
>   10104 |     pub __bindgen_anon_1: alt_instr__bindgen_ty_1__bindgen_ty_1,
>         |         ^^^^^^^^^^^^^^^^
> 

Reading the kernel sources this field corresponds to an u16 which indeed
represents a bit set and it says so in a comment on the field. I
couldn't replicate this issue, though, because this struct is used only
inside arch pretty much internally, then there's no problem to make it
opaque. Still, we have to be careful if these kind of things appear in
the future.

And I notice that You haven't mentioned the version of Bindgen that
You've used, including its linked libclang too. Otherwise I think this
could be accepted.

Reviewed-by: Martin Rodriguez Reboredo <yakoyoku@...il.com>

>   error: aborting due to previous error
> 
>   For more information about this error, try `rustc --explain E0588`.
>   make[1]: *** [rust/Makefile:389: rust/bindings.o] Error 1
>   make: *** [Makefile:1293: prepare] Error 2
> 
> Cc: Derek Barbosa <debarbos@...hat.com>
> Cc: Miguel Ojeda <ojeda@...nel.org>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> ---
>  rust/bindgen_parameters | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/rust/bindgen_parameters b/rust/bindgen_parameters
> index be4963bf720304da..552d9a85925b9945 100644
> --- a/rust/bindgen_parameters
> +++ b/rust/bindgen_parameters
> @@ -6,6 +6,7 @@
>  --opaque-type local_apic
>  
>  # Packed type cannot transitively contain a `#[repr(align)]` type.
> +--opaque-type alt_instr
>  --opaque-type x86_msi_data
>  --opaque-type x86_msi_addr_lo
>