2023-06-23 00:34:35

by Sami Tolvanen

[permalink] [raw]
Subject: [PATCH 0/2] Fix CFI failures with GCOV_PROFILE_ALL

Hi folks,

The following two patches fix CFI failures with GCOV_PROFILE_ALL,
where the compiler injects indirectly called functions to object
files that otherwise contain no executable code, and are not
processed by objtool or don't have CFI enabled. This results in
missing or incorrect type hashes during boot and when modules are
loaded.

Sami Tolvanen (2):
kbuild: Fix CFI failures with GCOV
kbuild: Disable GCOV for *.mod.o

init/Makefile | 1 +
scripts/Makefile.modfinal | 2 +-
scripts/Makefile.vmlinux | 1 +
3 files changed, 3 insertions(+), 1 deletion(-)


base-commit: 007034977130b49b618a5206aad54f634d9f169c
--
2.41.0.162.gfafddb0af9-goog



2023-06-23 00:51:53

by Sami Tolvanen

[permalink] [raw]
Subject: [PATCH 1/2] kbuild: Fix CFI failures with GCOV

With GCOV_PROFILE_ALL, Clang injects __llvm_gcov_* functions to
each object file, and the functions are indirectly called during
boot. However, when code is injected to object files that are not
part of vmlinux.o, it's also not processed by objtool, which breaks
CFI hash randomization as the hashes in these files won't be
included in the .cfi_sites section and thus won't be randomized.

Similarly to commit 42633ed852de ("kbuild: Fix CFI hash
randomization with KASAN"), disable GCOV for .vmlinux.export.o and
init/version-timestamp.o to avoid emitting unnecessary functions to
object files that don't otherwise have executable code.

Fixes: 0c3e806ec0f9 ("x86/cfi: Add boot time hash randomization")
Reported-by: Joe Fradley <[email protected]>
Signed-off-by: Sami Tolvanen <[email protected]>
---
init/Makefile | 1 +
scripts/Makefile.vmlinux | 1 +
2 files changed, 2 insertions(+)

diff --git a/init/Makefile b/init/Makefile
index 26de459006c4..ec557ada3c12 100644
--- a/init/Makefile
+++ b/init/Makefile
@@ -60,3 +60,4 @@ include/generated/utsversion.h: FORCE
$(obj)/version-timestamp.o: include/generated/utsversion.h
CFLAGS_version-timestamp.o := -include include/generated/utsversion.h
KASAN_SANITIZE_version-timestamp.o := n
+GCOV_PROFILE_version-timestamp.o := n
diff --git a/scripts/Makefile.vmlinux b/scripts/Makefile.vmlinux
index 10176dec97ea..3cd6ca15f390 100644
--- a/scripts/Makefile.vmlinux
+++ b/scripts/Makefile.vmlinux
@@ -19,6 +19,7 @@ quiet_cmd_cc_o_c = CC $@

ifdef CONFIG_MODULES
KASAN_SANITIZE_.vmlinux.export.o := n
+GCOV_PROFILE_.vmlinux.export.o := n
targets += .vmlinux.export.o
vmlinux: .vmlinux.export.o
endif
--
2.41.0.162.gfafddb0af9-goog


2023-06-23 09:36:20

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 0/2] Fix CFI failures with GCOV_PROFILE_ALL

On Fri, Jun 23, 2023 at 12:11:41AM +0000, Sami Tolvanen wrote:
> Hi folks,
>
> The following two patches fix CFI failures with GCOV_PROFILE_ALL,
> where the compiler injects indirectly called functions to object
> files that otherwise contain no executable code, and are not
> processed by objtool or don't have CFI enabled. This results in
> missing or incorrect type hashes during boot and when modules are
> loaded.
>
> Sami Tolvanen (2):
> kbuild: Fix CFI failures with GCOV
> kbuild: Disable GCOV for *.mod.o
>
> init/Makefile | 1 +
> scripts/Makefile.modfinal | 2 +-
> scripts/Makefile.vmlinux | 1 +
> 3 files changed, 3 insertions(+), 1 deletion(-)
>

Urgh, tricky stuff this.

Acked-by: Peter Zijlstra (Intel) <[email protected]>

And yes, objtool essentially assumes vmlinux.o is complete and does LTO
like passes. Is there something kbuild can do to ensure noting else gets
linked in after this?

2023-06-23 17:05:29

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH 0/2] Fix CFI failures with GCOV_PROFILE_ALL

