Re: [PATCH v10 05/11] modpost: Track module name for built-in modules

From: Luis Chamberlain <mcgrof@kernel.org>
To: Allen Webb <allenwebb@google.com>
Cc: "linux-modules@vger.kernel.org" <linux-modules@vger.kernel.org>,
	"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	gregkh@linuxfoundation.org, christophe.leroy@csgroup.eu,
	nick.alcock@oracle.com
Subject: Re: [PATCH v10 05/11] modpost: Track module name for built-in modules
Date: Tue, 23 May 2023 23:50:40 -0700	[thread overview]
Message-ID: <ZG2zwMTdJryhGrbU@bombadil.infradead.org> (raw)
In-Reply-To: <20230406190030.968972-6-allenwebb@google.com>

On Thu, Apr 06, 2023 at 02:00:24PM -0500, Allen Webb wrote:
> Keep track of the module name when processing match table symbols.

This should mention why this would be good. Otherwise, think about it,
ok, it's done but why? If the reason is that it will be needed later
you need to say that in this commit log entry. If its not used now, it
also needs to say that in this commit log so it is easier to review
and set expecataions correctly for the reviewer.

  Luis

> Signed-off-by: Allen Webb <allenwebb@google.com>
> ---
>  scripts/mod/file2alias.c | 39 +++++++++++++++++++++++++++++++++++----
>  scripts/mod/modpost.h    |  1 +
>  2 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index 91c2e7ba5e52..b392d51c3b06 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -28,6 +28,7 @@ typedef Elf64_Addr	kernel_ulong_t;
>  #include <stdint.h>
>  #endif
>  
> +#include <assert.h>
>  #include <ctype.h>
>  #include <stdbool.h>
>  
> @@ -1540,9 +1541,9 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
>  			Elf_Sym *sym, const char *symname)
>  {
>  	void *symval;
> -	char *zeros = NULL;
> -	const char *name, *identifier;
> -	unsigned int namelen;
> +	char *zeros = NULL, *modname_str = NULL;
> +	const char *name, *identifier, *modname;
> +	unsigned int namelen, modnamelen;
>  
>  	/* We're looking for a section relative symbol */
>  	if (!sym->st_shndx || get_secindex(info, sym) >= info->num_sections)
> @@ -1552,7 +1553,12 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
>  	if (ELF_ST_TYPE(sym->st_info) != STT_OBJECT)
>  		return;
>  
> -	/* All our symbols are of form __mod_<name>__<identifier>_device_table. */
> +	/*
> +	 * All our symbols are either of form
> +	 *   __mod_<name>__<identifier>_device_table
> +	 * or
> +	 *   __mod_<name>__<identifier>__kmod_<builtin-name>_device_table
> +	 */
>  	if (strncmp(symname, "__mod_", strlen("__mod_")))
>  		return;
>  	name = symname + strlen("__mod_");
> @@ -1564,8 +1570,30 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
>  	identifier = strstr(name, "__");
>  	if (!identifier)
>  		return;
> +	modnamelen = namelen;
>  	namelen = identifier - name;
>  
> +	/*
> +	 * In the vmlinuz.o case we want to handle __kmod_ so aliases from
> +	 * builtin modules are attributed correctly.
> +	 */
> +	modname = strstr(identifier + 2, "__kmod_");
> +	if (modname) {
> +		modname += strlen("__kmod_");
> +		modnamelen -= (modname - name) + strlen("_device_table");
> +		modname_str = malloc(modnamelen + 1);
> +		/* We don't want to continue if the allocation fails. */
> +		assert(modname_str);
> +		memcpy(modname_str, modname, modnamelen);
> +		modname_str[modnamelen] = '\0';
> +	}
> +
> +	if (modname_str)
> +		modname = modname_str;
> +	else
> +		modname = mod->name;
> +	mod->builtin_name = modname;
> +
>  	/* Handle all-NULL symbols allocated into .bss */
>  	if (info->sechdrs[get_secindex(info, sym)].sh_type & SHT_NOBITS) {
>  		zeros = calloc(1, sym->st_size);
> @@ -1597,6 +1625,9 @@ void handle_moddevtable(struct module *mod, struct elf_info *info,
>  		}
>  	}
>  	free(zeros);
> +	mod->builtin_name = NULL;
> +	if (modname_str)
> +		free(modname_str);
>  }
>  
>  /* Now add out buffered information to the generated C source */
> diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
> index 1178f40a73f3..34fe5fc0b02c 100644
> --- a/scripts/mod/modpost.h
> +++ b/scripts/mod/modpost.h
> @@ -128,6 +128,7 @@ struct module {
>  	struct list_head missing_namespaces;
>  	// Actual imported namespaces
>  	struct list_head imported_namespaces;
> +	const char *builtin_name;
>  	char name[];
>  };
>  
> -- 
> 2.39.2
> 