linux-kernel - Re: [PATCH V2] f2fs: fix atgc bug on issue in 32bits platform

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

Date:   Fri, 11 Nov 2022 10:57:21 +0100
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Zhiguo Niu" <niuzhiguo84@...il.com>
Cc:     "kernel test robot" <lkp@...el.com>,
        "zhiguo.niu" <zhiguo.niu@...soc.com>, jaegeuk@...nel.org,
        chao@...nel.org, linux-f2fs-devel@...ts.sourceforge.net,
        linux-kernel@...r.kernel.org, llvm@...ts.linux.dev,
        oe-kbuild-all@...ts.linux.dev
Subject: Re: [PATCH V2] f2fs: fix atgc bug on issue in 32bits platform

On Fri, Nov 11, 2022, at 10:40, Zhiguo Niu wrote:
> Arnd Bergmann <arnd@...db.de> 于2022年11月10日周四 21:45写道：
>   I thinks the gcc complier build warning :
> ----------------------------------------------------------------
>     In file included from fs/f2fs/segment.c:24:
>>> fs/f2fs/gc.h:73:1: warning: alignment 1 of 'struct victim_entry' is 
>
>>> less than 8 [-Wpacked-not-aligned]
>
>       73 | } __packed;
>
>          | ^
>
>  ---------------------------------------------------------------
>
> It is because struct rb_node has the attribute 
> "__attribute__((aligned(sizeof(long)", it is 8 bytes in 64bits platform.
>
> struct rb_node {
> unsigned long  __rb_parent_color;
> struct rb_node *rb_right;
> struct rb_node *rb_left;
> } __attribute__((aligned(sizeof(long))));
>
> so I just try to put __packed on union of struct victim_entry and i 
> also keep consistent with struct rb_entry.

No, that attribute has no effect on any architecture other
than m68k, which defaults to 16-bit alignment for 32-bit
members. I'm fairly sure the alignment attribute on
rb_node is entirely unrelated to the problems you are
seeing in f2fs that come from having a structure with
stricter (4 byte or 8 byte) alignment requirements embedded
in a structure with relaxed (single-byte) alignment:

> struct rb_entry {
> struct rb_node rb_node; /* rb node located in rb-tree */
> union {
> struct {
> unsigned int ofs; /* start offset of the entry */
> unsigned int len; /* length of the entry */
> };
> unsigned long long key; /* 64-bits key */
> } __packed;
> };

This tells the compiler that the anonymous union is 
entirely unconstrained, but the anonymous struct inside
it has the default alignment, which is the contradition
that gcc correctly warns about.

Since the only thing you need here is to lower the
alignment constraint from 8 bytes to 4 bytes, the easiest
way is to have the __packed annotation on the 'key'
member. This avoids all warnings, as long you do not
take the address of the 'key' member and pass it to
a function that expects an aligned 'u64' pointer.

     Arnd