linux-kernel - Re: [PATCH] kbuild: Disable KCSAN for autogenerated *.mod.c intermediaries

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]

Date: Wed, 27 Mar 2024 08:31:15 +0200
From: Nikolay Borisov <nik.borisov@...e.com>
To: Borislav Petkov <bp@...en8.de>, Masahiro Yamada <masahiroy@...nel.org>,
 Nathan Chancellor <nathan@...nel.org>, Nicolas Schier <nicolas@...sle.eu>,
 linux-kbuild@...r.kernel.org
Cc: Marco Elver <elver@...gle.com>, Josh Poimboeuf <jpoimboe@...nel.org>,
 Paul Menzel <pmenzel@...gen.mpg.de>, Thomas Gleixner <tglx@...utronix.de>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
 LKML <linux-kernel@...r.kernel.org>, kasan-dev@...glegroups.com,
 David Kaplan <David.Kaplan@....com>
Subject: Re: [PATCH] kbuild: Disable KCSAN for autogenerated *.mod.c
 intermediaries



On 26.03.24 г. 22:25 ч., Borislav Petkov wrote:
> On Tue, Mar 26, 2024 at 08:33:31PM +0100, Marco Elver wrote:
>> I think just removing instrumentation from the mod.c files is very reasonable.
> 
> Thanks!
> 
> @Masahiro: pls send this to Linus now as the commit which adds the
> warning is in 6.9 so we should make sure we release it with all issues
> fixed.
> 
> Thx.
> 
> ---
> From: "Borislav Petkov (AMD)" <bp@...en8.de>
> Date: Tue, 26 Mar 2024 21:11:01 +0100
> 
> When KCSAN and CONSTRUCTORS are enabled, one can trigger the
> 
>    "Unpatched return thunk in use. This should not happen!"
> 
> catch-all warning.
> 
> Usually, when objtool runs on the .o objects, it does generate a section
> .return_sites which contains all offsets in the objects to the return
> thunks of the functions present there. Those return thunks then get
> patched at runtime by the alternatives.
> 
> KCSAN and CONSTRUCTORS add this to the the object file's .text.startup
> section:
> 
>    -------------------
>    Disassembly of section .text.startup:
> 
>    ...
> 
>    0000000000000010 <_sub_I_00099_0>:
>      10:   f3 0f 1e fa             endbr64
>      14:   e8 00 00 00 00          call   19 <_sub_I_00099_0+0x9>
>                            15: R_X86_64_PLT32      __tsan_init-0x4
>      19:   e9 00 00 00 00          jmp    1e <__UNIQUE_ID___addressable_cryptd_alloc_aead349+0x6>
>                            1a: R_X86_64_PLT32      __x86_return_thunk-0x4
>    -------------------
> 
> which, if it is built as a module goes through the intermediary stage of
> creating a <module>.mod.c file which, when translated, receives a second
> constructor:
> 
>    -------------------
>    Disassembly of section .text.startup:
> 
>    0000000000000010 <_sub_I_00099_0>:
>      10:   f3 0f 1e fa             endbr64
>      14:   e8 00 00 00 00          call   19 <_sub_I_00099_0+0x9>
>                            15: R_X86_64_PLT32      __tsan_init-0x4
>      19:   e9 00 00 00 00          jmp    1e <_sub_I_00099_0+0xe>
>                            1a: R_X86_64_PLT32      __x86_return_thunk-0x4
> 
>    ...
> 
>    0000000000000030 <_sub_I_00099_0>:
>      30:   f3 0f 1e fa             endbr64
>      34:   e8 00 00 00 00          call   39 <_sub_I_00099_0+0x9>
>                            35: R_X86_64_PLT32      __tsan_init-0x4
>      39:   e9 00 00 00 00          jmp    3e <__ksymtab_cryptd_alloc_ahash+0x2>
>                            3a: R_X86_64_PLT32      __x86_return_thunk-0x4
>    -------------------
> 
> in the .ko file.
> 
> Objtool has run already so that second constructor's return thunk cannot
> be added to the .return_sites section and thus the return thunk remains
> unpatched and the warning rightfully fires.
> 
> Drop KCSAN flags from the mod.c generation stage as those constructors
> do not contain data races one would be interested about.
> 
> Debugged together with David Kaplan <David.Kaplan@....com> and Nikolay
> Borisov <nik.borisov@...e.com>.
> 
> Reported-by: Paul Menzel <pmenzel@...gen.mpg.de>
> Signed-off-by: Borislav Petkov (AMD) <bp@...en8.de>
> Link: https://lore.kernel.org/r/0851a207-7143-417e-be31-8bf2b3afb57d@molgen.mpg.de
> ---
>   scripts/Makefile.modfinal | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index 8568d256d6fb..79fcf2731686 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -23,7 +23,7 @@ modname = $(notdir $(@:.mod.o=))
>   part-of-module = y
>   
>   quiet_cmd_cc_o_c = CC [M]  $@
> -      cmd_cc_o_c = $(CC) $(filter-out $(CC_FLAGS_CFI) $(CFLAGS_GCOV), $(c_flags)) -c -o $@ $<
> +      cmd_cc_o_c = $(CC) $(filter-out $(CC_FLAGS_CFI) $(CFLAGS_GCOV) $(CFLAGS_KCSAN), $(c_flags)) -c -o $@ $<
>   
>   %.mod.o: %.mod.c FORCE
>   	$(call if_changed_dep,cc_o_c)


Reviewed-by: Nikolay Borisov <nik.borisov@...e.com>