linux-kernel - Re: [PATCH] livepatch: fixup klp-convert tool integration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.LSU.2.20.1612191524490.6638@pobox.suse.cz>
Date:   Mon, 19 Dec 2016 15:29:41 +0100 (CET)
From:   Miroslav Benes <mbenes@...e.cz>
To:     Konstantin Khlebnikov <khlebnikov@...dex-team.ru>
cc:     live-patching@...r.kernel.org,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] livepatch: fixup klp-convert tool integration

On Sat, 17 Dec 2016, Konstantin Khlebnikov wrote:

> I've found some minor problems, this patch fixes:
> 
> * save cmd_ld_ko_o into .module.cmd, if_changed_rule doesn't do that
> * fix bashisms for debian where /bin/sh is a symlink to /bin/dash
> * rename rule_link_module to rule_ld_ko_o, otherwise arg-check inside
>   if_changed_rule compares cmd_link_module and cmd_ld_ko_o
> * use HOSTLOADLIBES_$module instead of HOSTLDFLAGS: -lelf must be at the end

Ok, why not. I'll incorporate these changes to the patch set, if you're ok 
with it.

> * check modinfo -F livepatch only if CONFIG_LIVEPATCH is true

Right, this was the case with Josh's changes too if I understand modpost 
correctly (still causing me headaches).
 
> I think "modinfo -F" could be replaced with explicit mark in makefile,
> for example: LIVEPATCH_module.ko := y (like KASAN_SANITIZE_obj.o := n).

Yes, replacing modinfo is definitely something I plan to do and explicit 
mark in Makefile is one option how to deal with it. I also though about a 
mark somewhere in a source code, but it does not matter much, I guess.

Thanks,
Miroslav

> Signed-off-by: Konstantin Khlebnikov <khlebnikov@...dex-team.ru>
> ---
>  scripts/Kbuild.include     |    4 +++-
>  scripts/Makefile.modpost   |   24 +++++++++++-------------
>  scripts/livepatch/Makefile |    2 +-
>  3 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 179219845dfc..e299fde3423b 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -247,6 +247,8 @@ endif
>  # (needed for the shell)
>  make-cmd = $(call escsq,$(subst \#,\\\#,$(subst $$,$$$$,$(cmd_$(1)))))
>  
> +save-cmd = printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd
> +
>  # Find any prerequisites that is newer than target or that does not exist.
>  # PHONY targets skipped in both cases.
>  any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)
> @@ -256,7 +258,7 @@ any-prereq = $(filter-out $(PHONY),$?) $(filter-out $(PHONY) $(wildcard $^),$^)
>  if_changed = $(if $(strip $(any-prereq) $(arg-check)),                       \
>  	@set -e;                                                             \
>  	$(echo-cmd) $(cmd_$(1));                                             \
> -	printf '%s\n' 'cmd_$@ := $(make-cmd)' > $(dot-target).cmd, @:)
> +	$(save-cmd), @:)
>  
>  # Execute the command and also postprocess generated .d dependencies file.
>  if_changed_dep = $(if $(strip $(any-prereq) $(arg-check) ),                  \
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index 916dd347e8f6..5d149d0b05c2 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -123,24 +123,22 @@ quiet_cmd_ld_ko_o = LD [M]  $@
>  	$(LD) -r $(LDFLAGS)                                             \
>                   $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)             \
>                   -o $@ $(filter-out FORCE,$^) ;                         \
> -	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true) ;
> +	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
>  
> -ifdef CONFIG_LIVEPATCH
>  KLP_CONVERT = scripts/livepatch/klp-convert
> -      cmd_klp_convert =							\
> -	if [[ -n "`modinfo -F livepatch $@`" ]]; then			\
> -		mv $@ $(@:.ko=.klp.o);					\
> -		$(KLP_CONVERT) $(@:.ko=.klp.o) $@;			\
> -	fi ;
> -endif
> -
> -define rule_link_module
> -	$(call echo-cmd,ld_ko_o) $(cmd_ld_ko_o)				\
> -	$(cmd_klp_convert)
> +quiet_cmd_klp_convert = LIVEPATCH $@
> +      cmd_klp_convert = mv $@ $(@:.ko=.klp.o); $(KLP_CONVERT) $(@:.ko=.klp.o) $@
> +
> +define rule_ld_ko_o
> +       $(call echo-cmd,ld_ko_o) $(cmd_ld_ko_o) ;                        \
> +       $(call save-cmd,ld_ko_o) ;                                       \
> +       $(if $(CONFIG_LIVEPATCH),                                        \
> +         if [ -n "`modinfo -F livepatch $@`" ] ; then                   \
> +           $(call echo-cmd,klp_convert) $(cmd_klp_convert) ; fi)
>  endef
>  
>  $(modules): %.ko :%.o %.mod.o FORCE
> -	+$(call if_changed_rule,link_module)
> +	+$(call if_changed_rule,ld_ko_o)
>  
>  targets += $(modules)
>  
> diff --git a/scripts/livepatch/Makefile b/scripts/livepatch/Makefile
> index 221829bb34c7..bd5c1ae553ab 100644
> --- a/scripts/livepatch/Makefile
> +++ b/scripts/livepatch/Makefile
> @@ -4,4 +4,4 @@ always			:= $(hostprogs-y)
>  klp-convert-objs	:= klp-convert.o elf.o
>  
>  HOSTCFLAGS		:= -g -I$(INSTALL_HDR_PATH)/include -Wall
> -HOSTLDFLAGS		:= -lelf
> +HOSTLOADLIBES_klp-convert	:= -lelf
> 
> --
> To unsubscribe from this list: send the line "unsubscribe live-patching" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>