linux-kernel - Re: [bp:tip-x86-alternatives 1/1] error[E0588]: packed type cannot transitively contain a `#[repr(align)]` type

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: <Y8Ax/I5qOcVDZljG@zn.tnic>
Date:   Thu, 12 Jan 2023 17:14:52 +0100
From:   Borislav Petkov <bp@...en8.de>
To:     Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
Cc:     kernel test robot <lkp@...el.com>, llvm@...ts.linux.dev,
        oe-kbuild-all@...ts.linux.dev, rust-for-linux@...r.kernel.org,
        lkml <linux-kernel@...r.kernel.org>,
        Yujie Liu <yujie.liu@...el.com>
Subject: Re: [bp:tip-x86-alternatives 1/1] error[E0588]: packed type cannot
 transitively contain a `#[repr(align)]` type

On Sat, Jan 07, 2023 at 01:38:42AM +0100, Miguel Ojeda wrote:
> You are of course right that the instructions are not complete, I just
> meant to add a bit of context, i.e. that Rust got enabled due to the
> config, but as far as I understand, it shouldn't be getting enabled in
> the other ones for the moment.

Right, or at least the repro instructions should state it clear.

Btw, this is part of a long-running feedback process we're giving to the 0day
bot in order to make their reports as user friendly as possible.

> My point was that the script expects some variables set by `Makefile`,
> similar to `$CC` etc., so that output does not imply you have (or not)
> a suitable Rust toolchain installed (i.e. it will currently also fail
> if you have it installed).

Aha.

> Meanwhile (of course it is not the same as proper reproduction
> instructions since the LKP team may do something different), the
> documentation on how to set it up for a normal developer is at:
> https://www.kernel.org/doc/html/latest/rust/quick-start.html, in case
> it helps (if you are up for it... :)

Probably that link should be part of those reproduction instructions.

> > And while we're reporting bugs: the error message from the compiler itself could
> > use some "humanization" - I have zero clue what it is trying to tell me.
> 
> What would you want to see? We can ask the relevant Rust team to see
> if they can improve it.
> 
> In general, note that you can ask `rustc` to further explain an error
> giving it the code with `--explain`. The compiler suggests this
> itself, but sadly the robot cut it out :(

Well, I find having an --explain option too much. But there are perhaps reasons
for it.

One improvement could be, IMHO, they could turn on --explain automatically when
it results in a build error. So that you don't have to do it yourself.

What would be better, tho, is if there were no --explain option at all and the
warnings are as human readable as possible.

>     For more information about this error, try `rustc --explain E0588`
> 
> In this case, it gives:
> 
>     A type with `packed` representation hint has a field with `align`
>     representation hint.
> ...

so the struct is:

struct alt_instr {
        s32 instr_offset;       /* original instruction */
        s32 repl_offset;        /* offset to replacement instruction */

        union {
                struct {
                        u32 cpuid: 16;  /* CPUID bit set for replacement */
                        u32 flags: 16;  /* patching control flags */
                };
                u32 ft_flags;
        };

        u8  instrlen;           /* length of original instruction */
        u8  replacementlen;     /* length of new instruction */
} __packed;

and everything is naturally aligned.

So I'm guessing this is a rust bindings glue shortcoming or so...

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette