Re: [PATCH 2/8] kbuild: Support for Symbols.list creation

2017-09-04 Thread Joao Moreira 



On 08/31/2017 12:24 PM, Joe Lawrence wrote:

Hi Joao,


Hi Joe and Josh,

Thanks for the quick feedback, I'll be looking forward for your comments 
once you have the change to dig deeper :). I'll apply all typo-fixes, 
add klpclean to the PHONY targets and change the quiet_cmd invocation as 
suggested in v2.



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.


I've never tried building a livepatch out of tree (is that even
possible?) but does this make it any more/less harder?


Tbh, I never had tried it before, so I just emulated the process and it 
was simple and successful. All it required in addition to what is 
regular, was the introduction of the "LIVEPATCH_($bastarget).o := y" 
into the Makefile. All else was implicitly handled by Kbuild.


Best,
Joao

Re: [PATCH 2/8] kbuild: Support for Symbols.list creation

2017-09-04 Thread Joao Moreira 



On 08/31/2017 12:24 PM, Joe Lawrence wrote:

Hi Joao,


Hi Joe and Josh,

Thanks for the quick feedback, I'll be looking forward for your comments 
once you have the change to dig deeper :). I'll apply all typo-fixes, 
add klpclean to the PHONY targets and change the quiet_cmd invocation as 
suggested in v2.



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.


I've never tried building a livepatch out of tree (is that even
possible?) but does this make it any more/less harder?


Tbh, I never had tried it before, so I just emulated the process and it 
was simple and successful. All it required in addition to what is 
regular, was the introduction of the "LIVEPATCH_($bastarget).o := y" 
into the Makefile. All else was implicitly handled by Kbuild.


Best,
Joao

Re: [PATCH 2/8] kbuild: Support for Symbols.list creation

2017-08-31 Thread Josh Poimboeuf 
On Thu, Aug 31, 2017 at 11:24:39AM -0400, Joe Lawrence wrote:
> > +quiet_cmd_klp_map = LIVEPATCH  Symbols.list
> 
> nit: I don't think any other quiet_cmd invocations put a tab between the
> label and file list.  That said, "LIVEPATCH" is > 8 chars, so it's not
> going to line up anyway.

Maybe "SYMBOLS" would be more appropriate (and would fit).

-- 
Josh

Re: [PATCH 2/8] kbuild: Support for Symbols.list creation

2017-08-31 Thread Josh Poimboeuf 
On Thu, Aug 31, 2017 at 11:24:39AM -0400, Joe Lawrence wrote:
> > +quiet_cmd_klp_map = LIVEPATCH  Symbols.list
> 
> nit: I don't think any other quiet_cmd invocations put a tab between the
> label and file list.  That said, "LIVEPATCH" is > 8 chars, so it's not
> going to line up anyway.

Maybe "SYMBOLS" would be more appropriate (and would fit).

-- 
Josh

Re: [PATCH 2/8] kbuild: Support for Symbols.list creation

2017-08-31 Thread Joe Lawrence 
Hi Joao,

A few nitpicks in-line below...

On Tue, Aug 29, 2017 at 04:01:34PM -0300, Joao Moreira wrote:
> 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 is 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.

I've never tried building a livepatch out of tree (is that even
possible?) but does this make it any more/less harder?
 
> 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 statementes. This approach was originally
   ^^^
nit: s/statementes/statements

> proposed by Miroslav Benes as a workaround for identifying livepathces
 ^^^
nit: s/livepathces/livepatches

> 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 
> ---
>  .gitignore |  1 +
>  Makefile   | 27 ---
>  samples/livepatch/Makefile |  1 +
>  scripts/Makefile.build |  6 ++
>  4 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 0c39aa20b6ba..e6a67517a3bc 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -39,6 +39,7 @@ Module.symvers
>  *.dwo
>  *.su
>  *.c.[012]*.*
> +Symbols.list
>  
>  #
>  # Top-level generic files
> diff --git a/Makefile b/Makefile
> index e40c471abe29..e1d315e585d8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -320,10 +320,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  modules", compile modules
> @@ -1207,9 +1210,24 @@ all: modules
>  # duplicate lines in modules.order files.  Those are removed
>  # using awk while concatenating to the final file.
>  
> +quiet_cmd_klp_map = LIVEPATCHSymbols.list

nit: I don't think any other quiet_cmd invocations put a tab between the
label and file list.  That said, "LIVEPATCH" is > 8 chars, so it's not
going to line up anyway.

