Re: [PATCH 1/2] module: expose imported namespaces via sysfs

[Sami Tolvanen <samitolvanen@google.com>]

On Wed, Jan 07, 2026 at 10:47:09PM -0600, Nicholas Sielicki wrote:
>  
> +static ssize_t show_modinfo_import_ns(const struct module_attribute *mattr,
> +				      struct module_kobject *mk, char *buffer)
> +{
> +	return sysfs_emit(buffer, "%s\n", mk->mod->imported_namespaces);
> +}
> +
> +static int modinfo_import_ns_exists(struct module *mod)
> +{
> +	return mod->imported_namespaces != NULL;
> +}
> +
> +static const struct module_attribute modinfo_import_ns = {
> +	.attr = { .name = "import_ns", .mode = 0444 },
> +	.show = show_modinfo_import_ns,
> +	.test = modinfo_import_ns_exists,
> +};
> +

Don't we need a .setup function that initializes mod->imported_namespaces
to NULL? Currently, if setup_modinfo returns an error, the pointer remains
initialized to whatever value we read from .gnu.linkonce.this_module, and
we'll pass that arbitrary pointer to kfree.

This isn't normally a problem since modpost zero-initializes the field, but
we don't want to rely on userspace to initialize our pointers.

Also, define .free to release the buffer instead of adding a direct call
to free_modinfo.

>  static struct {
>  	char name[MODULE_NAME_LEN];
>  	char taints[MODULE_FLAGS_BUF_SIZE];
> @@ -1058,6 +1075,7 @@ const struct module_attribute *const modinfo_attrs[] = {
>  	&module_uevent,
>  	&modinfo_version,
>  	&modinfo_srcversion,
> +	&modinfo_import_ns,
>  	&modinfo_initstate,
>  	&modinfo_coresize,
>  #ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
> @@ -1753,11 +1771,48 @@ static void module_license_taint_check(struct module *mod, const char *license)
>  	}
>  }
>  
> +static int copy_modinfo_import_ns(struct module *mod, struct load_info *info)
> +{
> +	char *ns;
> +	size_t len, total_len = 0;
> +	char *buf, *p;
> +
> +	for_each_modinfo_entry(ns, info, "import_ns")
> +		total_len += strlen(ns) + 1;
> +
> +	if (!total_len) {
> +		mod->imported_namespaces = NULL;
> +		return 0;
> +	}
> +
> +	buf = kmalloc(total_len, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;

For example, if kmalloc fails, mod->imported_namespaces isn't initialized.

> +
> +	p = buf;
> +	for_each_modinfo_entry(ns, info, "import_ns") {
> +		len = strlen(ns);
> +		memcpy(p, ns, len);
> +		p += len;
> +		*p++ = '\n';
> +	}
> +	/* Replace trailing newline with null terminator. */
> +	*(p - 1) = '\0';
> +
> +	mod->imported_namespaces = buf;
> +	return 0;
> +}
> +
> +static void free_modinfo_import_ns(struct module *mod)
> +{
> +	kfree(mod->imported_namespaces);

mod->imported_namespaces = NULL;

> +}
> +
>  static int setup_modinfo(struct module *mod, struct load_info *info)
>  {
>  	const struct module_attribute *attr;
>  	char *imported_namespace;
> -	int i;
> +	int i, err;
>  
>  	for (i = 0; (attr = modinfo_attrs[i]); i++) {
>  		if (attr->setup)
> @@ -1776,6 +1831,10 @@ static int setup_modinfo(struct module *mod, struct load_info *info)
>  		}
>  	}

Also setup_modinfo can fail before copy_modinfo_import_ns is even
called.

> +	err = copy_modinfo_import_ns(mod, info);
> +	if (err)
> +		return err;
> +

Sami