Re: [PATCH 0/1] Fix exported symbol klp-relocation bug

[Miroslav Benes <mbenes@suse.cz>]

On Fri, 24 Jul 2026, Josh Poimboeuf wrote:

> On Thu, Jul 23, 2026 at 11:25:25AM +0200, Miroslav Benes wrote:
> > Hi,
> > 
> > > Next: symbol namespaces
> > > =======================
> > > 
> > > Now a harder question, I think, about symbol namespaces.  In the past,
> > > kpatch-build had supported patching symbols in namespace where
> > > MODULE_IMPORT_NS() is allowed.  Looking at kvm :: mmu.c
> > > ::kvm_flush_remote_tlbs(), that is annotated with
> > > EXPORT_SYMBOL_FOR_KVM_INTERNAL() instead.
> > > 
> > > Should we make an effort to support klp-relocations / patching to this
> > > use-case?
> > > 
> > > If modpost were to let klp-relocation symbols through, I *think*
> > > (untested) that might be enough... but it seems like that may violate
> > > the spirit of what the namespacing effort is trying to achieve.
> > > 
> > > Note that klp-post-link converts these symbols to SHN_LIVEPATCH before
> > > the module is loaded, so the kernel module loader already skips them in
> > > simplify_symbols() (see SHN_LIVEPATCH case).  AFAICT, the namespace
> > > check in modpost is the only enforcement point, and it's checking a
> > > symbol that will never be resolved through the normal module loading
> > > path anyway.
> > 
> > I think we will see more and more EXPORT_SYMBOL_FOR_MODULES() in the 
> > kernel. kvm is probably by far the most interesting one for us as of now.
> > 
> > Tough. All changes in upstream so far, as I remember, around reducing the 
> > possibility to use internal symbols for OOT modules (like kallsyms API but 
> > there are probably more) have been done with KLP usage in mind. Or at 
> > least people tried. However, there is a limit to it. If we introduce a 
> > workaround in upstream for this, people will definitely use it to get 
> > around the enforcement in their OOT modules. We should avoid that in my 
> > opinion.
> > 
> > So I would keep whatever we come up with in downstream.
> 
> klp relocs are already designed to enable accessing those.
> 
> The symbols being warned about by modpost are basically placeholder
> tombstone symbols which are only needed so as not to confuse objtool
> when it runs on the patch module.  They otherwise have no functional
> runtime purpose, as they are replaced by klp symbols/relocs.  So it's
> probably fine to just rename them to avoid triggering modpost:

Ok.
 
> diff --git a/tools/objtool/include/objtool/klp.h b/tools/objtool/include/objtool/klp.h
> index 6f60cf05db86..aab6db42052d 100644
> --- a/tools/objtool/include/objtool/klp.h
> +++ b/tools/objtool/include/objtool/klp.h
> @@ -23,6 +23,8 @@
>  #define KLP_RELOCS_SEC	"__klp_relocs"
>  #define KLP_STRINGS_SEC	".rodata.klp.str1.1"
>  
> +#define KLP_TOMBSTONE_PREFIX	".klp.tombstone."
> +
>  struct klp_reloc {
>  	void *offset;
>  	void *sym;
> diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
> index f7a02c4a2429..505553f9bdc1 100644
> --- a/tools/objtool/klp-diff.c
> +++ b/tools/objtool/klp-diff.c
> @@ -1359,6 +1359,7 @@ static int clone_reloc_klp(struct elfs *e, struct reloc *patched_reloc,
>  	s64 addend = reloc_addend(patched_reloc);
>  	const char *sym_modname, *sym_orig_name;
>  	static struct section *klp_relocs;
> +	char tombstone_name[SYM_NAME_LEN];
>  	struct symbol *sym, *klp_sym;
>  	unsigned long klp_reloc_off;
>  	char sym_name[SYM_NAME_LEN];
> @@ -1373,15 +1374,21 @@ static int clone_reloc_klp(struct elfs *e, struct reloc *patched_reloc,
>  	/*
>  	 * Keep the original reloc intact for now to avoid breaking objtool run
>  	 * which relies on proper relocations for many of its features.  This
> -	 * will be disabled later by "objtool klp post-link".
> +	 * reloc now targets a functionally dead tombstone symbol and will be
> +	 * disabled later by "objtool klp post-link".
>  	 *
> -	 * Convert it to UNDEF (and WEAK to avoid modpost warnings).
> +	 * To avoid modpost warnings, convert the symbol to UNDEF/WEAK and
> +	 * rename to .klp.tombstone.sym_name to prevent modpost from creating a
> +	 * false module dependency or warning about module namespaces.
>  	 */
>  
>  	sym = patched_sym->clone;
>  	if (!sym) {
> -		/* STB_WEAK: avoid modpost undefined symbol warnings */
> -		sym = elf_create_symbol(e->out, patched_sym->name, NULL,
> +		if (snprintf_check(tombstone_name, SYM_NAME_LEN,
> +				   KLP_TOMBSTONE_PREFIX "%s", patched_sym->name))
> +			return -1;
> +
> +		sym = elf_create_symbol(e->out, tombstone_name, NULL,
>  					STB_WEAK, patched_sym->type, 0, 0);
>  		if (!sym)
>  			return -1;

Yup, that looks like an improvement. Thank you.

Miroslav