[PATCH v9 01/15] modpost: fix removing numeric suffixes

[PATCH v9 01/15] modpost: fix removing numeric suffixes

[Alexander Lobakin]

For now, that condition from remove_dot():

if (m && (s[n + m] == '.' || s[n + m] == 0))

which was designed to test if it's a dot or a \0 after the suffix
is never satisfied.
This is due to that s[n + m] always points to the last digit of a
numeric suffix, not on the symbol next to it:

param_set_uint.0, s[n + m] is '0', s[n + m + 1] is '\0'

So it's off by one and was like that since 2014.

`-z uniq-symbol` linker flag which we are planning to use to
simplify livepatching brings numeric suffixes back, fix this.
Otherwise:

ERROR: modpost: "param_set_uint.0" [vmlinux] is a static EXPORT_SYMBOL

Fixes: fcd38ed0ff26 ("scripts: modpost: fix compilation warning")
Cc: stable@vger.kernel.org # 3.17+
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
---
 scripts/mod/modpost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index cb8ab7d91d30..ccc6d35580f2 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1971,7 +1971,7 @@ static char *remove_dot(char *s)
 
 	if (n && s[n]) {
 		size_t m = strspn(s + n + 1, "0123456789");
-		if (m && (s[n + m] == '.' || s[n + m] == 0))
+		if (m && (s[n + m + 1] == '.' || s[n + m + 1] == 0))
 			s[n] = 0;
 
 		/* strip trailing .lto */
-- 
2.33.1

Re: [PATCH v9 01/15] modpost: fix removing numeric suffixes

[Borislav Petkov]

On Thu, Dec 23, 2021 at 01:21:55AM +0100, Alexander Lobakin wrote:
> For now, that condition from remove_dot():
> 
> if (m && (s[n + m] == '.' || s[n + m] == 0))
> 
> which was designed to test if it's a dot or a \0 after the suffix
> is never satisfied.
> This is due to that s[n + m] always points to the last digit of a
> numeric suffix, not on the symbol next to it:
> 
> param_set_uint.0, s[n + m] is '0', s[n + m + 1] is '\0'
> 
> So it's off by one and was like that since 2014.

What's the relevance of this? Looking at

  7d02b490e93c ("Kbuild, lto: Drop .number postfixes in modpost")

what you're fixing here is something LTO-related. How do you trigger
this?

For a Cc:stable patch, I'm missing a lot of context.

> `-z uniq-symbol` linker flag which we are planning to use to
				     ^^

Who's "we"?

> simplify livepatching brings numeric suffixes back, fix this.
> Otherwise:
> 
> ERROR: modpost: "param_set_uint.0" [vmlinux] is a static EXPORT_SYMBOL
> 
> Fixes: fcd38ed0ff26 ("scripts: modpost: fix compilation warning")
> Cc: stable@vger.kernel.org # 3.17+
> Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>

...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v9 01/15] modpost: fix removing numeric suffixes

[Alexander Lobakin]

From: Borislav Petkov <bp@alien8.de>
Date: Thu, 23 Dec 2021 17:19:06 +0100

> On Thu, Dec 23, 2021 at 01:21:55AM +0100, Alexander Lobakin wrote:
> > For now, that condition from remove_dot():
> > 
> > if (m && (s[n + m] == '.' || s[n + m] == 0))
> > 
> > which was designed to test if it's a dot or a \0 after the suffix
> > is never satisfied.
> > This is due to that s[n + m] always points to the last digit of a
> > numeric suffix, not on the symbol next to it:
> > 
> > param_set_uint.0, s[n + m] is '0', s[n + m + 1] is '\0'
> > 
> > So it's off by one and was like that since 2014.
> 
> What's the relevance of this? Looking at
> 
>   7d02b490e93c ("Kbuild, lto: Drop .number postfixes in modpost")
> 
> what you're fixing here is something LTO-related. How do you trigger
> this?

It's just a couple lines below. I trigger this using `-z uniq-symbol`
which uses numeric suffixes for globals as well.

> 
> For a Cc:stable patch, I'm missing a lot of context.

It fixes a commit dated 2014, thus Cc:stable. Although the
remove_dot() might've been introduced for neverlanded GCC LTO, but
in fact numeric suffixes are used a lot by the toolchains in regular
builds as well. Just not for globals, that's why it's "well hidden".

> 
> > `-z uniq-symbol` linker flag which we are planning to use to
> 				     ^^
> 
> Who's "we"?

I thought it's a common saying in commit messages, isn't it?

