scripts/Makefile.modfinal | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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@alien8.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@amd.com> and Nikolay
Borisov <nik.borisov@suse.com>.
Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.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)
--
2.43.0
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquetteOn Wed, Mar 27, 2024 at 5:26 AM Borislav Petkov <bp@alien8.de> 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@alien8.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@amd.com> and Nikolay > Borisov <nik.borisov@suse.com>. > > Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> > Signed-off-by: Borislav Petkov (AMD) <bp@alien8.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) > -- > 2.43.0 > > > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette I applied. I fixed the typo "the the" and replaced Link: with Closes: to address the following checkpatch warnings: WARNING: Possible repeated word: 'the' #18: KCSAN and CONSTRUCTORS add this to the the object file's .text.startup WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report #70: Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de> Instead of filter-out, you could add KCSAN_SANITIZE := n to scripts/Makefile.modfinal because it is the reason why KCSAN_SANITIZE exists. But, that is not a big deal. GCOV flag is also filtered away instead of GCOV_PROFILE := n I will probably use a different approach later. -- Best Regards Masahiro Yamada
On Sun, Mar 31, 2024 at 07:32:30AM +0900, Masahiro Yamada wrote:
> I applied.
>
> I fixed the typo "the the" and replaced Link: with Closes:
> to address the following checkpatch warnings:
Thanks!
> Instead of filter-out, you could add
> KCSAN_SANITIZE := n
> to scripts/Makefile.modfinal because
> it is the reason why KCSAN_SANITIZE exists.
>
> But, that is not a big deal.
> GCOV flag is also filtered away instead of
> GCOV_PROFILE := n
Ah, that would've been more readable, yap.
> I will probably use a different approach later.
Right.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
Dear Borislav, Thank you and the others very much for coming up with a patch so quickly. Tested-by: Paul Menzel <pmenzel@molgen.mpg.de> # Dell XPS 13 9360/0596KF, BIOS 2.21.0 06/02/2022 Kind regards, Paul PS: Without your patch, I also so it one in QEMU q35, but not consistently.
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@alien8.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@amd.com> and Nikolay > Borisov <nik.borisov@suse.com>. > > Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> > Signed-off-by: Borislav Petkov (AMD) <bp@alien8.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@suse.com>
On Tue, 26 Mar 2024 at 21:26, Borislav Petkov <bp@alien8.de> 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@alien8.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@amd.com> and Nikolay > Borisov <nik.borisov@suse.com>. > > Reported-by: Paul Menzel <pmenzel@molgen.mpg.de> > Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de> > Link: https://lore.kernel.org/r/0851a207-7143-417e-be31-8bf2b3afb57d@molgen.mpg.de Reviewed-by: Marco Elver <elver@google.com> Thanks! > --- > 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) > -- > 2.43.0 > > > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette
© 2016 - 2026 Red Hat, Inc.