Re: [PATCH v2 11/28] kbuild: lto: postpone objtool

[Kees Cook]

On Thu, Sep 03, 2020 at 01:30:36PM -0700, Sami Tolvanen wrote:
> With LTO, LLVM bitcode won't be compiled into native code until
> modpost_link, or modfinal for modules. This change postpones calls
> to objtool until after these steps.
> 
> Signed-off-by: Sami Tolvanen <samitolvanen@xxxxxxxxxx>

For a "fail fast" style of building, it makes sense to have objtool run
as early as possible, so it makes sense to keep the current behavior in
non-LTO mode. I do wonder, though, if there is a real benefit to having
"fail fast" case. I imagine a lot of automated builds are using
--keep-going with make, and actually waiting until the end to do the
validation means more code will get build-tested before objtool rejects
the results. *shrug*

> ---
>  arch/Kconfig              |  2 +-
>  scripts/Makefile.build    |  2 ++
>  scripts/Makefile.modfinal | 24 ++++++++++++++++++++++--
>  scripts/link-vmlinux.sh   | 23 ++++++++++++++++++++++-
>  4 files changed, 47 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 71392e4a8900..7a418907e686 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -599,7 +599,7 @@ config LTO_CLANG
>  	depends on $(success,$(NM) --help | head -n 1 | grep -qi llvm)
>  	depends on $(success,$(AR) --help | head -n 1 | grep -qi llvm)
>  	depends on ARCH_SUPPORTS_LTO_CLANG
> -	depends on !FTRACE_MCOUNT_RECORD
> +	depends on HAVE_OBJTOOL_MCOUNT || !(X86_64 && DYNAMIC_FTRACE)
>  	depends on !KASAN
>  	depends on !GCOV_KERNEL
>  	select LTO
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index c348e6d6b436..b8f1f0d65a73 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -218,6 +218,7 @@ cmd_record_mcount = $(if $(findstring $(strip $(CC_FLAGS_FTRACE)),$(_c_flags)),
>  endif # USE_RECORDMCOUNT
>  
>  ifdef CONFIG_STACK_VALIDATION
> +ifndef CONFIG_LTO_CLANG
>  ifneq ($(SKIP_STACK_VALIDATION),1)
>  
>  __objtool_obj := $(objtree)/tools/objtool/objtool
> @@ -253,6 +254,7 @@ objtool_obj = $(if $(patsubst y%,, \
>  	$(__objtool_obj))
>  
>  endif # SKIP_STACK_VALIDATION
> +endif # CONFIG_LTO_CLANG
>  endif # CONFIG_STACK_VALIDATION
>  
>  # Rebuild all objects when objtool changes, or is enabled/disabled.
> diff --git a/scripts/Makefile.modfinal b/scripts/Makefile.modfinal
> index 1005b147abd0..909bd509edb4 100644
> --- a/scripts/Makefile.modfinal
> +++ b/scripts/Makefile.modfinal
> @@ -34,10 +34,30 @@ ifdef CONFIG_LTO_CLANG
>  # With CONFIG_LTO_CLANG, reuse the object file we compiled for modpost to
>  # avoid a second slow LTO link
>  prelink-ext := .lto
> -endif
> +
> +# ELF processing was skipped earlier because we didn't have native code,
> +# so let's now process the prelinked binary before we link the module.
> +
> +ifdef CONFIG_STACK_VALIDATION
> +ifneq ($(SKIP_STACK_VALIDATION),1)
> +cmd_ld_ko_o +=								\
> +	$(objtree)/tools/objtool/objtool				\
> +		$(if $(CONFIG_UNWINDER_ORC),orc generate,check)		\
> +		--module						\
> +		$(if $(CONFIG_FRAME_POINTER),,--no-fp)			\
> +		$(if $(CONFIG_GCOV_KERNEL),--no-unreachable,)		\
> +		$(if $(CONFIG_RETPOLINE),--retpoline,)			\
> +		$(if $(CONFIG_X86_SMAP),--uaccess,)			\
> +		$(if $(USE_OBJTOOL_MCOUNT),--mcount,)			\
> +		$(@:.ko=$(prelink-ext).o);
> +
> +endif # SKIP_STACK_VALIDATION
> +endif # CONFIG_STACK_VALIDATION

I wonder if objtool_args could be reused here instead of having two
places to keep in sync? It looks like that might mean moving things
around a bit before this patch, since I can't quite see if
Makefile.build's variables are visible to Makefile.modfinal?

> +
> +endif # CONFIG_LTO_CLANG
>  
>  quiet_cmd_ld_ko_o = LD [M]  $@
> -      cmd_ld_ko_o =                                                     \
> +      cmd_ld_ko_o +=                                                     \
>  	$(LD) -r $(KBUILD_LDFLAGS)					\
>  		$(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)		\
>  		$(addprefix -T , $(KBUILD_LDS_MODULE))			\
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 3e99a19b9195..a352a5ad9ef7 100755
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -93,8 +93,29 @@ objtool_link()
>  {
>  	local objtoolopt;
>  
> +	if [ "${CONFIG_LTO_CLANG} ${CONFIG_STACK_VALIDATION}" = "y y" ]; then
> +		# Don't perform vmlinux validation unless explicitly requested,
> +		# but run objtool on vmlinux.o now that we have an object file.
> +		if [ -n "${CONFIG_UNWINDER_ORC}" ]; then
> +			objtoolopt="orc generate"
> +		else
> +			objtoolopt="check"
> +		fi
> +
> +		if [ -n ${USE_OBJTOOL_MCOUNT} ]; then
> +			objtoolopt="${objtoolopt} --mcount"
> +		fi
> +	fi
> +
>  	if [ -n "${CONFIG_VMLINUX_VALIDATION}" ]; then
> -		objtoolopt="check --vmlinux"
> +		if [ -z "${objtoolopt}" ]; then
> +			objtoolopt="check --vmlinux"
> +		else
> +			objtoolopt="${objtoolopt} --vmlinux"
> +		fi
> +	fi
> +
> +	if [ -n "${objtoolopt}" ]; then
>  		if [ -z "${CONFIG_FRAME_POINTER}" ]; then
>  			objtoolopt="${objtoolopt} --no-fp"
>  		fi
> -- 
> 2.28.0.402.g5ffc5be6b7-goog
> 

-- 
Kees Cook