linux-kernel - Re: [PATCH v1] setlocalversion: use ${objtree}/include/config/auto.conf

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] [day] [month] [year] [list]

Message-ID: <CAABy=s18Sn-v78=_UWXnZ-o8Yx9ZnRdd+NqVBN_qKcZAu4dgDw@mail.gmail.com>
Date: Wed, 12 Mar 2025 16:02:50 -0700
From: "Hong, Yifan" <elsk@...gle.com>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: Rasmus Villemoes <linux@...musvillemoes.dk>, kernel-team@...roid.com, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] setlocalversion: use ${objtree}/include/config/auto.conf

On Wed, Mar 12, 2025 at 12:13 PM Hong, Yifan <elsk@...gle.com> wrote:
>
> On Wed, Mar 12, 2025 at 12:31 AM Masahiro Yamada <masahiroy@...nel.org> wrote:
> >
> > On Wed, Mar 12, 2025 at 11:12 AM HONG Yifan <elsk@...gle.com> wrote:
> > >
> > > setlocalversion reads include/config/auto.conf, which is located below
> > > $(objtree) with commit 214c0eea43b2 ("kbuild: add $(objtree)/ prefix to
> > > some in-kernel build artifacts").
> > >
> > > Hence, the setlocalversion script needs to use
> > > $(objtree)/include/config/auto.conf as well.
> > >
> > > Note that $(objtree) is not necessarily `.` when O (aka KBUILD_OUTPUT)
> > > is set, because of commit 13b25489b6f8 ("kbuild: change working
> > > directory to external module directory with M=").
> >
> > Is this a real issue?
> > If so, please attach some commands to reproduce an issue.
> >
> > setlocalversion is invoked only at line 1238 of the top-level Makefile,
> > within the check "ifeq ($(KBUILD_EXTMOD),)"
> > So, it is never called with external module builds.
>
> Thanks Masahiro. You are right; this is not a real issue as the code
> is right now.
>
> In our case, we have this issue because we have
> https://lore.kernel.org/all/20210121213641.3477522-1-willmcvicker@google.com/
> ("[PATCH v6] modules: introduce the MODULE_SCMVERSION config")
> in our tree to support the SCM version for modules. The patch was not
> accepted so
> we applied this patch locally. Hence, technically this patch should also only be
> applied locally by us. The paragraph that says "Note that $(objtree) is not
> necessarily `.`..." is not correct.
>
> Still, I think it makes sense to be consistent with other places that mentions
> include/config/auto.conf.
>
> Should I update the commit message and send another patch? Or would you
> like to reject this change?
>
> >
> >
> >
> >
> >
> >
> >
> > > Signed-off-by: HONG Yifan <elsk@...gle.com>
> > > ---
> > > Implementation note: Should I test -z ${objtree} before using it? Otherwise it
> > > looks at /include/config/auto.conf which is wrong.
> > > Or should I fall back to `.` if objtree is not found in the environment
> > > variables?
> > > I could also `exit 1` if $objtree is not set. Please let me know what you think.
> > >
> > >  scripts/setlocalversion | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/scripts/setlocalversion b/scripts/setlocalversion
> > > index 28169d7e143b..88f54eb5a7c2 100755
> > > --- a/scripts/setlocalversion
> > > +++ b/scripts/setlocalversion
> > > @@ -186,7 +186,7 @@ if ${no_local}; then
> > >         exit 0
> > >  fi
> > >
> > > -if ! test -e include/config/auto.conf; then
> > > +if ! test -e ${objtree}/include/config/auto.conf; then
> > >         echo "Error: kernelrelease not valid - run 'make prepare' to update it" >&2
> > >         exit 1
> > >  fi

Actually, I noticed two more include/config/auto.conf mentions below
this line. If the overall idea looks good to you, I'll upload a
separate version that fixes the two mentions below as well.

> > > --
> > > 2.49.0.rc0.332.g42c0ae87b1-goog
> > >
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada