linux-kernel - Re: [PATCH v1 1/2] LoongArch: Define barrier_before

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: <53b0ed80-4bbe-490d-1e3b-f2dac1aaacc9@loongson.cn>
Date: Wed, 21 Aug 2024 18:02:41 +0800
From: Jinyang He <hejinyang@...ngson.cn>
To: Xi Ruoyao <xry111@...111.site>, Huacai Chen <chenhuacai@...nel.org>,
 Tiezhu Yang <yangtiezhu@...ngson.cn>
Cc: Arnd Bergmann <arnd@...db.de>, loongarch@...ts.linux.dev,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/2] LoongArch: Define barrier_before_unreachable() as
 empty

On 2024-08-21 15:52, Xi Ruoyao wrote:

> On Wed, 2024-08-21 at 15:37 +0800, Huacai Chen wrote:
>>> I am not sure whether the GCC bug has been fixed, I can not find the
>>> fixup in the link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=82365
>>> and in the GCC repo. So I am not sure whether it is time and proper
>>> to remove this workaround in the common header totally, just remove
>>> it in the arch specified header when compiling kernel with a newer
>>> GCC version (for example GCC 12.1 or higher on LoongArch) at least.
>> What's your opinion? From my point of view, this GCC bug hasn't been
>> fixed. So there may still be potential problems.
> I'm pretty sure it isn't fixed.  Using the test case from the bug
> report:
>
> struct i2c_board_info {
>          char type[20];
>          char pad[100];
> };
>
> #ifdef NONORETURN
> void fortify_panic();
> #else
> void fortify_panic() __attribute__((noreturn));
> #endif
>
>
> int f(int a)
> {
>    if (a)
>      fortify_panic();
> }
>
>
> void i2c_new_device(struct i2c_board_info *);
> int em28xx_dvb_init(int model, int a, int b, int c, int d)
> {
>          switch (model) {
>          case 1:{
>                          struct i2c_board_info info = {};
>                          f(a);
>                          i2c_new_device(&info);
>                          break;
>                  }
>          case 2:{
>                          struct i2c_board_info info = {};
>                          f(b);
>                          i2c_new_device(&info);
>                          break;
>                  }
>          case 3:{
>                          struct i2c_board_info info = { };
>                          f(c);
>                          i2c_new_device(&info);
>                          break;
>                  }
>          case 4:{
>                          struct i2c_board_info info = { };
>                          f(d);
>                          i2c_new_device(&info);
>                          break;
>                  }
>          }
>          return 0;
> }
>
> $ cc -v
> Using built-in specs.
> COLLECT_GCC=cc
> COLLECT_LTO_WRAPPER=/usr/libexec/gcc/loongarch64-unknown-linux-gnu/14.2.0/lto-wrapper
> Target: loongarch64-unknown-linux-gnu
> Configured with: ../configure --prefix=/usr LD=ld --enable-languages=c,c++ --enable-default-pie --enable-default-ssp --disable-multilib --with-build-config=bootstrap-lto --disable-fixincludes --with-system-zlib --enable-host-pie
> Thread model: posix
> Supported LTO compression algorithms: zlib zstd
> gcc version 14.2.0 (GCC)
> $ cc t.c -S -Wframe-larger-than=1 -DNONORETURN -O2
> t.c: In function 'em28xx_dvb_init':
> t.c:50:1: warning: the frame size of 144 bytes is larger than 1 bytes [-Wframe-larger-than=]
>     50 | }
>        | ^
> $ cc t.c -S -Wframe-larger-than=1 -O2
> t.c: In function 'em28xx_dvb_init':
> t.c:50:1: warning: the frame size of 512 bytes is larger than 1 bytes [-Wframe-larger-than=]
>     50 | }
>        | ^
>
> And I'm puzzled why "unreachable instruction" is not a problem on x86?
>
Hi, Ruoyao,

I looked the gcc:machine_kexec.c:rtl:jump2+:kexec_reboot, it seems
the gcc thought the block `asm volatile ("")` is same. So for -Os,
if 2+ these blocks appears, it create "b" to the same block.

I tried "-fno-crossjumping" to disable the jump2 pass, it works.
Could we change this block different to solve this? (e.g. by __COUNTER__).
But it seems break the original trick. :-(

Jinyang