diff mbox series

[v2] module: allow UNUSED_KSYMS_WHITELIST to be relative against objtree.

Message ID 20240410194802.62036-1-elsk@google.com (mailing list archive)
State New, archived
Headers show
Series [v2] module: allow UNUSED_KSYMS_WHITELIST to be relative against objtree. | expand

Commit Message

Hong, Yifan April 10, 2024, 7:48 p.m. UTC
If UNUSED_KSYMS_WHITELIST is a file generated
before Kbuild runs, and the source tree is in
a read-only filesystem, the developer must put
the file somewhere and specify an absolute
path to UNUSED_KSYMS_WHITELIST. This worked,
but if IKCONFIG=y, an absolute path is embedded
into .config and eventually into vmlinux, causing
the build to be less reproducible when building
on a different machine.

This patch makes the handling of
UNUSED_KSYMS_WHITELIST to be similar to
MODULE_SIG_KEY.

First, check if UNUSED_KSYMS_WHITELIST is an
absolute path, just as before this patch. If so,
use the path as is.

If it is a relative path, use wildcard to check
the existence of the file below objtree first.
If it does not exist, fall back to the original
behavior of adding $(srctree)/ before the value.

After this patch, the developer can put the generated
file in objtree, then use a relative path against
objtree in .config, eradicating any absolute paths
that may be evaluated differently on different machines.

Signed-off-by: Yifan Hong <elsk@google.com>
---
V1 -> V2: properly handle absolute paths by treating
  them as-is.

 kernel/module/Kconfig    | 2 +-
 scripts/Makefile.modpost | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Elliot Berman April 10, 2024, 8:27 p.m. UTC | #1
On Wed, Apr 10, 2024 at 07:48:02PM +0000, Yifan Hong wrote:
> If UNUSED_KSYMS_WHITELIST is a file generated
> before Kbuild runs, and the source tree is in
> a read-only filesystem, the developer must put
> the file somewhere and specify an absolute
> path to UNUSED_KSYMS_WHITELIST. This worked,
> but if IKCONFIG=y, an absolute path is embedded
> into .config and eventually into vmlinux, causing
> the build to be less reproducible when building
> on a different machine.
> 
> This patch makes the handling of
> UNUSED_KSYMS_WHITELIST to be similar to
> MODULE_SIG_KEY.
> 
> First, check if UNUSED_KSYMS_WHITELIST is an
> absolute path, just as before this patch. If so,
> use the path as is.
> 
> If it is a relative path, use wildcard to check
> the existence of the file below objtree first.
> If it does not exist, fall back to the original
> behavior of adding $(srctree)/ before the value.
> 
> After this patch, the developer can put the generated
> file in objtree, then use a relative path against
> objtree in .config, eradicating any absolute paths
> that may be evaluated differently on different machines.
> 
> Signed-off-by: Yifan Hong <elsk@google.com>

I wonder if we should have a macro to do it so we can have a common
macro for the other time this is done for sig-key in
scripts/Makefile.modinst?

maybe-srctree = $(if $(wildcard $1),,$(srctree)/)$1

I'd let Masahiro/others decide if it's worth it. Otherwise,

Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>

> ---
> V1 -> V2: properly handle absolute paths by treating
>   them as-is.
> 
>  kernel/module/Kconfig    | 2 +-
>  scripts/Makefile.modpost | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> index f3e0329337f6..cb8377a18927 100644
> --- a/kernel/module/Kconfig
> +++ b/kernel/module/Kconfig
> @@ -392,7 +392,7 @@ config UNUSED_KSYMS_WHITELIST
>  	  exported at all times, even in absence of in-tree users. The value to
>  	  set here is the path to a text file containing the list of symbols,
>  	  one per line. The path can be absolute, or relative to the kernel
> -	  source tree.
> +	  source or obj tree.
>  
>  config MODULES_TREE_LOOKUP
>  	def_bool y
> diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> index 739402f45509..36952638bbc6 100644
> --- a/scripts/Makefile.modpost
> +++ b/scripts/Makefile.modpost
> @@ -94,7 +94,7 @@ targets += .vmlinux.objs
>  
>  ifdef CONFIG_TRIM_UNUSED_KSYMS
>  ksym-wl := $(CONFIG_UNUSED_KSYMS_WHITELIST)
> -ksym-wl := $(if $(filter-out /%, $(ksym-wl)),$(srctree)/)$(ksym-wl)
> +ksym-wl := $(if $(filter-out /%, $(ksym-wl)),$(if $(wildcard $(ksym-wl)),,$(srctree)/))$(ksym-wl)
>  modpost-args += -t $(addprefix -u , $(ksym-wl))
>  modpost-deps += $(ksym-wl)
>  endif
> -- 
> 2.44.0.478.gd926399ef9-goog
> 
>
Hong, Yifan April 10, 2024, 8:41 p.m. UTC | #2
On Wed, Apr 10, 2024 at 1:27 PM Elliot Berman <quic_eberman@quicinc.com> wrote:
>
> On Wed, Apr 10, 2024 at 07:48:02PM +0000, Yifan Hong wrote:
> > If UNUSED_KSYMS_WHITELIST is a file generated
> > before Kbuild runs, and the source tree is in
> > a read-only filesystem, the developer must put
> > the file somewhere and specify an absolute
> > path to UNUSED_KSYMS_WHITELIST. This worked,
> > but if IKCONFIG=y, an absolute path is embedded
> > into .config and eventually into vmlinux, causing
> > the build to be less reproducible when building
> > on a different machine.
> >
> > This patch makes the handling of
> > UNUSED_KSYMS_WHITELIST to be similar to
> > MODULE_SIG_KEY.
> >
> > First, check if UNUSED_KSYMS_WHITELIST is an
> > absolute path, just as before this patch. If so,
> > use the path as is.
> >
> > If it is a relative path, use wildcard to check
> > the existence of the file below objtree first.
> > If it does not exist, fall back to the original
> > behavior of adding $(srctree)/ before the value.
> >
> > After this patch, the developer can put the generated
> > file in objtree, then use a relative path against
> > objtree in .config, eradicating any absolute paths
> > that may be evaluated differently on different machines.
> >
> > Signed-off-by: Yifan Hong <elsk@google.com>
>
> I wonder if we should have a macro to do it so we can have a common
> macro for the other time this is done for sig-key in
> scripts/Makefile.modinst?
>
> maybe-srctree = $(if $(wildcard $1),,$(srctree)/)$1
>
> I'd let Masahiro/others decide if it's worth it. Otherwise,
>
> Reviewed-by: Elliot Berman <quic_eberman@quicinc.com>

