diff mbox series

[v9,02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search

Message ID 20211223002209.1092165-3-alexandr.lobakin@intel.com (mailing list archive)
State Changes Requested
Headers show
Series Function Granular KASLR | expand

Commit Message

Alexander Lobakin Dec. 23, 2021, 12:21 a.m. UTC
Position-based search, which means that if we have several symbols
with the same name, we additionally need to provide an "index" of
the desired symbol, is fragile. Par exemple, it breaks when two
symbols with the same name are located in different sections.

Since a while, LD has a flag `-z unique-symbol` which appends
numeric suffixes to the functions with the same name (in symtab
and strtab).
Check for its availability and always prefer when the livepatching
is on. This needs a little adjustment to the modpost to make it
strip suffixes before adding exports.

depmod needs some treatment as well, tho its false-positibe warnings
about unknown symbols are harmless and don't alter the return code.
And there is a bunch more livepatch code to optimize-out after
introducing this, but let's leave it for later.

Suggested-by: H.J. Lu <hjl.tools@gmail.com>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Suggested-by: Josh Poimboeuf <jpoimboe@redhat.com>
Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com>
---
 Makefile                |  6 ++++++
 init/Kconfig            |  3 +++
 kernel/livepatch/core.c | 20 +++++++++++++-------
 scripts/mod/modpost.c   | 42 ++++++++++++++++++++++-------------------
 4 files changed, 45 insertions(+), 26 deletions(-)

Comments

Borislav Petkov Dec. 30, 2021, 11:10 a.m. UTC | #1
On Thu, Dec 23, 2021 at 01:21:56AM +0100, Alexander Lobakin wrote:
> [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search

nuke?

I think you wanna say something about avoiding position-based search if
toolchain supports -z ...

> Position-based search, which means that if we have several symbols
> with the same name, we additionally need to provide an "index" of
> the desired symbol, is fragile. Par exemple, it breaks when two
				  ^^^^^^^^^^^^

We already have hard time with the English in commit messages, let's
avoid the French pls.

> symbols with the same name are located in different sections.
> 
> Since a while, LD has a flag `-z unique-symbol` which appends
> numeric suffixes to the functions with the same name (in symtab
> and strtab).
> Check for its availability and always prefer when the livepatching
> is on.

Why only then?

It looks to me like we want this unconditionally, no?

> This needs a little adjustment to the modpost to make it
> strip suffixes before adding exports.
> 
> depmod needs some treatment as well, tho its false-positibe warnings

Unknown word [false-positibe] in commit message, suggestions:
        ['false-positive', 'false-positioned', 'prepositional']

Please introduce a spellchecker into your patch creation workflow.

> about unknown symbols are harmless and don't alter the return code.
> And there is a bunch more livepatch code to optimize-out after
> introducing this, but let's leave it for later.

...

> @@ -171,17 +173,21 @@ static int klp_find_object_symbol(const char *objname, const char *name,
>  
>  	/*
>  	 * Ensure an address was found. If sympos is 0, ensure symbol is unique;
> -	 * otherwise ensure the symbol position count matches sympos.
> +	 * otherwise ensure the symbol position count matches sympos. If the LD
> +	 * `-z unique` flag is enabled, sympos checks are not relevant.
	   ^^^^^^^^^^^

-z unique-symbol

>  	 */
> -	if (args.addr == 0)
> +	if (args.addr == 0) {
>  		pr_err("symbol '%s' not found in symbol table\n", name);
> -	else if (args.count > 1 && sympos == 0) {
> +	} else if (IS_ENABLED(CONFIG_LD_HAS_Z_UNIQUE_SYMBOL)) {
> +		goto out_ok;

This is silly - just do it all here.

> +	} else if (args.count > 1 && sympos == 0) {
>  		pr_err("unresolvable ambiguity for symbol '%s' in object '%s'\n",
>  		       name, objname);
>  	} else if (sympos != args.count && sympos > 0) {
>  		pr_err("symbol position %lu for symbol '%s' in object '%s' not found\n",
>  		       sympos, name, objname ? objname : "vmlinux");
>  	} else {
> +out_ok:
>  		*addr = args.addr;
>  		return 0;
>  	}

Looks straight-forward otherwise but I'm no livepatcher so I'd prefer if
they have a look too.
Fangrui Song Dec. 30, 2021, 6:31 p.m. UTC | #2
On Thu, Dec 30, 2021 at 3:11 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Dec 23, 2021 at 01:21:56AM +0100, Alexander Lobakin wrote:
> > [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search
>
> nuke?
>
> I think you wanna say something about avoiding position-based search if
> toolchain supports -z ...
>
> > Position-based search, which means that if we have several symbols
> > with the same name, we additionally need to provide an "index" of
> > the desired symbol, is fragile. Par exemple, it breaks when two
>                                   ^^^^^^^^^^^^
>
> We already have hard time with the English in commit messages, let's
> avoid the French pls.
>
> > symbols with the same name are located in different sections.
> >
> > Since a while, LD has a flag `-z unique-symbol` which appends
> > numeric suffixes to the functions with the same name (in symtab
> > and strtab).
> > Check for its availability and always prefer when the livepatching
> > is on.
>
> Why only then?
>
> It looks to me like we want this unconditionally, no?
>
> > This needs a little adjustment to the modpost to make it
> > strip suffixes before adding exports.
> >
> > depmod needs some treatment as well, tho its false-positibe warnings
>
> Unknown word [false-positibe] in commit message, suggestions:
>         ['false-positive', 'false-positioned', 'prepositional']
>
> Please introduce a spellchecker into your patch creation workflow.
>
> > about unknown symbols are harmless and don't alter the return code.
> > And there is a bunch more livepatch code to optimize-out after
> > introducing this, but let's leave it for later.
>
> ...
>
> > @@ -171,17 +173,21 @@ static int klp_find_object_symbol(const char *objname, const char *name,
> >
> >       /*
> >        * Ensure an address was found. If sympos is 0, ensure symbol is unique;
> > -      * otherwise ensure the symbol position count matches sympos.
> > +      * otherwise ensure the symbol position count matches sympos. If the LD
> > +      * `-z unique` flag is enabled, sympos checks are not relevant.
>            ^^^^^^^^^^^
>
> -z unique-symbol
>
> >        */
> > -     if (args.addr == 0)
> > +     if (args.addr == 0) {
> >               pr_err("symbol '%s' not found in symbol table\n", name);
> > -     else if (args.count > 1 && sympos == 0) {
> > +     } else if (IS_ENABLED(CONFIG_LD_HAS_Z_UNIQUE_SYMBOL)) {
> > +             goto out_ok;
>
> This is silly - just do it all here.
>
> > +     } else if (args.count > 1 && sympos == 0) {
> >               pr_err("unresolvable ambiguity for symbol '%s' in object '%s'\n",
> >                      name, objname);
> >       } else if (sympos != args.count && sympos > 0) {
> >               pr_err("symbol position %lu for symbol '%s' in object '%s' not found\n",
> >                      sympos, name, objname ? objname : "vmlinux");
> >       } else {
> > +out_ok:
> >               *addr = args.addr;
> >               return 0;
> >       }
>
> Looks straight-forward otherwise but I'm no livepatcher so I'd prefer if
> they have a look too.
>
> --
> Regards/Gruss,
>     Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
>

Apologies since I haven't read the patch series.

The option does not exist in ld.lld and I am a bit concerning about
its semantics: https://maskray.me/blog/2020-11-15-explain-gnu-linker-options#z-unique-symbol

I thought that someone forwarded my comments (originally posted months
on a feature request ago) here but seems not.
(I am a ld.lld maintainer.)
Miroslav Benes Jan. 3, 2022, 1:44 p.m. UTC | #3
> --- a/kernel/livepatch/core.c
> +++ b/kernel/livepatch/core.c
> @@ -143,11 +143,13 @@ static int klp_find_callback(void *data, const char *name,
>  	args->count++;
>  
>  	/*
> -	 * Finish the search when the symbol is found for the desired position
> -	 * or the position is not defined for a non-unique symbol.
> +	 * Finish the search when unique symbol names are enabled
> +	 * or the symbol is found for the desired position or the
> +	 * position is not defined for a non-unique symbol.
>  	 */
> -	if ((args->pos && (args->count == args->pos)) ||
> -	    (!args->pos && (args->count > 1)))
> +	if (IS_ENABLED(CONFIG_LD_HAS_Z_UNIQUE_SYMBOL) ||
> +	    (args->pos && args->count == args->pos) ||
> +	    (!args->pos && args->count > 1))
>  		return 1;
>  
>  	return 0;
> @@ -171,17 +173,21 @@ static int klp_find_object_symbol(const char *objname, const char *name,
>  
>  	/*
>  	 * Ensure an address was found. If sympos is 0, ensure symbol is unique;
> -	 * otherwise ensure the symbol position count matches sympos.
> +	 * otherwise ensure the symbol position count matches sympos. If the LD
> +	 * `-z unique` flag is enabled, sympos checks are not relevant.
>  	 */
> -	if (args.addr == 0)
> +	if (args.addr == 0) {
>  		pr_err("symbol '%s' not found in symbol table\n", name);
> -	else if (args.count > 1 && sympos == 0) {
> +	} else if (IS_ENABLED(CONFIG_LD_HAS_Z_UNIQUE_SYMBOL)) {
> +		goto out_ok;
> +	} else if (args.count > 1 && sympos == 0) {
>  		pr_err("unresolvable ambiguity for symbol '%s' in object '%s'\n",
>  		       name, objname);
>  	} else if (sympos != args.count && sympos > 0) {
>  		pr_err("symbol position %lu for symbol '%s' in object '%s' not found\n",
>  		       sympos, name, objname ? objname : "vmlinux");
>  	} else {
> +out_ok:
>  		*addr = args.addr;
>  		return 0;
>  	}

I think it would be better to return to something like

if (IS_ENABLED(CONFIG_LD_HAS_Z_UNIQUE_SYMBOL))
        sympos = 0;

in klp_find_object_symbol() just after kallsyms search is performed.

You would not need the above changes at all. I did not like it before when 
sympos clearing was proposed, but the situation was different back then. 
'-z unique-symbol' was not available. It has changed and we have a 
guarantee that symbols are unique now. There would not be any impact on 
the user.

What do you think?

Miroslav
Miroslav Benes Jan. 3, 2022, 1:55 p.m. UTC | #4
On Thu, 30 Dec 2021, Fāng-ruì Sòng wrote:

> On Thu, Dec 30, 2021 at 3:11 AM Borislav Petkov <bp@alien8.de> wrote:
> >
> > On Thu, Dec 23, 2021 at 01:21:56AM +0100, Alexander Lobakin wrote:
> > > [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search

...

> Apologies since I haven't read the patch series.
> 
> The option does not exist in ld.lld and I am a bit concerning about
> its semantics: https://maskray.me/blog/2020-11-15-explain-gnu-linker-options#z-unique-symbol
> 
> I thought that someone forwarded my comments (originally posted months
> on a feature request ago) here but seems not.
> (I am a ld.lld maintainer.)

Do you mean 
https://lore.kernel.org/all/20210123225928.z5hkmaw6qjs2gu5g@google.com/T/#u 
?

Unfortunately, it did not lead anywhere. I think that '-z unique-symbol' 
option should work fine as long as the live patching is concerned. Maybe I 
misunderstood but your concerns mentioned at the blog do not apply. The 
stability is not an issue for us since we (KLP) always work with already 
built and fixed kernel. And(at least) GCC already uses number suffices for 
IPA clones and it has not been a problem anywhere.

Am I wrong?

Thanks

Miroslav
Alexander Lobakin Jan. 3, 2022, 4:06 p.m. UTC | #5
From: Miroslav Benes <mbenes@suse.cz>
Date: Mon, 3 Jan 2022 14:55:42 +0100 (CET)

> On Thu, 30 Dec 2021, Fāng-ruì Sòng wrote:
> 
> > On Thu, Dec 30, 2021 at 3:11 AM Borislav Petkov <bp@alien8.de> wrote:
> > >
> > > On Thu, Dec 23, 2021 at 01:21:56AM +0100, Alexander Lobakin wrote:
> > > > [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search
> 
> ...
> 
> > Apologies since I haven't read the patch series.
> > 
> > The option does not exist in ld.lld and I am a bit concerning about
> > its semantics: https://maskray.me/blog/2020-11-15-explain-gnu-linker-options#z-unique-symbol
> > 
> > I thought that someone forwarded my comments (originally posted months
> > on a feature request ago) here but seems not.
> > (I am a ld.lld maintainer.)
> 
> Do you mean 
> https://lore.kernel.org/all/20210123225928.z5hkmaw6qjs2gu5g@google.com/T/#u 
> ?
> 
> Unfortunately, it did not lead anywhere. I think that '-z unique-symbol' 
> option should work fine as long as the live patching is concerned. Maybe I 
> misunderstood but your concerns mentioned at the blog do not apply. The 
> stability is not an issue for us since we (KLP) always work with already 
> built and fixed kernel. And(at least) GCC already uses number suffices for 
> IPA clones and it has not been a problem anywhere.

LLD doesn't have such an option, so FG-KASLR + livepatching builds
wouldn't be available for LLVM with the current approach (or we'd
still need a stub that prints "FG-KASLR is not compatible with
sympos != 0").
Unfortunately, I discovered this a bit late, just after sending this
revision.

OTOH, there's no easy alternative. <file + function> pair looks
appealing, but is it even possible for now to implement in the
kernel without much refactoring?

>
> Am I wrong?
> 
> Thanks
> 
> Miroslav 

Thanks,
Al
Alexander Lobakin Jan. 3, 2022, 4:29 p.m. UTC | #6
From: Borislav Petkov <bp@alien8.de>
Date: Thu, 30 Dec 2021 12:10:33 +0100

> On Thu, Dec 23, 2021 at 01:21:56AM +0100, Alexander Lobakin wrote:
> > [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search
> 
> nuke?
> 
> I think you wanna say something about avoiding position-based search if
> toolchain supports -z ...

Correct. A "vocabulary fail" moment.

> 
> > Position-based search, which means that if we have several symbols
> > with the same name, we additionally need to provide an "index" of
> > the desired symbol, is fragile. Par exemple, it breaks when two
> 				  ^^^^^^^^^^^^
> 
> We already have hard time with the English in commit messages, let's
> avoid the French pls.
> 
> > symbols with the same name are located in different sections.
> > 
> > Since a while, LD has a flag `-z unique-symbol` which appends
> > numeric suffixes to the functions with the same name (in symtab
> > and strtab).
> > Check for its availability and always prefer when the livepatching
> > is on.
> 
> Why only then?
> 
> It looks to me like we want this unconditionally, no?

To be as least invasive as possible for now. We can turn it on
unconditionally after a while. LLD doesn't support it and this
and there are some different opinions about unique-symbol in
general.
Maybe FG-KASLR builds will reveal that some of the concerns are
true, who knows. It wouldn't need to get turned off back again
then.

> 
> > This needs a little adjustment to the modpost to make it
> > strip suffixes before adding exports.
> > 
> > depmod needs some treatment as well, tho its false-positibe warnings
> 
> Unknown word [false-positibe] in commit message, suggestions:
>         ['false-positive', 'false-positioned', 'prepositional']
> 
> Please introduce a spellchecker into your patch creation workflow.

It's here, but refused to work this time or so <O> I have definitely
run checkpatch with codespell against the series I can't recall any
reported typos.

> 
> > about unknown symbols are harmless and don't alter the return code.
> > And there is a bunch more livepatch code to optimize-out after
> > introducing this, but let's leave it for later.
> 
> ...
> 
> > @@ -171,17 +173,21 @@ static int klp_find_object_symbol(const char *objname, const char *name,
> >  
> >  	/*
> >  	 * Ensure an address was found. If sympos is 0, ensure symbol is unique;
> > -	 * otherwise ensure the symbol position count matches sympos.
> > +	 * otherwise ensure the symbol position count matches sympos. If the LD
> > +	 * `-z unique` flag is enabled, sympos checks are not relevant.
> 	   ^^^^^^^^^^^
> 
> -z unique-symbol
> 
> >  	 */
> > -	if (args.addr == 0)
> > +	if (args.addr == 0) {
> >  		pr_err("symbol '%s' not found in symbol table\n", name);
> > -	else if (args.count > 1 && sympos == 0) {
> > +	} else if (IS_ENABLED(CONFIG_LD_HAS_Z_UNIQUE_SYMBOL)) {
> > +		goto out_ok;
> 
> This is silly - just do it all here.

Yeah, a "big brain" moment from me. Or even reset sympos to 0 when
unique-symbol is enabled, like Mirek suggests.

> 
> > +	} else if (args.count > 1 && sympos == 0) {
> >  		pr_err("unresolvable ambiguity for symbol '%s' in object '%s'\n",
> >  		       name, objname);
> >  	} else if (sympos != args.count && sympos > 0) {
> >  		pr_err("symbol position %lu for symbol '%s' in object '%s' not found\n",
> >  		       sympos, name, objname ? objname : "vmlinux");
> >  	} else {
> > +out_ok:
> >  		*addr = args.addr;
> >  		return 0;
> >  	}
> 
> Looks straight-forward otherwise but I'm no livepatcher so I'd prefer if
> they have a look too.
> 
> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

Thanks,
Al
Fangrui Song Jan. 5, 2022, 3:24 a.m. UTC | #7
On 2022-01-03, Alexander Lobakin wrote:
>From: Miroslav Benes <mbenes@suse.cz>
>Date: Mon, 3 Jan 2022 14:55:42 +0100 (CET)
>
>> On Thu, 30 Dec 2021, Fāng-ruì Sòng wrote:
>>
>> > On Thu, Dec 30, 2021 at 3:11 AM Borislav Petkov <bp@alien8.de> wrote:
>> > >
>> > > On Thu, Dec 23, 2021 at 01:21:56AM +0100, Alexander Lobakin wrote:
>> > > > [PATCH v9 02/15] livepatch: use `-z unique-symbol` if available to nuke pos-based search
>>
>> ...
>>
>> > Apologies since I haven't read the patch series.
>> >
>> > The option does not exist in ld.lld and I am a bit concerning about
>> > its semantics: https://maskray.me/blog/2020-11-15-explain-gnu-linker-options#z-unique-symbol
>> >
>> > I thought that someone forwarded my comments (originally posted months
>> > on a feature request ago) here but seems not.
>> > (I am a ld.lld maintainer.)
>>
>> Do you mean
>> https://lore.kernel.org/all/20210123225928.z5hkmaw6qjs2gu5g@google.com/T/#u
>> ?
>>
>> Unfortunately, it did not lead anywhere. I think that '-z unique-symbol'
>> option should work fine as long as the live patching is concerned. Maybe I
>> misunderstood but your concerns mentioned at the blog do not apply. The
>> stability is not an issue for us since we (KLP) always work with already
>> built and fixed kernel. And(at least) GCC already uses number suffices for
>> IPA clones and it has not been a problem anywhere.

The stability problem may not happen frequently but is possible if the
compiler performs some IPA with new code.

Such disturbence is probably more likely with LTO or PGO.
For Clang LTO, Makefile currently specifies -mllvm -import-instr-limit=5.
If a function close to the boundary happens to cross the boundary,
if inlined into other translation units, the stability issue may affect
many translation units.

>LLD doesn't have such an option, so FG-KASLR + livepatching builds
>wouldn't be available for LLVM with the current approach (or we'd
>still need a stub that prints "FG-KASLR is not compatible with
>sympos != 0").
>Unfortunately, I discovered this a bit late, just after sending this
>revision.
>
>OTOH, there's no easy alternative. <file + function> pair looks
>appealing, but is it even possible for now to implement in the
>kernel without much refactoring?

<file + symbol> pair looks good to me and will solve the stability problem.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index d85f1ff79f5c..9dc15c67d132 100644
--- a/Makefile
+++ b/Makefile
@@ -882,6 +882,12 @@  ifdef CONFIG_DEBUG_SECTION_MISMATCH
 KBUILD_CFLAGS += -fno-inline-functions-called-once
 endif
 
+# Prefer linking with the `-z unique-symbol` if available, this eliminates
+# position-based search
+ifeq ($(CONFIG_LD_HAS_Z_UNIQUE_SYMBOL)$(CONFIG_LIVEPATCH),yy)
+KBUILD_LDFLAGS += -z unique-symbol
+endif
+
 ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
 KBUILD_CFLAGS_KERNEL += -ffunction-sections -fdata-sections
 LDFLAGS_vmlinux += --gc-sections
diff --git a/init/Kconfig b/init/Kconfig
index 4b7bac10c72d..37926d19a74a 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -86,6 +86,9 @@  config CC_HAS_ASM_INLINE
 config CC_HAS_NO_PROFILE_FN_ATTR
 	def_bool $(success,echo '__attribute__((no_profile_instrument_function)) int x();' | $(CC) -x c - -c -o /dev/null -Werror)
 
+config LD_HAS_Z_UNIQUE_SYMBOL
+	def_bool $(ld-option,-z unique-symbol)
+
 config CONSTRUCTORS
 	bool
 
diff --git a/kernel/livepatch/core.c b/kernel/livepatch/core.c
index 335d988bd811..b2c787297f85 100644
--- a/kernel/livepatch/core.c
+++ b/kernel/livepatch/core.c
@@ -143,11 +143,13 @@  static int klp_find_callback(void *data, const char *name,
 	args->count++;
 
 	/*
-	 * Finish the search when the symbol is found for the desired position
-	 * or the position is not defined for a non-unique symbol.
+	 * Finish the search when unique symbol names are enabled
+	 * or the symbol is found for the desired position or the
+	 * position is not defined for a non-unique symbol.
 	 */
-	if ((args->pos && (args->count == args->pos)) ||
-	    (!args->pos && (args->count > 1)))
+	if (IS_ENABLED(CONFIG_LD_HAS_Z_UNIQUE_SYMBOL) ||
+	    (args->pos && args->count == args->pos) ||
+	    (!args->pos && args->count > 1))
 		return 1;
 
 	return 0;
@@ -171,17 +173,21 @@  static int klp_find_object_symbol(const char *objname, const char *name,
 
 	/*
 	 * Ensure an address was found. If sympos is 0, ensure symbol is unique;
-	 * otherwise ensure the symbol position count matches sympos.
+	 * otherwise ensure the symbol position count matches sympos. If the LD
+	 * `-z unique` flag is enabled, sympos checks are not relevant.
 	 */
-	if (args.addr == 0)
+	if (args.addr == 0) {
 		pr_err("symbol '%s' not found in symbol table\n", name);
-	else if (args.count > 1 && sympos == 0) {
+	} else if (IS_ENABLED(CONFIG_LD_HAS_Z_UNIQUE_SYMBOL)) {
+		goto out_ok;
+	} else if (args.count > 1 && sympos == 0) {
 		pr_err("unresolvable ambiguity for symbol '%s' in object '%s'\n",
 		       name, objname);
 	} else if (sympos != args.count && sympos > 0) {
 		pr_err("symbol position %lu for symbol '%s' in object '%s' not found\n",
 		       sympos, name, objname ? objname : "vmlinux");
 	} else {
+out_ok:
 		*addr = args.addr;
 		return 0;
 	}
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index ccc6d35580f2..f39cc73a082c 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -689,11 +689,28 @@  static void handle_modversion(const struct module *mod,
 	sym_set_crc(symname, crc);
 }
 
+static char *remove_dot(char *s)
+{
+	size_t n = strcspn(s, ".");
+
+	if (n && s[n]) {
+		size_t m = strspn(s + n + 1, "0123456789");
+
+		if (m && (s[n + m + 1] == '.' || s[n + m + 1] == 0))
+			s[n] = 0;
+
+		/* strip trailing .lto */
+		if (strends(s, ".lto"))
+			s[strlen(s) - 4] = '\0';
+	}
+
+	return s;
+}
+
 static void handle_symbol(struct module *mod, struct elf_info *info,
 			  const Elf_Sym *sym, const char *symname)
 {
 	enum export export;
-	const char *name;
 
 	if (strstarts(symname, "__ksymtab"))
 		export = export_from_secname(info, get_secindex(info, sym));
@@ -734,8 +751,11 @@  static void handle_symbol(struct module *mod, struct elf_info *info,
 	default:
 		/* All exported symbols */
 		if (strstarts(symname, "__ksymtab_")) {
-			name = symname + strlen("__ksymtab_");
-			sym_add_exported(name, mod, export);
+			char *name;
+
+			name = NOFAIL(strdup(symname + strlen("__ksymtab_")));
+			sym_add_exported(remove_dot(name), mod, export);
+			free(name);
 		}
 		if (strcmp(symname, "init_module") == 0)
 			mod->has_init = 1;
@@ -1965,22 +1985,6 @@  static void check_sec_ref(struct module *mod, const char *modname,
 	}
 }
 
-static char *remove_dot(char *s)
-{
-	size_t n = strcspn(s, ".");
-
-	if (n && s[n]) {
-		size_t m = strspn(s + n + 1, "0123456789");
-		if (m && (s[n + m + 1] == '.' || s[n + m + 1] == 0))
-			s[n] = 0;
-
-		/* strip trailing .lto */
-		if (strends(s, ".lto"))
-			s[strlen(s) - 4] = '\0';
-	}
-	return s;
-}
-
 static void read_symbols(const char *modname)
 {
 	const char *symname;