Re: [PATCH 02/14] objtool/klp: Normalize Module.symvers paths to module names

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] normalize_modname() returns an interior pointer, causing the original heap allocation base pointer to be lost in read_exports() and __find_modname().
--

--- Patch [2]: [PATCH 02/14] objtool/klp: Normalize Module.symvers paths to module names ---
commit 8f5aadae3b846aa3bb569cfc55ef783d4449690f
Author: Joe Lawrence <joe.lawrence@redhat.com>

objtool/klp: Normalize Module.symvers paths to module names

This commit normalizes Module.symvers object paths to runtime module names
for exported symbols to prevent a crash during livepatch loading. It extracts
the module name processing into a shared normalize_modname() helper.

> diff --git a/tools/objtool/klp-diff.c b/tools/objtool/klp-diff.c
> index aeb99d572300c..15d37d955af0e 100644
> --- a/tools/objtool/klp-diff.c
> +++ b/tools/objtool/klp-diff.c
> @@ -83,6 +83,35 @@ static char *escape_str(const char *orig)
[ ... ]
> +static char *normalize_modname(char *name)
> +{
> +	char *slash = strrchr(name, '/');
> +
> +	if (slash)
> +		name = slash + 1;
> +
> +	for (char *c = name; *c; c++) {
> +		if (*c == '-')
> +			*c = '_';
> +		else if (*c == '.') {
> +			*c = '\0';
> +			break;
> +		}
> +	}
> +	return name;
> +}
> +
>  static int read_exports(void)
>  {
[ ... ]
> @@ -150,6 +179,9 @@ static int read_exports(void)
>  			return -1;
>  		}
>  
> +		if (strcmp(export->mod, "vmlinux"))
> +			export->mod = normalize_modname(export->mod);
> +

[Severity: Medium]
Does this assignment leak the original pointer returned by strdup()?

Looking at read_exports(), export->mod is allocated via strdup() right
before this check:

        export->mod = strdup(mod);
        if (!export->mod) {
                ERROR_GLIBC("strdup");
                return -1;
        }

        if (strcmp(export->mod, "vmlinux"))
                export->mod = normalize_modname(export->mod);

When processing module paths with a slash, normalize_modname() returns
an interior pointer (slash + 1).

If this offset pointer overwrites export->mod, the original base
address from strdup() is permanently lost. 

While objtool often relies on process termination for cleanup, if these
strings are ever freed later, passing an interior pointer to free() will
cause an invalid free crash.

Could this be avoided by shifting the string contents in-place with
memmove() instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1785727106.git.jpoimboe@kernel.org?part=2