Thanks for your review!

Also maybe relevant: I notice that SYSTEM_TRUSTED_KEYS is
yet another path in .config, but it is used as-is in
certs/Makefile, without adding the $(srctree) prefix. If we had this
macro, I think it should be applied to all of
UNUSED_KSYMS_WHITELIST, MODULE_SIG_KEY,
SYSTEM_TRUSTED_KEYS, and potentially other paths.

But these configs are beyond my knowledge and beyond the scope
of this patch, so I decide not to touch them at this moment.

>
> > ---
> > V1 -> V2: properly handle absolute paths by treating
> >   them as-is.
> >
> >  kernel/module/Kconfig    | 2 +-
> >  scripts/Makefile.modpost | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
> > index f3e0329337f6..cb8377a18927 100644
> > --- a/kernel/module/Kconfig
> > +++ b/kernel/module/Kconfig
> > @@ -392,7 +392,7 @@ config UNUSED_KSYMS_WHITELIST
> >         exported at all times, even in absence of in-tree users. The value to
> >         set here is the path to a text file containing the list of symbols,
> >         one per line. The path can be absolute, or relative to the kernel
> > -       source tree.
> > +       source or obj tree.
> >
> >  config MODULES_TREE_LOOKUP
> >       def_bool y
> > diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
> > index 739402f45509..36952638bbc6 100644
> > --- a/scripts/Makefile.modpost
> > +++ b/scripts/Makefile.modpost
> > @@ -94,7 +94,7 @@ targets += .vmlinux.objs
> >
> >  ifdef CONFIG_TRIM_UNUSED_KSYMS
> >  ksym-wl := $(CONFIG_UNUSED_KSYMS_WHITELIST)
> > -ksym-wl := $(if $(filter-out /%, $(ksym-wl)),$(srctree)/)$(ksym-wl)
> > +ksym-wl := $(if $(filter-out /%, $(ksym-wl)),$(if $(wildcard $(ksym-wl)),,$(srctree)/))$(ksym-wl)
> >  modpost-args += -t $(addprefix -u , $(ksym-wl))
> >  modpost-deps += $(ksym-wl)
> >  endif
> > --
> > 2.44.0.478.gd926399ef9-goog
> >
> >
Luis Chamberlain April 11, 2024, 4:29 p.m. UTC | #3
On Wed, Apr 10, 2024 at 07:48:02PM +0000, Yifan Hong wrote:
> If UNUSED_KSYMS_WHITELIST is a file generated
> before Kbuild runs, and the source tree is in
> a read-only filesystem, the developer must put
> the file somewhere and specify an absolute
> path to UNUSED_KSYMS_WHITELIST. This worked,
> but if IKCONFIG=y, an absolute path is embedded
> into .config and eventually into vmlinux, causing
> the build to be less reproducible when building
> on a different machine.
> 
> This patch makes the handling of
> UNUSED_KSYMS_WHITELIST to be similar to
> MODULE_SIG_KEY.
> 
> First, check if UNUSED_KSYMS_WHITELIST is an
> absolute path, just as before this patch. If so,
> use the path as is.
> 
> If it is a relative path, use wildcard to check
> the existence of the file below objtree first.
> If it does not exist, fall back to the original
> behavior of adding $(srctree)/ before the value.
> 
> After this patch, the developer can put the generated
> file in objtree, then use a relative path against
> objtree in .config, eradicating any absolute paths
> that may be evaluated differently on different machines.
> 
> Signed-off-by: Yifan Hong <elsk@google.com>

Applied and pushed, thanks.

  Luis
diff mbox series

Patch

diff --git a/kernel/module/Kconfig b/kernel/module/Kconfig
index f3e0329337f6..cb8377a18927 100644
--- a/kernel/module/Kconfig
+++ b/kernel/module/Kconfig
@@ -392,7 +392,7 @@  config UNUSED_KSYMS_WHITELIST
 	  exported at all times, even in absence of in-tree users. The value to
 	  set here is the path to a text file containing the list of symbols,
 	  one per line. The path can be absolute, or relative to the kernel
-	  source tree.
+	  source or obj tree.
 
 config MODULES_TREE_LOOKUP
 	def_bool y
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index 739402f45509..36952638bbc6 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -94,7 +94,7 @@  targets += .vmlinux.objs
 
 ifdef CONFIG_TRIM_UNUSED_KSYMS
 ksym-wl := $(CONFIG_UNUSED_KSYMS_WHITELIST)
-ksym-wl := $(if $(filter-out /%, $(ksym-wl)),$(srctree)/)$(ksym-wl)
+ksym-wl := $(if $(filter-out /%, $(ksym-wl)),$(if $(wildcard $(ksym-wl)),,$(srctree)/))$(ksym-wl)
 modpost-args += -t $(addprefix -u , $(ksym-wl))
 modpost-deps += $(ksym-wl)
 endif