| Message ID | 20231120110607.98956-1-dmantipov@yandex.ru |
|---|---|
| State | New |
| Headers | show |
| Series | uapi: propagate __struct_group() attributes to the container union | expand |
On Mon, 2023-11-20 at 14:05 +0300, Dmitry Antipov wrote: > Recently the kernel test robot has reported an ARM-specific BUILD_BUG_ON() > in an old and unmaintained wil6210 wireless driver. > Heh. I guess it wasn't unmaintained enough for someone to touch it and add struct_group() to it... > include/uapi/linux/stddef.h | 2 +- > No idea what tree this should go through, but I guess wireless isn't appropriate. johannes
On Fri, Nov 24, 2023 at 06:37:14PM +0100, Johannes Berg wrote: > On Mon, 2023-11-20 at 14:05 +0300, Dmitry Antipov wrote: > > Recently the kernel test robot has reported an ARM-specific BUILD_BUG_ON() > > in an old and unmaintained wil6210 wireless driver. > > > > Heh. I guess it wasn't unmaintained enough for someone to touch it and > add struct_group() to it... It needed it since it was using memcpy across fields. But it's been a while since that change happened, so finding this bug now is pretty impressive. :) > > > include/uapi/linux/stddef.h | 2 +- > > > > No idea what tree this should go through, but I guess wireless isn't > appropriate. I'll take it; thanks for the find Dmitry!
On Mon, 20 Nov 2023 14:05:08 +0300, Dmitry Antipov wrote: > Recently the kernel test robot has reported an ARM-specific BUILD_BUG_ON() > in an old and unmaintained wil6210 wireless driver. The problem comes from > the structure packing rules of old ARM ABI ('-mabi=apcs-gnu'). For example, > the following structure is packed to 18 bytes instead of 16: > > struct poorly_packed { > unsigned int a; > unsigned int b; > unsigned short c; > union { > struct { > unsigned short d; > unsigned int e; > } __attribute__((packed)); > struct { > unsigned short d; > unsigned int e; > } __attribute__((packed)) inner; > }; > } __attribute__((packed)); > > [...] Applied to for-linus/hardening, thanks! [1/1] uapi: propagate __struct_group() attributes to the container union https://git.kernel.org/kees/c/4e86f32a13af Take care,
diff --git a/include/uapi/linux/stddef.h b/include/uapi/linux/stddef.h index 5c6c4269f7ef..2ec6f35cda32 100644 --- a/include/uapi/linux/stddef.h +++ b/include/uapi/linux/stddef.h @@ -27,7 +27,7 @@ union { \ struct { MEMBERS } ATTRS; \ struct TAG { MEMBERS } ATTRS NAME; \ - } + } ATTRS #ifdef __cplusplus /* sizeof(struct{}) is 1 in C++, not 0, can't use C version of the macro. */
Recently the kernel test robot has reported an ARM-specific BUILD_BUG_ON() in an old and unmaintained wil6210 wireless driver. The problem comes from the structure packing rules of old ARM ABI ('-mabi=apcs-gnu'). For example, the following structure is packed to 18 bytes instead of 16: struct poorly_packed { unsigned int a; unsigned int b; unsigned short c; union { struct { unsigned short d; unsigned int e; } __attribute__((packed)); struct { unsigned short d; unsigned int e; } __attribute__((packed)) inner; }; } __attribute__((packed)); To fit it into 16 bytes, it's required to add packed attribute to the container union as well: struct poorly_packed { unsigned int a; unsigned int b; unsigned short c; union { struct { unsigned short d; unsigned int e; } __attribute__((packed)); struct { unsigned short d; unsigned int e; } __attribute__((packed)) inner; } __attribute__((packed)); } __attribute__((packed)); Thanks to Andrew Pinski of GCC team for sorting the things out at https://gcc.gnu.org/pipermail/gcc/2023-November/242888.html. Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202311150821.cI4yciFE-lkp@intel.com Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> --- include/uapi/linux/stddef.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)