On Fri, Jun 23, 2023 at 12:11:41AM +0000, Sami Tolvanen wrote:
> Hi folks,
>
> The following two patches fix CFI failures with GCOV_PROFILE_ALL,
> where the compiler injects indirectly called functions to object
> files that otherwise contain no executable code, and are not
> processed by objtool or don't have CFI enabled. This results in
> missing or incorrect type hashes during boot and when modules are
> loaded.
>
> Sami Tolvanen (2):
> kbuild: Fix CFI failures with GCOV
> kbuild: Disable GCOV for *.mod.o
>
> init/Makefile | 1 +
> scripts/Makefile.modfinal | 2 +-
> scripts/Makefile.vmlinux | 1 +
> 3 files changed, 3 insertions(+), 1 deletion(-)

Nice hunting!

Reviewed-by: Kees Cook <[email protected]>

Should these get Cc: stable tags maybe?

--
Kees Cook

2023-06-23 18:32:40

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH 0/2] Fix CFI failures with GCOV_PROFILE_ALL

On Thu, Jun 22, 2023 at 5:11 PM Sami Tolvanen <[email protected]> wrote:
>
> Hi folks,
>
> The following two patches fix CFI failures with GCOV_PROFILE_ALL,
> where the compiler injects indirectly called functions to object
> files that otherwise contain no executable code, and are not
> processed by objtool or don't have CFI enabled. This results in
> missing or incorrect type hashes during boot and when modules are
> loaded.
>
> Sami Tolvanen (2):
> kbuild: Fix CFI failures with GCOV
> kbuild: Disable GCOV for *.mod.o
>
> init/Makefile | 1 +
> scripts/Makefile.modfinal | 2 +-
> scripts/Makefile.vmlinux | 1 +
> 3 files changed, 3 insertions(+), 1 deletion(-)

Thanks for the patches!
Reviewed-by: Nick Desaulniers <[email protected]>

>
>
> base-commit: 007034977130b49b618a5206aad54f634d9f169c
> --
> 2.41.0.162.gfafddb0af9-goog
>
>


--
Thanks,
~Nick Desaulniers

2023-06-23 20:46:10

by Sami Tolvanen

[permalink] [raw]
Subject: Re: [PATCH 0/2] Fix CFI failures with GCOV_PROFILE_ALL

On Fri, Jun 23, 2023 at 9:32 AM Kees Cook <[email protected]> wrote:
>
> On Fri, Jun 23, 2023 at 12:11:41AM +0000, Sami Tolvanen wrote:
> > Hi folks,
> >
> > The following two patches fix CFI failures with GCOV_PROFILE_ALL,
> > where the compiler injects indirectly called functions to object
> > files that otherwise contain no executable code, and are not
> > processed by objtool or don't have CFI enabled. This results in
> > missing or incorrect type hashes during boot and when modules are
> > loaded.
> >
> > Sami Tolvanen (2):
> > kbuild: Fix CFI failures with GCOV
> > kbuild: Disable GCOV for *.mod.o
> >
> > init/Makefile | 1 +
> > scripts/Makefile.modfinal | 2 +-
> > scripts/Makefile.vmlinux | 1 +
> > 3 files changed, 3 insertions(+), 1 deletion(-)
>
> Nice hunting!
>
> Reviewed-by: Kees Cook <[email protected]>
>
> Should these get Cc: stable tags maybe?

I was under the impression that Fixes: tags would be sufficient these
days, but agreed, explicit Cc: probably wouldn't hurt.

Sami

2023-06-24 09:11:03

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH 0/2] Fix CFI failures with GCOV_PROFILE_ALL

On Sat, Jun 24, 2023 at 5:39 AM Sami Tolvanen <[email protected]> wrote:
>
> On Fri, Jun 23, 2023 at 9:32 AM Kees Cook <[email protected]> wrote:
> >
> > On Fri, Jun 23, 2023 at 12:11:41AM +0000, Sami Tolvanen wrote:
> > > Hi folks,
> > >
> > > The following two patches fix CFI failures with GCOV_PROFILE_ALL,
> > > where the compiler injects indirectly called functions to object
> > > files that otherwise contain no executable code, and are not
> > > processed by objtool or don't have CFI enabled. This results in
> > > missing or incorrect type hashes during boot and when modules are
> > > loaded.
> > >
> > > Sami Tolvanen (2):
> > > kbuild: Fix CFI failures with GCOV
> > > kbuild: Disable GCOV for *.mod.o
> > >
> > > init/Makefile | 1 +
> > > scripts/Makefile.modfinal | 2 +-
> > > scripts/Makefile.vmlinux | 1 +
> > > 3 files changed, 3 insertions(+), 1 deletion(-)
> >
> > Nice hunting!
> >
> > Reviewed-by: Kees Cook <[email protected]>
> >
> > Should these get Cc: stable tags maybe?
>
> I was under the impression that Fixes: tags would be sufficient these
> days, but agreed, explicit Cc: probably wouldn't hurt.
>
> Sami


Both applied to linux-kbuild.

I also think Fixes: tags would be enough
to make them back-ported.


--
Best Regards
Masahiro Yamada