linux-kernel - Re: [linux-next:master 12681/13861] drivers/i2c/i2c-core-base.o: warning: objtool: __i2c_transfer+0x120: stack state mismatch: reg1[24]=-1+0 reg2[24]=-2-24

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: <5f830e7a-7cfa-3c62-ba3f-f93808e95821@loongson.cn>
Date: Tue, 8 Apr 2025 10:45:51 +0800
From: Tiezhu Yang <yangtiezhu@...ngson.cn>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: Philip Li <philip.li@...el.com>, kernel test robot <lkp@...el.com>,
 Guenter Roeck <linux@...ck-us.net>, oe-kbuild-all@...ts.linux.dev,
 Andrew Morton <akpm@...ux-foundation.org>,
 Linux Memory Management List <linux-mm@...ck.org>,
 Alessandro Carminati <acarmina@...hat.com>,
 Peter Zijlstra <peterz@...radead.org>, linux-kernel@...r.kernel.org,
 loongarch@...ts.linux.dev
Subject: Re: [linux-next:master 12681/13861] drivers/i2c/i2c-core-base.o:
 warning: objtool: __i2c_transfer+0x120: stack state mismatch: reg1[24]=-1+0
 reg2[24]=-2-24

On 04/08/2025 09:23 AM, Josh Poimboeuf wrote:
> On Mon, Apr 07, 2025 at 06:52:10PM +0800, Tiezhu Yang wrote:
>> There is a potential execution path with only using s0 and ra
>> (without using s1, s2, s3, etc): 2d58-->2d70-->2f88-->2e78-->2e84
>
> [...]
>
>> From this point of view, it seems that there is no problem for the
>> generated instructions of the current code, it is not a runtime bug,
>> just a GCC optimization.
>
> I don't see how this is responsive to my email.
>
> I described a code path which revealed a GCC bug, specifically with asm
> goto (unless I got something wrong).  Then you responded with a
> *completely different* code path.
>
> How does that prove my original code path isn't possible?
>
> To summarize, the path I found was
>
>   2d58 ... 2d9c -> 2da8 .. 2dc4 -> 2ebc .. 2ec0 (runtime patched static branch) -> 2e78 .. 2e84 (ret)

Sorry, you are right, I misunderstood.

>> (2) Analysis
>>
>> In fact, the generated objtool warning is because the break instruction
>> (2ee8) which is before the restoring s1 instruction (2eec) is annotated
>> as dead end.
>
> Actually, it's the opposite.  Objtool would normally consider BREAK to
> be a dead end.  But it's annotated as "reachable", aka "non dead end".
>
>> This issue is introduced by the following changes:
>>
>>  #define __WARN_FLAGS(flags)					\
>>  do {								\
>>  	instrumentation_begin();				\
>> -	__BUG_FLAGS(BUGFLAG_WARNING|(flags), ANNOTATE_REACHABLE(10001b));\
>> +	if (!KUNIT_IS_SUPPRESSED_WARNING(__func__))			\
>> +		__BUG_FLAGS(BUGFLAG_WARNING|(flags), ANNOTATE_REACHABLE(10001b));\
>>  	instrumentation_end();					\
>>  } while (0)
>>
>> of commit e61a8b4b0d83 ("loongarch: add support for suppressing warning
>> backtraces") in the linux-next.git.
>
> Putting that annotation behind a conditional should not break anything.

OK, got it.

>> (4) Solution 1
>> One way is to annotate __BUG_ENTRY() as reachable whether
>> KUNIT_IS_SUPPRESSED_WARNING() is true or false, like this:
>>
>> ---8<---
>> diff --git a/arch/loongarch/include/asm/bug.h
>> b/arch/loongarch/include/asm/bug.h
>> index b79ff6696ce6..e41ebeaba204 100644
>> --- a/arch/loongarch/include/asm/bug.h
>> +++ b/arch/loongarch/include/asm/bug.h
>> @@ -60,8 +60,9 @@
>>  #define __WARN_FLAGS(flags)                                    \
>>  do {                                                           \
>>         instrumentation_begin();                                \
>> -       if (!KUNIT_IS_SUPPRESSED_WARNING(__func__))                     \
>> -               __BUG_FLAGS(BUGFLAG_WARNING|(flags),
>> ANNOTATE_REACHABLE(10001b));\
>> +       if (!KUNIT_IS_SUPPRESSED_WARNING(__func__))             \
>> +               __BUG_FLAGS(BUGFLAG_WARNING|(flags), "");       \
>> +       __BUG_FLAGS(0, ANNOTATE_REACHABLE(10001b));             \
>>         instrumentation_end();                                  \
>>  } while (0)
>
> Huh?  That's basically:
>
> 	if (!suppress_warning)
> 		WARN();
> 	BUG();
>
> So it upgrades a conditional WARN to an unconditional BUG???
>
> Not to mention the reachable annotations are backwards: the WARN() is
> annotated as dead end while the BUG() is annotated reachable.
>
> Even if that silences objtool somehow, it will most definitely have the
> wrong runtime behavior.

Yes, my original analysis seems wrong.

>> (5) Solution 2
>> The other way is to use "-fno-shrink-wrap" to aovid such issue under
>> CONFIG_OBJTOOL at compile-time, like this:
>
> As far as I can tell, that would be a workaround to get objtool to stop
> warning about a legitimate compiler bug.

So this is a run-time bug rather than a compile-time warning, it should
put the option "-fno-shrink-wrap" outside CONFIG_OBJTOOL in
arch/loongarch/Makefile as a workaround, like this:

diff --git a/arch/loongarch/Makefile b/arch/loongarch/Makefile
index 0304eabbe606..98241e3015fb 100644
--- a/arch/loongarch/Makefile
+++ b/arch/loongarch/Makefile
@@ -108,6 +108,8 @@ KBUILD_CFLAGS                       += 
-fno-jump-tables # keep compatibility with older compilers
  endif
  endif

+KBUILD_CFLAGS                  += -fno-shrink-wrap
+
  KBUILD_RUSTFLAGS               += 
--target=loongarch64-unknown-none-softfloat -Ccode-model=small
  KBUILD_RUSTFLAGS_KERNEL                += 
-Zdirect-access-external-data=yes
  KBUILD_RUSTFLAGS_MODULE                += -Zdirect-access-external-data=no

Thanks,
Tiezhu