> 
> > simplify livepatching brings numeric suffixes back, fix this.
> > Otherwise:
> > 
> > ERROR: modpost: "param_set_uint.0" [vmlinux] is a static EXPORT_SYMBOL
> > 
> > Fixes: fcd38ed0ff26 ("scripts: modpost: fix compilation warning")
> > Cc: stable@vger.kernel.org # 3.17+
> > Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
> 
> ...
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

Thanks,
Al

Re: [PATCH v9 01/15] modpost: fix removing numeric suffixes

[Borislav Petkov]

On Mon, Dec 27, 2021 at 07:22:46PM +0100, Alexander Lobakin wrote:
> It's just a couple lines below. I trigger this using `-z uniq-symbol`
> which uses numeric suffixes for globals as well.

Aha, so that's for the fgkaslr purposes now.

> It fixes a commit dated 2014, thus Cc:stable. Although the
> remove_dot() might've been introduced for neverlanded GCC LTO, but
> in fact numeric suffixes are used a lot by the toolchains in regular
> builds as well. Just not for globals, that's why it's "well hidden".

Does "well hidden" warrant a stable backport then? Because if no
toolchain is using numeric suffixes for globals, then no need for the
stable tag, I'd say.

> I thought it's a common saying in commit messages, isn't it?

Lemme give you my canned and a lot more eloquent explanation for that:

"Please use passive voice in your commit message: no "we" or "I", etc,
and describe your changes in imperative mood.

Also, pls read section "2) Describe your changes" in
Documentation/process/submitting-patches.rst for more details.

Also, see section "Changelog" in
Documentation/process/maintainer-tip.rst

Bottom line is: personal pronouns are ambiguous in text, especially with
so many parties/companies/etc developing the kernel so let's avoid them
please."

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v9 01/15] modpost: fix removing numeric suffixes

[Alexander Lobakin]

From: Borislav Petkov <bp@alien8.de>
Date: Mon, 27 Dec 2021 22:26:02 +0100

> On Mon, Dec 27, 2021 at 07:22:46PM +0100, Alexander Lobakin wrote:
> > It's just a couple lines below. I trigger this using `-z uniq-symbol`
> > which uses numeric suffixes for globals as well.
> 
> Aha, so that's for the fgkaslr purposes now.

Well, linking using unique names is meant to be used always
when available and livepatching is enabled, at least I hope so.

> 
> > It fixes a commit dated 2014, thus Cc:stable. Although the
> > remove_dot() might've been introduced for neverlanded GCC LTO, but
> > in fact numeric suffixes are used a lot by the toolchains in regular
> > builds as well. Just not for globals, that's why it's "well hidden".
> 
> Does "well hidden" warrant a stable backport then? Because if no
> toolchain is using numeric suffixes for globals, then no need for the
> stable tag, I'd say.

Hmm, makes sense. The fact that I haven't seen any similar reports
or issues (even on LTO builds) sorta says there are no benefits from
backporting this.
Ok, I'll drop the tag. It's never too late anyway to port this in
case someone will face it.

> 
> > I thought it's a common saying in commit messages, isn't it?
> 
> Lemme give you my canned and a lot more eloquent explanation for that:
> 
> "Please use passive voice in your commit message: no "we" or "I", etc,
> and describe your changes in imperative mood.
> 
> Also, pls read section "2) Describe your changes" in
> Documentation/process/submitting-patches.rst for more details.
> 
> Also, see section "Changelog" in
> Documentation/process/maintainer-tip.rst
> 
> Bottom line is: personal pronouns are ambiguous in text, especially with
> so many parties/companies/etc developing the kernel so let's avoid them
> please."
> 
> Thx.

Ah, you're right. "Common used" doesn't mean "correct". I'll fix it
in the next spin being published after accumulating a bunch more
comments.
Thanks!

> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

Al

Re: [PATCH v9 01/15] modpost: fix removing numeric suffixes

[Miroslav Benes]

On Thu, 23 Dec 2021, Alexander Lobakin wrote:

> For now, that condition from remove_dot():
> 
> if (m && (s[n + m] == '.' || s[n + m] == 0))
> 
> which was designed to test if it's a dot or a \0 after the suffix
> is never satisfied.
> This is due to that s[n + m] always points to the last digit of a
> numeric suffix, not on the symbol next to it:
> 
> param_set_uint.0, s[n + m] is '0', s[n + m + 1] is '\0'
> 
> So it's off by one and was like that since 2014.
> 
> `-z uniq-symbol` linker flag which we are planning to use to

`-z unique-symbol`

Miroslav