> +SLIST = $(objtree)/Symbols.list
> +
> +define cmd_klp_map
> + $(shell echo "*vmlinux" > $(SLIST)) 
> \
> + $(shell nm -f posix $(objtree)/vmlinux | cut -d\  -f1 >> $(SLIST))  
> \
> + $(foreach m, $(wildcard $(MODVERDIR)/*.mod),
>

Re: [PATCH 2/8] kbuild: Support for Symbols.list creation

2017-08-31 Thread Joe Lawrence 
Hi Joao,

A few nitpicks in-line below...

On Tue, Aug 29, 2017 at 04:01:34PM -0300, Joao Moreira wrote:
> 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 is 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.

I've never tried building a livepatch out of tree (is that even
possible?) but does this make it any more/less harder?
 
> 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 statementes. This approach was originally
   ^^^
nit: s/statementes/statements

> proposed by Miroslav Benes as a workaround for identifying livepathces
 ^^^
nit: s/livepathces/livepatches

> 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 
> ---
>  .gitignore |  1 +
>  Makefile   | 27 ---
>  samples/livepatch/Makefile |  1 +
>  scripts/Makefile.build |  6 ++
>  4 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/.gitignore b/.gitignore
> index 0c39aa20b6ba..e6a67517a3bc 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -39,6 +39,7 @@ Module.symvers
>  *.dwo
>  *.su
>  *.c.[012]*.*
> +Symbols.list
>  
>  #
>  # Top-level generic files
> diff --git a/Makefile b/Makefile
> index e40c471abe29..e1d315e585d8 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -320,10 +320,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  modules", compile modules
> @@ -1207,9 +1210,24 @@ all: modules
>  # duplicate lines in modules.order files.  Those are removed
>  # using awk while concatenating to the final file.
>  
> +quiet_cmd_klp_map = LIVEPATCHSymbols.list

nit: I don't think any other quiet_cmd invocations put a tab between the
label and file list.  That said, "LIVEPATCH" is > 8 chars, so it's not
going to line up anyway.

> +SLIST = $(objtree)/Symbols.list
> +
> +define cmd_klp_map
> + $(shell echo "*vmlinux" > $(SLIST)) 
> \
> + $(shell nm -f posix $(objtree)/vmlinux | cut -d\  -f1 >> $(SLIST))  
> \
> + $(foreach m, $(wildcard $(MODVERDIR)/*.mod),
> \
> +

[PATCH 2/8] kbuild: Support for Symbols.list creation

2017-08-29 Thread Joao Moreira 
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 is 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 statementes. This approach was originally
proposed by Miroslav Benes as a workaround for identifying livepathces
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 
---
 .gitignore |  1 +
 Makefile   | 27 ---
 samples/livepatch/Makefile |  1 +
 scripts/Makefile.build |  6 ++
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/.gitignore b/.gitignore
index 0c39aa20b6ba..e6a67517a3bc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -39,6 +39,7 @@ Module.symvers
 *.dwo
 *.su
 *.c.[012]*.*
+Symbols.list
 
 #
 # Top-level generic files
diff --git a/Makefile b/Makefile
index e40c471abe29..e1d315e585d8 100644
--- a/Makefile
+++ b/Makefile
@@ -320,10 +320,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  modules", compile modules
@@ -1207,9 +1210,24 @@ all: modules
 # duplicate lines in modules.order files.  Those are removed
 # using awk while concatenating to the final file.
 
+quiet_cmd_klp_map = LIVEPATCH  Symbols.list
+SLIST = $(objtree)/Symbols.list
+
+define cmd_klp_map
+   $(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.';

[PATCH 2/8] kbuild: Support for Symbols.list creation

2017-08-29 Thread Joao Moreira 
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 is 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 statementes. This approach was originally
proposed by Miroslav Benes as a workaround for identifying livepathces
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 
---
 .gitignore |  1 +
 Makefile   | 27 ---
 samples/livepatch/Makefile |  1 +
 scripts/Makefile.build |  6 ++
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/.gitignore b/.gitignore
index 0c39aa20b6ba..e6a67517a3bc 100644
--- a/.gitignore
+++ b/.gitignore
@@ -39,6 +39,7 @@ Module.symvers
 *.dwo
 *.su
 *.c.[012]*.*
+Symbols.list
 
 #
 # Top-level generic files
diff --git a/Makefile b/Makefile
index e40c471abe29..e1d315e585d8 100644
--- a/Makefile
+++ b/Makefile
@@ -320,10 +320,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  modules", compile modules
@@ -1207,9 +1210,24 @@ all: modules
 # duplicate lines in modules.order files.  Those are removed
 # using awk while concatenating to the final file.
 
+quiet_cmd_klp_map = LIVEPATCH  Symbols.list
+SLIST = $(objtree)/Symbols.list
+
+define cmd_klp_map
+   $(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