diff mbox series

[v3,2/9] kbuild: Support for Symbols.list creation

Message ID 20190410155058.9437-3-joe.lawrence@redhat.com (mailing list archive)
State New, archived
Headers show
Series klp-convert livepatch build tooling | expand

Commit Message

Joe Lawrence April 10, 2019, 3:50 p.m. UTC 
From: Joao Moreira <jmoreira@suse.de>

For automatic resolution of livepatch relocations, a file called
Symbols.list is used. This file maps symbols within every compiled
kernel object allowing the identification of symbols whose name is
unique, thus relocation can be automatically inferred, or providing
information that helps developers when code annotation is required for
solving the matter.

Add support for creating Symbols.list in the main Makefile. First,
ensure that built-in is compiled when CONFIG_LIVEPATCH is enabled (as
required to achieve a complete Symbols.list file). Define the command to
build Symbols.list (cmd_klp_map) and hook it in the modules rule.

As it is undesirable to have symbols from livepatch objects inside
Symbols.list, make livepatches discernible by modifying
scripts/Makefile.build to create a .livepatch file for each livepatch
in $(MODVERDIR). This file then used by cmd_klp_map to identify and
bypass livepatches.

For identifying livepatches during the build process, a flag variable
LIVEPATCH_$(basetarget).o is considered in scripts/Makefile.build. This
way, set this flag for the livepatch sample Makefile in
samples/livepatch/Makefile.

Finally, Add a clean rule to ensure that Symbols.list is removed during
clean.

Notes:

To achieve a correct Symbols.list file, all kernel objects must be
considered, thus, its construction require these objects to be priorly
built. On the other hand, invoking scripts/Makefile.modpost without
having a complete Symbols.list in place would occasionally lead to
in-tree livepatches being post-processed incorrectly. To prevent this
from becoming a circular dependency, the construction of Symbols.list
uses non-post-processed kernel objects and such does not cause harm as
the symbols normally referenced from within livepatches are visible at
this stage. Also due to these requirements, the spot in-between modules
compilation and the invocation of scripts/Makefile.modpost was picked
for hooking cmd_klp_map.

The approach based on .livepatch files was proposed as an alternative
to using MODULE_INFO statements. This approach was originally
proposed by Miroslav Benes as a workaround for identifying livepathes
without depending on modinfo during the modpost stage. It was moved to
this patch as the approach also shown to be useful while building
Symbols.list.

Signed-off-by: Joao Moreira <jmoreira@suse.de>
Signed-off-by: Joe Lawrence <joe.lawrence@redhat.com>
---
 .gitignore                 |  1 +
 Makefile                   | 30 ++++++++++++++++++++++++++----
 samples/livepatch/Makefile |  1 +
 scripts/Makefile.build     |  7 +++++++
 4 files changed, 35 insertions(+), 4 deletions(-)

Comments

Artem Savkov April 11, 2019, 9:18 a.m. UTC | #1 
On Wed, Apr 10, 2019 at 11:50:51AM -0400, Joe Lawrence wrote:
> -clean: archclean vmlinuxclean
> +klpclean:
> +	$(Q) rm -f $(objtree)/Symbols.list

nit: $(SLIST) can be used here.

> +clean: archclean vmlinuxclean klpclean
>  
>  # mrproper - Delete all generated files, including .config
>  #
> diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
> index 2472ce39a18d..8b9b42a258ad 100644
> --- a/samples/livepatch/Makefile
> +++ b/samples/livepatch/Makefile
> @@ -1,3 +1,4 @@
> +LIVEPATCH_livepatch-sample := y
>  obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-sample.o
>  obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-mod.o
>  obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-fix1.o
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 76ca30cc4791..ca76bd2080f0 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -246,6 +246,11 @@ cmd_gen_ksymdeps = \
>  	$(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
>  endif
>  
> +ifdef CONFIG_LIVEPATCH
> +cmd_livepatch = $(if $(LIVEPATCH_$(basetarget)),		\
> +	$(shell touch $(MODVERDIR)/$(basetarget).livepatch))
> +endif
> +
>  define rule_cc_o_c
>  	$(call cmd,checksrc)
>  	$(call cmd_and_fixdep,cc_o_c)
> @@ -280,6 +285,7 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
>  $(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
>  	$(call cmd,force_checksrc)
>  	$(call if_changed_rule,cc_o_c)
> +	$(call cmd_livepatch)

nit: maybe use "cmd,livepatch" to be consistent with the other call of
this function.

>  	@{ echo $(@:.o=.ko); echo $@; \
>  	   $(cmd_undef_syms); } > $(MODVERDIR)/$(@F:.o=.mod)
>  
> @@ -456,6 +462,7 @@ cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^) $(cmd_secanalysis
>  
>  $(multi-used-m): FORCE
>  	$(call if_changed,link_multi-m)
> +	$(call cmd,livepatch)
>  	@{ echo $(@:.o=.ko); echo $(filter %.o,$^); \
>  	   $(cmd_undef_syms); } > $(MODVERDIR)/$(@F:.o=.mod)
>  $(call multi_depend, $(multi-used-m), .o, -objs -y -m)
> -- 
> 2.20.1
>
Joe Lawrence April 11, 2019, 3:18 p.m. UTC | #2 
On 4/11/19 5:18 AM, Artem Savkov wrote:
> On Wed, Apr 10, 2019 at 11:50:51AM -0400, Joe Lawrence wrote:
>> -clean: archclean vmlinuxclean
>> +klpclean:
>> +	$(Q) rm -f $(objtree)/Symbols.list
> 
> nit: $(SLIST) can be used here.
> 
>> +clean: archclean vmlinuxclean klpclean
>>   
>>   # mrproper - Delete all generated files, including .config
>>   #
>> diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
>> index 2472ce39a18d..8b9b42a258ad 100644
>> --- a/samples/livepatch/Makefile
>> +++ b/samples/livepatch/Makefile
>> @@ -1,3 +1,4 @@
>> +LIVEPATCH_livepatch-sample := y
>>   obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-sample.o
>>   obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-mod.o
>>   obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-fix1.o
>> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
>> index 76ca30cc4791..ca76bd2080f0 100644
>> --- a/scripts/Makefile.build
>> +++ b/scripts/Makefile.build
>> @@ -246,6 +246,11 @@ cmd_gen_ksymdeps = \
>>   	$(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
>>   endif
>>   
>> +ifdef CONFIG_LIVEPATCH
>> +cmd_livepatch = $(if $(LIVEPATCH_$(basetarget)),		\
>> +	$(shell touch $(MODVERDIR)/$(basetarget).livepatch))
>> +endif
>> +
>>   define rule_cc_o_c
>>   	$(call cmd,checksrc)
>>   	$(call cmd_and_fixdep,cc_o_c)
>> @@ -280,6 +285,7 @@ $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
>>   $(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
>>   	$(call cmd,force_checksrc)
>>   	$(call if_changed_rule,cc_o_c)
>> +	$(call cmd_livepatch)
> 
> nit: maybe use "cmd,livepatch" to be consistent with the other call of
> this function.
> 

Both of these make sense, thanks Artem.

-- Joe
Miroslav Benes April 11, 2019, 7:04 p.m. UTC | #3 
> diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
> index 2472ce39a18d..8b9b42a258ad 100644
> --- a/samples/livepatch/Makefile
> +++ b/samples/livepatch/Makefile
> @@ -1,3 +1,4 @@
> +LIVEPATCH_livepatch-sample := y
>  obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-sample.o
>  obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-mod.o
>  obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-fix1.o

We discussed this and I think there has been no conclusion yet. Shouldn't 
we do the same (add LIVEPATCH_) for the other samples here as well? So 
they are not picked up by cmd_klp_map and their symbols stored in 
Symbols.list?

Miroslav
Joe Lawrence April 16, 2019, 2:13 p.m. UTC | #4 
On 4/11/19 3:04 PM, Miroslav Benes wrote:
> 
>> diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
>> index 2472ce39a18d..8b9b42a258ad 100644
>> --- a/samples/livepatch/Makefile
>> +++ b/samples/livepatch/Makefile
>> @@ -1,3 +1,4 @@
>> +LIVEPATCH_livepatch-sample := y
>>   obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-sample.o
>>   obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-mod.o
>>   obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-fix1.o
> 
> We discussed this and I think there has been no conclusion yet. Shouldn't
> we do the same (add LIVEPATCH_) for the other samples here as well?

In v2 review, I mentioned moving this hunk to ("modpost: Add modinfo 
flag to livepatch modules") along with the same change to the other 
sample modules.  I didn't apply that for v3, but can do so for the next 
spin if it makes the commits easier to review.

-- Joe
Miroslav Benes April 16, 2019, 7:02 p.m. UTC | #5 
On Tue, 16 Apr 2019, Joe Lawrence wrote:

> On 4/11/19 3:04 PM, Miroslav Benes wrote:
> > 
> >> diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
> >> index 2472ce39a18d..8b9b42a258ad 100644
> >> --- a/samples/livepatch/Makefile
> >> +++ b/samples/livepatch/Makefile
> >> @@ -1,3 +1,4 @@
> >> +LIVEPATCH_livepatch-sample := y
> >>   obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-sample.o
> >>   obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-mod.o
> >>   obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-fix1.o
> > 
> > We discussed this and I think there has been no conclusion yet. Shouldn't
> > we do the same (add LIVEPATCH_) for the other samples here as well?
> 
> In v2 review, I mentioned moving this hunk to ("modpost: Add modinfo flag to
> livepatch modules") along with the same change to the other sample modules.  I
> didn't apply that for v3, but can do so for the next spin if it makes the
> commits easier to review.

I think it belongs here, but I may be missing something. Anyway, it is not 
really important, so I'd leave it up to you.

Miroslav
diff mbox series

Patch

diff --git a/.gitignore b/.gitignore
index a20ac26aa2f5..5cd5758f5ffe 100644
--- a/.gitignore
+++ b/.gitignore
@@ -45,6 +45,7 @@ 
 *.xz
 Module.symvers
 modules.builtin
+Symbols.list
 
 #
 # Top-level generic files
diff --git a/Makefile b/Makefile
index 15c8251d4d5e..2c07bdd87b2f 100644
--- a/Makefile
+++ b/Makefile
@@ -574,10 +574,13 @@  KBUILD_BUILTIN := 1
 # If we have only "make modules", don't compile built-in objects.
 # When we're building modules with modversions, we need to consider
 # the built-in objects during the descend as well, in order to
-# make sure the checksums are up to date before we record them.
+# make sure the checksums are up to date before we record them. The
+# same applies for building livepatches, as built-in objects may hold
+# symbols which are referenced from livepatches and are required by
+# klp-convert post-processing tool for resolving these cases.
 
 ifeq ($(MAKECMDGOALS),modules)
-  KBUILD_BUILTIN := $(if $(CONFIG_MODVERSIONS),1)
+  KBUILD_BUILTIN := $(if $(or $(CONFIG_MODVERSIONS), $(CONFIG_LIVEPATCH)),1)
 endif
 
 # If we have "make <whatever> modules", compile modules
@@ -1261,9 +1264,25 @@  all: modules
 # duplicate lines in modules.order files.  Those are removed
 # using awk while concatenating to the final file.
 
+quiet_cmd_klp_map = KLP     Symbols.list
+SLIST = $(objtree)/Symbols.list
+
+define cmd_klp_map
+	$(shell echo "klp-convert-symbol-data.0.1" > $(SLIST))				\
+	$(shell echo "*vmlinux" >> $(SLIST))						\
+	$(shell nm -f posix $(objtree)/vmlinux | cut -d\  -f1 >> $(SLIST))		\
+	$(foreach m, $(wildcard $(MODVERDIR)/*.mod),					\
+		$(eval mod = $(patsubst %.ko,%.o,$(shell head -n1 $(m))))		\
+		$(if $(wildcard $(MODVERDIR)/$(shell basename -s .o $(mod)).livepatch),,\
+			$(eval fmod = $(subst $(quote),_,$(subst -,_,$(mod))))		\
+			$(shell echo "*$(shell basename -s .o $(fmod))" >> $(SLIST))	\
+			$(shell nm -f posix $(mod) | cut -d\  -f1 >> $(SLIST))))
+endef
+
 PHONY += modules
 modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux) modules.builtin
 	$(Q)$(AWK) '!x[$$0]++' $(vmlinux-dirs:%=$(objtree)/%/modules.order) > $(objtree)/modules.order
+	$(if $(CONFIG_LIVEPATCH), $(call cmd,klp_map))
 	@$(kecho) '  Building modules, stage 2.';
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost
 
@@ -1350,7 +1369,7 @@  clean: rm-dirs  := $(CLEAN_DIRS)
 clean: rm-files := $(CLEAN_FILES)
 clean-dirs      := $(addprefix _clean_, . $(vmlinux-alldirs) Documentation samples)
 
-PHONY += $(clean-dirs) clean archclean vmlinuxclean
+PHONY += $(clean-dirs) clean archclean vmlinuxclean klpclean
 $(clean-dirs):
 	$(Q)$(MAKE) $(clean)=$(patsubst _clean_%,%,$@)
 
@@ -1358,7 +1377,10 @@  vmlinuxclean:
 	$(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
 	$(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
 
-clean: archclean vmlinuxclean
+klpclean:
+	$(Q) rm -f $(objtree)/Symbols.list
+
+clean: archclean vmlinuxclean klpclean
 
 # mrproper - Delete all generated files, including .config
 #
diff --git a/samples/livepatch/Makefile b/samples/livepatch/Makefile
index 2472ce39a18d..8b9b42a258ad 100644
--- a/samples/livepatch/Makefile
+++ b/samples/livepatch/Makefile
@@ -1,3 +1,4 @@ 
+LIVEPATCH_livepatch-sample := y
 obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-sample.o
 obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-mod.o
 obj-$(CONFIG_SAMPLE_LIVEPATCH) += livepatch-shadow-fix1.o
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 76ca30cc4791..ca76bd2080f0 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -246,6 +246,11 @@  cmd_gen_ksymdeps = \
 	$(CONFIG_SHELL) $(srctree)/scripts/gen_ksymdeps.sh $@ >> $(dot-target).cmd
 endif
 
+ifdef CONFIG_LIVEPATCH
+cmd_livepatch = $(if $(LIVEPATCH_$(basetarget)),		\
+	$(shell touch $(MODVERDIR)/$(basetarget).livepatch))
+endif
+
 define rule_cc_o_c
 	$(call cmd,checksrc)
 	$(call cmd_and_fixdep,cc_o_c)
@@ -280,6 +285,7 @@  $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
 $(single-used-m): $(obj)/%.o: $(src)/%.c $(recordmcount_source) $(objtool_dep) FORCE
 	$(call cmd,force_checksrc)
 	$(call if_changed_rule,cc_o_c)
+	$(call cmd_livepatch)
 	@{ echo $(@:.o=.ko); echo $@; \
 	   $(cmd_undef_syms); } > $(MODVERDIR)/$(@F:.o=.mod)
 
@@ -456,6 +462,7 @@  cmd_link_multi-m = $(LD) $(ld_flags) -r -o $@ $(filter %.o,$^) $(cmd_secanalysis
 
 $(multi-used-m): FORCE
 	$(call if_changed,link_multi-m)
+	$(call cmd,livepatch)
 	@{ echo $(@:.o=.ko); echo $(filter %.o,$^); \
 	   $(cmd_undef_syms); } > $(MODVERDIR)/$(@F:.o=.mod)
 $(call multi_depend, $(multi-used-m), .o, -objs -y -m)