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: <bzy7cad37tafrbcmsstn355fpljxxmi25ifc4piihp6ln3ztxh@zp3c7ydsjmuq>
Date: Mon, 7 Apr 2025 18:23:25 -0700
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Tiezhu Yang <yangtiezhu@...ngson.cn>
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 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)

> (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.

> (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.

> (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.

-- 
Josh