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: <f215591d-1d5f-b5ae-38fe-4f6407bb2116@posteo.de>
Date:   Tue, 23 Mar 2021 08:26:55 +0100
From:   Patrick Menschel <menschel.p@...teo.de>
To:     Rong Chen <rong.a.chen@...el.com>
Cc:     Vincent MAILHOL <mailhol.vincent@...adoo.fr>,
        Oliver Hartkopp <socketcan@...tkopp.net>,
        kernel test robot <lkp@...el.com>, kbuild-all@...ts.01.org,
        open list <linux-kernel@...r.kernel.org>,
        Marc Kleine-Budde <mkl@...gutronix.de>,
        linux-can <linux-can@...r.kernel.org>
Subject: Re: [kbuild-all] Re: include/linux/compiler_types.h:315:38: error:
 call to '__compiletime_assert_536' declared with attribute error:
 BUILD_BUG_ON failed: offsetof(struct can_frame, len) != offsetof(struct
 canfd_frame, len) || offsetof(struct can_frame, data) != offsetof(struc...


Am 23.03.21 um 07:06 schrieb Rong Chen:
>>>> Ugh! Why did the compiler extend the space for the union to 4 bytes?!?
>> Just a random idea but maybe the added padding is due to some
>> kind of odd intrication with the __attribute__((__aligned__(8)))
>> just below? Does this reproduce if we remove the
>> __attribute__((__aligned__(8)))?
> 
> Here is the layout without __attribute__((__aligned__(8))),
> the union is still extended to 4 bytes:
> 
> struct can_frame {
>         canid_t                    can_id;               /* 0     4 */
>         union {
>                 __u8               len;                  /* 4     1 */
>                 __u8               can_dlc;              /* 4     1 */
>         };                                               /* 4     4 */
>         __u8                       __pad;                /* 8     1 */
>         __u8                       __res0;               /* 9     1 */
>         __u8                       len8_dlc;             /* 10     1 */
>         __u8                       data[8];              /* 11     8 */
> 
>         /* size: 20, cachelines: 1, members: 6 */
>         /* padding: 1 */
>         /* last cacheline: 20 bytes */
> };
> 
> Best Regards,
> Rong Chen

Hi,

I would suggest a try with __attribute__((__aligned__(8))) only on
can_frame, not on data[8].

If the structure length is a multiple of 8, the compiler should
recognize this and keep the union a single byte in favor of an array
configuration of that struct.

The __attribute__((__aligned__(8))) on data[8] has strange propagation
effects upwards.

If the attributes are really necessary, I would suggest to have both

__attribute__((packed))
__attribute__((__aligned__(8)))

on structure level instead of inside, so no padding happens inside the
structure while the structure itself is aligned.
Using aligned and packaged inside a structure may be contradictive to
the compiler.

This reminds me of the alignment/gap issue with my python3
implementation of bcm message while alternating between X86_64 and
ARMHF. Using c_types was a mess but bytestrings worked in the end.
Be aware native alignment apparently is 4 on armhf linux and 8 on X86_64
linux.

https://marc.info/?l=linux-can&m=161251622904527&w=2
https://gitlab.com/Menschel/socketcan/-/commit/afc6744129448ae4333629fc0297808dd42e3530#e522710a8423075cfd1147ae6b7f44facac3ffb0_133_132

Best Regards,
Patrick Menschel

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