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
| ||
|
Message-ID: <CAK8P3a1x0dU=x=mnBC8JeDG=dsQNfyO7X=16jm0WUwQ8wwLp=w@mail.gmail.com> Date: Tue, 2 Nov 2021 09:47:28 +0100 From: Arnd Bergmann <arnd@...db.de> To: Geert Uytterhoeven <geert@...ux-m68k.org> Cc: Taras Chornyi <tchornyi@...vell.com>, "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Volodymyr Mytnyk <vmytnyk@...vell.com>, Yevhen Orlov <yevhen.orlov@...ision.eu>, Vadym Kochan <vkochan@...vell.com>, Arnd Bergmann <arnd@...db.de>, Networking <netdev@...r.kernel.org>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org> Subject: Re: [PATCH] [-next] net: marvell: prestera: Add explicit padding On Tue, Nov 2, 2021 at 9:24 AM Geert Uytterhoeven <geert@...ux-m68k.org> wrote: > > On m68k: > > In function ‘prestera_hw_build_tests’, > inlined from ‘prestera_hw_switch_init’ at drivers/net/ethernet/marvell/prestera/prestera_hw.c:788:2: > ././include/linux/compiler_types.h:335:38: error: call to ‘__compiletime_assert_345’ declared with attribute error: BUILD_BUG_ON failed: sizeof(struct prestera_msg_switch_attr_req) != 16 > ... > > The driver assumes structure members are naturally aligned, but does not > add explicit padding, thus breaking architectures where integral values > are not always naturally aligned (e.g. on m68k, __alignof(int) is 2, not > 4). > > Fixes: bb5dbf2cc64d5cfa ("net: marvell: prestera: add firmware v4.0 support") > Signed-off-by: Geert Uytterhoeven <geert@...ux-m68k.org> Looks good to me, Reviewed-by: Arnd Bergmann <arnd@...db.de> > Compile-tested only. > > BTW, I sincerely doubt the use of __packed on structs like: > > union prestera_msg_switch_param { > u8 mac[ETH_ALEN]; > __le32 ageing_timeout_ms; > } __packed; > > This struct is only used as a member in another struct, where it is > be naturally aligned anyway. Agreed, this __packed attribute is clearly bogus and should be removed. Same for +struct prestera_msg_event_port_param { + union { + struct { + u8 oper; + __le32 mode; + __le32 speed; + u8 duplex; + u8 fc; + u8 fec; + } __packed mac; + struct { + u8 mdix; + __le64 lmode_bmap; + u8 fc; + } __packed phy; + } __packed; +} __packed __aligned(4); This makes no sense at all. I would suggest marking only the individual fields that are misaligned as __packed, but not the structure itself. and then there is this + union { + struct { + u8 admin:1; + u8 fc; + u8 ap_enable; + union { + struct { + __le32 mode; + u8 inband:1; + __le32 speed; + u8 duplex; + u8 fec; + u8 fec_supp; + } __packed reg_mode; + struct { + __le32 mode; + __le32 speed; + u8 fec; + u8 fec_supp; + } __packed ap_modes[PRESTERA_AP_PORT_MAX]; + } __packed; + } __packed mac; + struct { + u8 admin:1; + u8 adv_enable; + __le64 modes; + __le32 mode; + u8 mdix; + } __packed phy; + } __packed link; which puts misaligned bit fields in the middle of a packed structure! Arnd
Powered by blists - more mailing lists