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