mbox series

[0/2] Fix CFI failures with GCOV_PROFILE_ALL

Message ID 20230623001140.1111494-4-samitolvanen@google.com
Headers
Series Fix CFI failures with GCOV_PROFILE_ALL |

Message

Sami Tolvanen June 23, 2023, 12:11 a.m. UTC 
  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

Comments

Peter Zijlstra June 23, 2023, 9:13 a.m. UTC | #1 
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?
Kees Cook June 23, 2023, 4:32 p.m. UTC | #2 
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?
Nick Desaulniers June 23, 2023, 5:47 p.m. UTC | #3 
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
>
>
Sami Tolvanen June 23, 2023, 8:38 p.m. UTC | #4 
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
Masahiro Yamada June 24, 2023, 8:43 a.m. UTC | #5 
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.