[PATCH 0/2] Fix CFI failures with GCOV_PROFILE_ALL

[PATCH 0/2] Fix CFI failures with GCOV_PROFILE_ALL

[Sami Tolvanen]

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

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

[Sami Tolvanen]

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 <joefradley@google.com>
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 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

[PATCH 2/2] kbuild: Disable GCOV for *.mod.o

[Sami Tolvanen]

With GCOV_PROFILE_ALL, Clang injects __llvm_gcov_* functions to each
object file, including the *.mod.o. As we filter out CC_FLAGS_CFI
for *.mod.o, the compiler won't generate type hashes for the
injected functions, and therefore indirectly calling them during
module loading trips indirect call checking.

Enabling CFI for *.mod.o isn't sufficient to fix this issue after
commit 0c3e806ec0f9 ("x86/cfi: Add boot time hash randomization"),
as *.mod.o aren't processed by objtool, which means any hashes
emitted there won't be randomized. Therefore, in addition to
disabling CFI for *.mod.o, also disable GCOV, as the object files
don't otherwise contain any executable code.

Fixes: cf68fffb66d6 ("add support for Clang CFI")
Reported-by: Joe Fradley <joefradley@google.com>
Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
---
 scripts/Makefile.modfinal | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
index 4703f652c009..fc19f67039bd 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), $(c_flags)) -c -o $@ $<
+      cmd_cc_o_c = $(CC) $(filter-out $(CC_FLAGS_CFI) $(CFLAGS_GCOV), $(c_flags)) -c -o $@ $<
 
 %.mod.o: %.mod.c FORCE
 	$(call if_changed_dep,cc_o_c)
-- 
2.41.0.162.gfafddb0af9-goog

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

[Peter Zijlstra]

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) <peterz@infradead.org>

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?

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

[Kees Cook]

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 <keescook@chromium.org>

Should these get Cc: stable tags maybe?

-- 
Kees Cook

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

[Nick Desaulniers]

On Thu, Jun 22, 2023 at 5:11 PM Sami Tolvanen <samitolvanen@google.com> 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 <ndesaulniers@google.com>

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


-- 
Thanks,
~Nick Desaulniers

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

[Sami Tolvanen]

On Fri, Jun 23, 2023 at 9:32 AM Kees Cook <keescook@chromium.org> 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 <keescook@chromium.org>
>
> 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

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

[Masahiro Yamada]

On Sat, Jun 24, 2023 at 5:39 AM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> On Fri, Jun 23, 2023 at 9:32 AM Kees Cook <keescook@chromium.org> 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 <keescook@chromium.org>
> >
> > 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