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] [day] [month] [year] [list]

Message-ID: <54dabf8bd3374543b2c72631db4f9c11@AcuMS.aculab.com>
Date:   Sat, 14 Jan 2023 15:35:55 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Borislav Petkov' <bp@...en8.de>,
        Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
CC:     kernel test robot <lkp@...el.com>,
        "llvm@...ts.linux.dev" <llvm@...ts.linux.dev>,
        "oe-kbuild-all@...ts.linux.dev" <oe-kbuild-all@...ts.linux.dev>,
        "rust-for-linux@...r.kernel.org" <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

From: Borislav Petkov
> Sent: 12 January 2023 16:15
...
> > 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.

While there would be no padding between any of the stricture members
that is largely irrelevant.
The __packed removes the two pad bytes from the end of the structure
and also tells the compiler that the structure itself may not be
aligned in memory.

So the bitfields can be misaligned in memory.
Quite why they are bitfields is anybodies guess, u16 would suffice.
Using u16 would also generate a known memory layout (bitfields are
best part of compiler defined - certainly not endianness defined.)

If the intent of the __packed is to save 2 bytes on every copy
of the structure then __packed __aligned(2) will have the same
effect while allowing the compiler to use 16-bit accesses.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)