Re: [PATCH v4 00/45] x86: Kernel IBT

From: Peter Zijlstra <peterz@infradead.org>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	X86 ML <x86@kernel.org>,
	joao@overdrivepizza.com, hjl.tools@gmail.com,
	Josh Poimboeuf <jpoimboe@redhat.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Kees Cook <keescook@chromium.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	Mark Rutland <mark.rutland@arm.com>,
	alyssa.milburn@intel.com, Miroslav Benes <mbenes@suse.cz>,
	Steven Rostedt <rostedt@goodmis.org>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>, bpf <bpf@vger.kernel.org>,
	masahiroy@kernel.org
Subject: Re: [PATCH v4 00/45] x86: Kernel IBT
Date: Tue, 15 Mar 2022 09:15:22 +0100	[thread overview]
Message-ID: <20220315081522.GA8939@worktop.programming.kicks-ass.net> (raw)
In-Reply-To: <Yi9YOdn5Nbq9BBwd@hirez.programming.kicks-ass.net>

On Mon, Mar 14, 2022 at 03:59:05PM +0100, Peter Zijlstra wrote:
> On Sun, Mar 13, 2022 at 09:52:14AM +0100, Peter Zijlstra wrote:
> > On Sat, Mar 12, 2022 at 05:33:39PM -0800, Alexei Starovoitov wrote:
> > > During the build with gcc 8.5 I see:
> > > 
> > > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > > .ibt_endbr_seal, skipping
> > > arch/x86/crypto/crc32c-intel.o: warning: objtool: file already has
> > > .orc_unwind section, skipping
> > >   LD [M]  crypto/async_tx/async_xor.ko
> > >   LD [M]  crypto/authenc.ko
> > > make[3]: *** [../scripts/Makefile.modfinal:61:
> > > arch/x86/crypto/crc32c-intel.ko] Error 255
> > > make[3]: *** Waiting for unfinished jobs....
> > > 
> > > but make clean cures it.
> > > I suspect it's some missing makefile dependency.
> > 
> > Yes, I recently ran into it; I've been trying to kick Makefile into
> > submission but have not had success yet. Will try again on Monday.
> > 
> > Problem appears to be that it will re-link .ko even though .o hasn't
> > changed, resulting in duplicate objtool runs. I've been trying to have
> > makefile generate .o.objtool empty file to serve as dependency marker to
> > avoid doing second objtool run, but like said, no luck yet.
> 
> Masahiro-san, I'm trying the below, but afaict it's not working because
> the rule for the .o file itself:
> 
Ha, sleep, it is marvelous!

The below appears to be working as desired.

---
Index: linux-2.6/scripts/Makefile.build
===================================================================

--- linux-2.6.orig/scripts/Makefile.build
+++ linux-2.6/scripts/Makefile.build
@@ -86,12 +86,18 @@ ifdef need-builtin
 targets-for-builtin += $(obj)/built-in.a
 endif
 
-targets-for-modules := $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
+targets-for-modules :=
 
 ifdef CONFIG_LTO_CLANG
 targets-for-modules += $(patsubst %.o, %.lto.o, $(filter %.o, $(obj-m)))
 endif
 
+ifdef CONFIG_X86_KERNEL_IBT
+targets-for-modules += $(patsubst %.o, %.objtool, $(filter %.o, $(obj-m)))
+endif
+
+targets-for-modules += $(patsubst %.o, %.mod, $(filter %.o, $(obj-m)))
+
 ifdef need-modorder
 targets-for-modules += $(obj)/modules.order
 endif
@@ -276,6 +282,19 @@ cmd_mod = { \
 $(obj)/%.mod: $(obj)/%$(mod-prelink-ext).o FORCE
 	$(call if_changed,mod)
 
+#
+# Since objtool will re-write the file it will change the timestamps, therefore
+# it is critical that the %.objtool file gets a timestamp *after* objtool runs.
+#
+# Additionally, care must be had with ordering this rule against the other rules
+# that take %.o as a dependency.
+#
+cmd_objtool_mod =							\
+	true $(cmd_objtool) ; touch $@
+
+$(obj)/%.objtool: $(obj)/%$(mod-prelink-ext).o FORCE
+	$(call if_changed,objtool_mod)
+
 quiet_cmd_cc_lst_c = MKLST   $@
       cmd_cc_lst_c = $(CC) $(c_flags) -g -c -o $*.o $< && \
 		     $(CONFIG_SHELL) $(srctree)/scripts/makelst $*.o \
Index: linux-2.6/scripts/Makefile.lib
===================================================================
--- linux-2.6.orig/scripts/Makefile.lib
+++ linux-2.6/scripts/Makefile.lib
@@ -552,9 +552,8 @@ objtool_args =								\
 	$(if $(CONFIG_FTRACE_MCOUNT_USE_OBJTOOL), --mcount)		\
 	$(if $(CONFIG_SLS), --sls)
 
-cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $@)
-cmd_objtool_mod = $(if $(objtool-enabled), $(objtool) $(objtool_args) $(@:.ko=.o) ; )
-cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$@: $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
+cmd_objtool = $(if $(objtool-enabled), ; $(objtool) $(objtool_args) $(@:.objtool=.o))
+cmd_gen_objtooldep = $(if $(objtool-enabled), { echo ; echo '$(@:.objtool=.o): $$(wildcard $(objtool))' ; } >> $(dot-target).cmd)
 
 endif # CONFIG_STACK_VALIDATION
 
@@ -575,8 +574,8 @@ $(obj)/%.o: objtool-enabled :=
 
 # instead run objtool on the module as a whole, right before
 # the final link pass with the linker script.
-%.ko: objtool-enabled = y
-%.ko: part-of-module := y
+$(obj)/%.objtool: objtool-enabled = y
+$(obj)/%.objtool: part-of-module := y
 
 else
 
Index: linux-2.6/scripts/Makefile.modfinal
===================================================================
--- linux-2.6.orig/scripts/Makefile.modfinal
+++ linux-2.6/scripts/Makefile.modfinal
@@ -32,7 +32,6 @@ ARCH_POSTLINK := $(wildcard $(srctree)/a
 
 quiet_cmd_ld_ko_o = LD [M]  $@
       cmd_ld_ko_o +=							\
-	$(cmd_objtool_mod)						\
 	$(LD) -r $(KBUILD_LDFLAGS)					\
 		$(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)		\
 		-T scripts/module.lds -o $@ $(filter %.o, $^);		\
