[PATCH v2] module: print version for external modules in print_modules()

[PATCH v2] module: print version for external modules in print_modules()

[Yafang Shao]

We maintain a vmcore analysis script on each server that automatically
parses /var/crash/XXXX/vmcore-dmesg.txt to categorize vmcores. This helps
us save considerable effort by avoiding analysis of known bugs.

For vmcores triggered by a driver bug, the system calls print_modules() to
list the loaded modules. However, print_modules() does not output module
version information. Across a large fleet of servers, there are often many
different module versions running simultaneously, and we need to know which
driver version caused a given vmcore.

Currently, the only reliable way to obtain the module version associated
with a vmcore is to analyze the /var/crash/XXXX/vmcore file itself—an
operation that is resource-intensive. Therefore, we propose printing the
driver version directly in the log, which is far more efficient.

The motivation behind this change is that the external NVIDIA driver
[0] frequently causes kernel panics across our server fleet.
While we continuously upgrade to newer NVIDIA driver versions,
upgrading the entire fleet is time-consuming. Therefore, we need to
identify which driver version is responsible for each panic.

In-tree modules are tied to the specific kernel version already, so
printing their versions is redundant. However, for external drivers (like
proprietary networking or GPU stacks), the version is the single most
critical piece of metadata for triage. Therefore, to avoid bloating the
information about loaded modules, we only print the version for external
modules.

- Before this patch

  Modules linked in: mlx5_core(O) nvidia(PO) nvme_core

- After this patch

  Modules linked in: mlx5_core-5.8-2.0.3(O) nvidia-535.274.02(PO) nvme_core
                              ^^^^^^^^^^          ^^^^^^^^^^^

  Note: nvme_core is a in-tree module[1], so its version isn't printed.

Link: https://github.com/NVIDIA/open-gpu-kernel-modules/tags [0]
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/nvme/host/core.c?h=v6.19-rc3#n5448 [1]
Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 kernel/module/main.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

---
v1->v2: 
- print it for external module only (Petr, Aaron)
- add comment for it (Aaron)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 710ee30b3bea..16263ce23e92 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -3901,7 +3901,11 @@ void print_modules(void)
 	list_for_each_entry_rcu(mod, &modules, list) {
 		if (mod->state == MODULE_STATE_UNFORMED)
 			continue;
-		pr_cont(" %s%s", mod->name, module_flags(mod, buf, true));
+		pr_cont(" %s", mod->name);
+		/* Only append version for out-of-tree modules */
+		if (mod->version && test_bit(TAINT_OOT_MODULE, &mod->taints))
+			pr_cont("-%s", mod->version);
+		pr_cont("%s", module_flags(mod, buf, true));
 	}
 
 	print_unloaded_tainted_modules();
-- 
2.43.5

Re: [PATCH v2] module: print version for external modules in print_modules()

[Yafang Shao]

On Wed, Dec 31, 2025 at 5:40 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> We maintain a vmcore analysis script on each server that automatically
> parses /var/crash/XXXX/vmcore-dmesg.txt to categorize vmcores. This helps
> us save considerable effort by avoiding analysis of known bugs.
>
> For vmcores triggered by a driver bug, the system calls print_modules() to
> list the loaded modules. However, print_modules() does not output module
> version information. Across a large fleet of servers, there are often many
> different module versions running simultaneously, and we need to know which
> driver version caused a given vmcore.
>
> Currently, the only reliable way to obtain the module version associated
> with a vmcore is to analyze the /var/crash/XXXX/vmcore file itself—an
> operation that is resource-intensive. Therefore, we propose printing the
> driver version directly in the log, which is far more efficient.
>
> The motivation behind this change is that the external NVIDIA driver
> [0] frequently causes kernel panics across our server fleet.
> While we continuously upgrade to newer NVIDIA driver versions,
> upgrading the entire fleet is time-consuming. Therefore, we need to
> identify which driver version is responsible for each panic.
>
> In-tree modules are tied to the specific kernel version already, so
> printing their versions is redundant. However, for external drivers (like
> proprietary networking or GPU stacks), the version is the single most
> critical piece of metadata for triage. Therefore, to avoid bloating the
> information about loaded modules, we only print the version for external
> modules.
>
> - Before this patch
>
>   Modules linked in: mlx5_core(O) nvidia(PO) nvme_core
>
> - After this patch
>
>   Modules linked in: mlx5_core-5.8-2.0.3(O) nvidia-535.274.02(PO) nvme_core
>                               ^^^^^^^^^^          ^^^^^^^^^^^
>
>   Note: nvme_core is a in-tree module[1], so its version isn't printed.
>
> Link: https://github.com/NVIDIA/open-gpu-kernel-modules/tags [0]
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/nvme/host/core.c?h=v6.19-rc3#n5448 [1]
> Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
> Reviewed-by: Aaron Tomlin <atomlin@atomlin.com>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  kernel/module/main.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> ---
> v1->v2:
> - print it for external module only (Petr, Aaron)
> - add comment for it (Aaron)
>
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 710ee30b3bea..16263ce23e92 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3901,7 +3901,11 @@ void print_modules(void)
>         list_for_each_entry_rcu(mod, &modules, list) {
>                 if (mod->state == MODULE_STATE_UNFORMED)
>                         continue;
> -               pr_cont(" %s%s", mod->name, module_flags(mod, buf, true));
> +               pr_cont(" %s", mod->name);
> +               /* Only append version for out-of-tree modules */
> +               if (mod->version && test_bit(TAINT_OOT_MODULE, &mod->taints))
> +                       pr_cont("-%s", mod->version);
> +               pr_cont("%s", module_flags(mod, buf, true));
>         }
>
>         print_unloaded_tainted_modules();
> --
> 2.43.5
>

Just checking in on this patch. It looks like it hasn't been merged
yet. Is it good to go, or does it need something else?

-- 
Regards
Yafang

Re: [PATCH v2] module: print version for external modules in print_modules()

[Sami Tolvanen]

On Wed, Feb 25, 2026 at 6:19 PM Yafang Shao <laoar.shao@gmail.com> wrote:
> Just checking in on this patch. It looks like it hasn't been merged
> yet. Is it good to go, or does it need something else?

The patch looks reasonable to me. I have it in my queue for v7.1.

Sami

Re: [PATCH v2] module: print version for external modules in print_modules()

[Sami Tolvanen]

On Wed, Dec 31, 2025 at 05:40:04PM +0800, Yafang Shao wrote:
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
> @@ -3901,7 +3901,11 @@ void print_modules(void)
>  	list_for_each_entry_rcu(mod, &modules, list) {
>  		if (mod->state == MODULE_STATE_UNFORMED)
>  			continue;
> -		pr_cont(" %s%s", mod->name, module_flags(mod, buf, true));
> +		pr_cont(" %s", mod->name);
> +		/* Only append version for out-of-tree modules */
> +		if (mod->version && test_bit(TAINT_OOT_MODULE, &mod->taints))
> +			pr_cont("-%s", mod->version);
> +		pr_cont("%s", module_flags(mod, buf, true));

On second thought, is using mod->version here safe? We release the
memory for mod->version in:

  free_module
    -> mod_sysfs_teardown
    -> module_remove_modinfo_attrs
    -> attr->free = free_modinfo_version

And this happens before the module is removed from the
list. Couldn't there be a race condition where we read a non-NULL
mod->version here, but the buffer is being concurrently released
by another core that's unloading the module, resulting in a
use-after-free in the pr_cont call?

In order to do this safely, we should presumably drop the attr->free
call from module_remove_modinfo_attrs and release the attributes
only after the synchronize_rcu call in free_module (there's already
free_modinfo we can use), so mod->version is valid for the entire
time the module is on the list.

Thoughts?

Sami

Re: [PATCH v2] module: print version for external modules in print_modules()

[Yafang Shao]

On Fri, Mar 6, 2026 at 7:43 AM Sami Tolvanen <samitolvanen@google.com> wrote:
>
> On Wed, Dec 31, 2025 at 05:40:04PM +0800, Yafang Shao wrote:
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -3901,7 +3901,11 @@ void print_modules(void)
> >       list_for_each_entry_rcu(mod, &modules, list) {
> >               if (mod->state == MODULE_STATE_UNFORMED)
> >                       continue;
> > -             pr_cont(" %s%s", mod->name, module_flags(mod, buf, true));
> > +             pr_cont(" %s", mod->name);
> > +             /* Only append version for out-of-tree modules */
> > +             if (mod->version && test_bit(TAINT_OOT_MODULE, &mod->taints))
> > +                     pr_cont("-%s", mod->version);
> > +             pr_cont("%s", module_flags(mod, buf, true));
>
> On second thought, is using mod->version here safe? We release the
> memory for mod->version in:
>
>   free_module
>     -> mod_sysfs_teardown
>     -> module_remove_modinfo_attrs
>     -> attr->free = free_modinfo_version
>
> And this happens before the module is removed from the
> list. Couldn't there be a race condition where we read a non-NULL
> mod->version here, but the buffer is being concurrently released
> by another core that's unloading the module, resulting in a
> use-after-free in the pr_cont call?

You're right. This can happen.

>
> In order to do this safely, we should presumably drop the attr->free
> call from module_remove_modinfo_attrs and release the attributes
> only after the synchronize_rcu call in free_module (there's already
> free_modinfo we can use), so mod->version is valid for the entire
> time the module is on the list.
>
> Thoughts?

It looks like this could work. I'll analyze it further—thanks for the
suggestion.

-- 
Regards
Yafang

Re: [PATCH v2] module: print version for external modules in print_modules()

[Petr Pavlu]

On 3/6/26 12:43 AM, Sami Tolvanen wrote:
> On Wed, Dec 31, 2025 at 05:40:04PM +0800, Yafang Shao wrote:
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
>> @@ -3901,7 +3901,11 @@ void print_modules(void)
>>  	list_for_each_entry_rcu(mod, &modules, list) {
>>  		if (mod->state == MODULE_STATE_UNFORMED)
>>  			continue;
>> -		pr_cont(" %s%s", mod->name, module_flags(mod, buf, true));
>> +		pr_cont(" %s", mod->name);
>> +		/* Only append version for out-of-tree modules */
>> +		if (mod->version && test_bit(TAINT_OOT_MODULE, &mod->taints))
>> +			pr_cont("-%s", mod->version);
>> +		pr_cont("%s", module_flags(mod, buf, true));
> 
> On second thought, is using mod->version here safe? We release the
> memory for mod->version in:
> 
>   free_module
>     -> mod_sysfs_teardown
>     -> module_remove_modinfo_attrs
>     -> attr->free = free_modinfo_version
> 
> And this happens before the module is removed from the
> list. Couldn't there be a race condition where we read a non-NULL
> mod->version here, but the buffer is being concurrently released
> by another core that's unloading the module, resulting in a
> use-after-free in the pr_cont call?
> 
> In order to do this safely, we should presumably drop the attr->free
> call from module_remove_modinfo_attrs and release the attributes
> only after the synchronize_rcu call in free_module (there's already
> free_modinfo we can use), so mod->version is valid for the entire
> time the module is on the list.

This looks reasonable to me as a simple fix. I also noticed that
setup_modinfo() with its attr->setup() calls is invoked unconditionally
in kernel/module/main.c, while module_remove_modinfo_attrs() with
attr->free() is present in kernel/module/sysfs.c, which is conditional
on CONFIG_SYSFS. In the unlikely configuration where CONFIG_SYSFS=n and
CONFIG_MODULES=y, this can result in a memory leak of module::version
when a module is unloaded.

In general, I think this could benefit from more cleanup in the future.
Most of the code related to modinfo_attrs should be moved into
kernel/module/sysfs.c. Since module::version is now used from
print_modules(), which is part of the general module loader code, the
initialization of the variable should be independent of all sysfs logic.
Ideally, the sysfs code should only read module::version and no longer
manage it.

-- 
Thanks,
Petr

Re: [PATCH v2] module: print version for external modules in print_modules()

[Yafang Shao]

On Fri, Mar 6, 2026 at 6:10 PM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> On 3/6/26 12:43 AM, Sami Tolvanen wrote:
> > On Wed, Dec 31, 2025 at 05:40:04PM +0800, Yafang Shao wrote:
> >> --- a/kernel/module/main.c
> >> +++ b/kernel/module/main.c
> >> @@ -3901,7 +3901,11 @@ void print_modules(void)
> >>      list_for_each_entry_rcu(mod, &modules, list) {
> >>              if (mod->state == MODULE_STATE_UNFORMED)
> >>                      continue;
> >> -            pr_cont(" %s%s", mod->name, module_flags(mod, buf, true));
> >> +            pr_cont(" %s", mod->name);
> >> +            /* Only append version for out-of-tree modules */
> >> +            if (mod->version && test_bit(TAINT_OOT_MODULE, &mod->taints))
> >> +                    pr_cont("-%s", mod->version);
> >> +            pr_cont("%s", module_flags(mod, buf, true));
> >
> > On second thought, is using mod->version here safe? We release the
> > memory for mod->version in:
> >
> >   free_module
> >     -> mod_sysfs_teardown
> >     -> module_remove_modinfo_attrs
> >     -> attr->free = free_modinfo_version
> >
> > And this happens before the module is removed from the
> > list. Couldn't there be a race condition where we read a non-NULL
> > mod->version here, but the buffer is being concurrently released
> > by another core that's unloading the module, resulting in a
> > use-after-free in the pr_cont call?
> >
> > In order to do this safely, we should presumably drop the attr->free
> > call from module_remove_modinfo_attrs and release the attributes
> > only after the synchronize_rcu call in free_module (there's already
> > free_modinfo we can use), so mod->version is valid for the entire
> > time the module is on the list.
>
> This looks reasonable to me as a simple fix.

I will send a fix for it first.

> I also noticed that
> setup_modinfo() with its attr->setup() calls is invoked unconditionally
> in kernel/module/main.c, while module_remove_modinfo_attrs() with
> attr->free() is present in kernel/module/sysfs.c, which is conditional
> on CONFIG_SYSFS. In the unlikely configuration where CONFIG_SYSFS=n and
> CONFIG_MODULES=y, this can result in a memory leak of module::version
> when a module is unloaded.
>
> In general, I think this could benefit from more cleanup in the future.
> Most of the code related to modinfo_attrs should be moved into
> kernel/module/sysfs.c.

Following the change suggested by Sami, the modinfo_attrs definition,
along with attr->free() and attr->setup(), remains in
kernel/module/main.c. Should any of these components be relocated to
kernel/module/sysfs.c?

> Since module::version is now used from
> print_modules(), which is part of the general module loader code, the
> initialization of the variable should be independent of all sysfs logic.
> Ideally, the sysfs code should only read module::version and no longer
> manage it.

After the above change, both the allocation and freeing of
module::version now reside in kernel/module/main.c. Is this
sufficient?

-- 
Regards
Yafang

Re: [PATCH v2] module: print version for external modules in print_modules()

[Petr Pavlu]

On 3/8/26 3:14 PM, Yafang Shao wrote:
> On Fri, Mar 6, 2026 at 6:10 PM Petr Pavlu <petr.pavlu@suse.com> wrote:
>>
>> On 3/6/26 12:43 AM, Sami Tolvanen wrote:
>>> On Wed, Dec 31, 2025 at 05:40:04PM +0800, Yafang Shao wrote:
>>>> --- a/kernel/module/main.c
>>>> +++ b/kernel/module/main.c
>>>> @@ -3901,7 +3901,11 @@ void print_modules(void)
>>>>      list_for_each_entry_rcu(mod, &modules, list) {
>>>>              if (mod->state == MODULE_STATE_UNFORMED)
>>>>                      continue;
>>>> -            pr_cont(" %s%s", mod->name, module_flags(mod, buf, true));
>>>> +            pr_cont(" %s", mod->name);
>>>> +            /* Only append version for out-of-tree modules */
>>>> +            if (mod->version && test_bit(TAINT_OOT_MODULE, &mod->taints))
>>>> +                    pr_cont("-%s", mod->version);
>>>> +            pr_cont("%s", module_flags(mod, buf, true));
>>>
>>> On second thought, is using mod->version here safe? We release the
>>> memory for mod->version in:
>>>
>>>   free_module
>>>     -> mod_sysfs_teardown
>>>     -> module_remove_modinfo_attrs
>>>     -> attr->free = free_modinfo_version
>>>
>>> And this happens before the module is removed from the
>>> list. Couldn't there be a race condition where we read a non-NULL
>>> mod->version here, but the buffer is being concurrently released
>>> by another core that's unloading the module, resulting in a
>>> use-after-free in the pr_cont call?
>>>
>>> In order to do this safely, we should presumably drop the attr->free
>>> call from module_remove_modinfo_attrs and release the attributes
>>> only after the synchronize_rcu call in free_module (there's already
>>> free_modinfo we can use), so mod->version is valid for the entire
>>> time the module is on the list.
>>
>> This looks reasonable to me as a simple fix.
> 
> I will send a fix for it first.

Ack.

> 
>> I also noticed that
>> setup_modinfo() with its attr->setup() calls is invoked unconditionally
>> in kernel/module/main.c, while module_remove_modinfo_attrs() with
>> attr->free() is present in kernel/module/sysfs.c, which is conditional
>> on CONFIG_SYSFS. In the unlikely configuration where CONFIG_SYSFS=n and
>> CONFIG_MODULES=y, this can result in a memory leak of module::version
>> when a module is unloaded.
>>
>> In general, I think this could benefit from more cleanup in the future.
>> Most of the code related to modinfo_attrs should be moved into
>> kernel/module/sysfs.c.
> 
> Following the change suggested by Sami, the modinfo_attrs definition,
> along with attr->free() and attr->setup(), remains in
> kernel/module/main.c. Should any of these components be relocated to
> kernel/module/sysfs.c?
> 
>> Since module::version is now used from
>> print_modules(), which is part of the general module loader code, the
>> initialization of the variable should be independent of all sysfs logic.
>> Ideally, the sysfs code should only read module::version and no longer
>> manage it.
> 
> After the above change, both the allocation and freeing of
> module::version now reside in kernel/module/main.c. Is this
> sufficient?

I think we can further clean up this area by moving more items into
kernel/module/sysfs.c and untangling module::(src)version from the sysfs
code. A quick prototype is shown below. However, this doesn't need to be
done now and I can send this separately later.

-- 
Thanks,
Petr


diff --git a/drivers/base/core.c b/drivers/base/core.c
index 791f9e444df8..dfff1b5fa463 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4296,7 +4296,7 @@ struct device *__root_device_register(const char *name, struct module *owner)
 		return ERR_PTR(err);
 	}
 
-#ifdef CONFIG_MODULES	/* gotta find a "cleaner" way to do this */
+#if defined(CONFIG_MODULES) && defined(CONFIG_SYSFS)	/* gotta find a "cleaner" way to do this */
 	if (owner) {
 		struct module_kobject *mk = &owner->mkobj;
 
diff --git a/include/linux/module.h b/include/linux/module.h
index 14f391b186c6..5959a9695d93 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -57,9 +57,7 @@ struct module_attribute {
 			char *);
 	ssize_t (*store)(const struct module_attribute *, struct module_kobject *,
 			 const char *, size_t count);
-	void (*setup)(struct module *, const char *);
 	int (*test)(struct module *);
-	void (*free)(struct module *);
 };
 
 struct module_version_attribute {
@@ -408,12 +406,15 @@ struct module {
 	unsigned char build_id[BUILD_ID_SIZE_MAX];
 #endif
 
+	const char *version;
+	const char *srcversion;
+
+#ifdef CONFIG_SYSFS
 	/* Sysfs stuff. */
 	struct module_kobject mkobj;
 	struct module_attribute *modinfo_attrs;
-	const char *version;
-	const char *srcversion;
 	struct kobject *holders_dir;
+#endif
 
 	/* Exported symbols */
 	const struct kernel_symbol *syms;
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 618202578b42..cbaa14af9b5a 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -47,9 +47,6 @@ struct kernel_symbol {
 extern struct mutex module_mutex;
 extern struct list_head modules;
 
-extern const struct module_attribute *const modinfo_attrs[];
-extern const size_t modinfo_attrs_count;
-
 /* Provided by the linker */
 extern const struct kernel_symbol __start___ksymtab[];
 extern const struct kernel_symbol __stop___ksymtab[];
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c3ce106c70af..c665c4144080 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -577,36 +577,6 @@ bool __is_module_percpu_address(unsigned long addr, unsigned long *can_addr)
 
 #endif /* CONFIG_SMP */
 
-#define MODINFO_ATTR(field)	\
-static void setup_modinfo_##field(struct module *mod, const char *s)  \
-{                                                                     \
-	mod->field = kstrdup(s, GFP_KERNEL);                          \
-}                                                                     \
-static ssize_t show_modinfo_##field(const struct module_attribute *mattr, \
-			struct module_kobject *mk, char *buffer)      \
-{                                                                     \
-	return scnprintf(buffer, PAGE_SIZE, "%s\n", mk->mod->field);  \
-}                                                                     \
-static int modinfo_##field##_exists(struct module *mod)               \
-{                                                                     \
-	return mod->field != NULL;                                    \
-}                                                                     \
-static void free_modinfo_##field(struct module *mod)                  \
-{                                                                     \
-	kfree(mod->field);                                            \
-	mod->field = NULL;                                            \
-}                                                                     \
-static const struct module_attribute modinfo_##field = {              \
-	.attr = { .name = __stringify(field), .mode = 0444 },         \
-	.show = show_modinfo_##field,                                 \
-	.setup = setup_modinfo_##field,                               \
-	.test = modinfo_##field##_exists,                             \
-	.free = free_modinfo_##field,                                 \
-};
-
-MODINFO_ATTR(version);
-MODINFO_ATTR(srcversion);
-
 static struct {
 	char name[MODULE_NAME_LEN];
 	char taints[MODULE_FLAGS_BUF_SIZE];
@@ -886,15 +856,6 @@ void symbol_put_addr(void *addr)
 }
 EXPORT_SYMBOL_GPL(symbol_put_addr);
 
-static ssize_t show_refcnt(const struct module_attribute *mattr,
-			   struct module_kobject *mk, char *buffer)
-{
-	return sprintf(buffer, "%i\n", module_refcount(mk->mod));
-}
-
-static const struct module_attribute modinfo_refcnt =
-	__ATTR(refcnt, 0444, show_refcnt, NULL);
-
 void __module_get(struct module *module)
 {
 	if (module) {
@@ -961,118 +922,6 @@ size_t module_flags_taint(unsigned long taints, char *buf)
 	return l;
 }
 
-static ssize_t show_initstate(const struct module_attribute *mattr,
-			      struct module_kobject *mk, char *buffer)
-{
-	const char *state = "unknown";
-
-	switch (mk->mod->state) {
-	case MODULE_STATE_LIVE:
-		state = "live";
-		break;
-	case MODULE_STATE_COMING:
-		state = "coming";
-		break;
-	case MODULE_STATE_GOING:
-		state = "going";
-		break;
-	default:
-		BUG();
-	}
-	return sprintf(buffer, "%s\n", state);
-}
-
-static const struct module_attribute modinfo_initstate =
-	__ATTR(initstate, 0444, show_initstate, NULL);
-
-static ssize_t store_uevent(const struct module_attribute *mattr,
-			    struct module_kobject *mk,
-			    const char *buffer, size_t count)
-{
-	int rc;
-
-	rc = kobject_synth_uevent(&mk->kobj, buffer, count);
-	return rc ? rc : count;
-}
-
-const struct module_attribute module_uevent =
-	__ATTR(uevent, 0200, NULL, store_uevent);
-
-static ssize_t show_coresize(const struct module_attribute *mattr,
-			     struct module_kobject *mk, char *buffer)
-{
-	unsigned int size = mk->mod->mem[MOD_TEXT].size;
-
-	if (!IS_ENABLED(CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC)) {
-		for_class_mod_mem_type(type, core_data)
-			size += mk->mod->mem[type].size;
-	}
-	return sprintf(buffer, "%u\n", size);
-}
-
-static const struct module_attribute modinfo_coresize =
-	__ATTR(coresize, 0444, show_coresize, NULL);
-
-#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
-static ssize_t show_datasize(const struct module_attribute *mattr,
-			     struct module_kobject *mk, char *buffer)
-{
-	unsigned int size = 0;
-
-	for_class_mod_mem_type(type, core_data)
-		size += mk->mod->mem[type].size;
-	return sprintf(buffer, "%u\n", size);
-}
-
-static const struct module_attribute modinfo_datasize =
-	__ATTR(datasize, 0444, show_datasize, NULL);
-#endif
-
-static ssize_t show_initsize(const struct module_attribute *mattr,
-			     struct module_kobject *mk, char *buffer)
-{
-	unsigned int size = 0;
-
-	for_class_mod_mem_type(type, init)
-		size += mk->mod->mem[type].size;
-	return sprintf(buffer, "%u\n", size);
-}
-
-static const struct module_attribute modinfo_initsize =
-	__ATTR(initsize, 0444, show_initsize, NULL);
-
-static ssize_t show_taint(const struct module_attribute *mattr,
-			  struct module_kobject *mk, char *buffer)
-{
-	size_t l;
-
-	l = module_flags_taint(mk->mod->taints, buffer);
-	buffer[l++] = '\n';
-	return l;
-}
-
-static const struct module_attribute modinfo_taint =
-	__ATTR(taint, 0444, show_taint, NULL);
-
-const struct module_attribute *const modinfo_attrs[] = {
-	&module_uevent,
-	&modinfo_version,
-	&modinfo_srcversion,
-	&modinfo_initstate,
-	&modinfo_coresize,
-#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
-	&modinfo_datasize,
-#endif
-	&modinfo_initsize,
-	&modinfo_taint,
-#ifdef CONFIG_MODULE_UNLOAD
-	&modinfo_refcnt,
-#endif
-	NULL,
-};
-
-const size_t modinfo_attrs_count = ARRAY_SIZE(modinfo_attrs);
-
 static const char vermagic[] = VERMAGIC_STRING;
 
 int try_to_force_load(struct module *mod, const char *reason)
@@ -1762,14 +1611,11 @@ static void module_license_taint_check(struct module *mod, const char *license)
 
 static int setup_modinfo(struct module *mod, struct load_info *info)
 {
-	const struct module_attribute *attr;
 	char *imported_namespace;
-	int i;
 
-	for (i = 0; (attr = modinfo_attrs[i]); i++) {
-		if (attr->setup)
-			attr->setup(mod, get_modinfo(info, attr->attr.name));
-	}
+	/* TODO Check for failed kstrdup(). */
+	mod->version = kstrdup(get_modinfo(info, "version"), GFP_KERNEL);
+	mod->srcversion = kstrdup(get_modinfo(info, "srcversion"), GFP_KERNEL);
 
 	for_each_modinfo_entry(imported_namespace, info, "import_ns") {
 		/*
@@ -1788,13 +1634,8 @@ static int setup_modinfo(struct module *mod, struct load_info *info)
 
 static void free_modinfo(struct module *mod)
 {
-	const struct module_attribute *attr;
-	int i;
-
-	for (i = 0; (attr = modinfo_attrs[i]); i++) {
-		if (attr->free)
-			attr->free(mod);
-	}
+	kfree(mod->version);
+	kfree(mod->srcversion);
 }
 
 bool __weak module_init_section(const char *name)
@@ -3060,8 +2901,10 @@ static noinline int do_init_module(struct module *mod)
 	blocking_notifier_call_chain(&module_notify_list,
 				     MODULE_STATE_LIVE, mod);
 
+#ifdef CONFIG_SYSFS
 	/* Delay uevent until module has finished its init routine */
 	kobject_uevent(&mod->mkobj.kobj, KOBJ_ADD);
+#endif
 
 	/*
 	 * We need to finish all async code before the module init sequence
diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c
index 01c65d608873..f2e3a625bcd5 100644
--- a/kernel/module/sysfs.c
+++ b/kernel/module/sysfs.c
@@ -14,6 +14,144 @@
 #include <linux/mutex.h>
 #include "internal.h"
 
+#define MODINFO_ATTR(field)	\
+static ssize_t show_modinfo_##field(const struct module_attribute *mattr, \
+			struct module_kobject *mk, char *buffer)      \
+{                                                                     \
+	return scnprintf(buffer, PAGE_SIZE, "%s\n", mk->mod->field);  \
+}                                                                     \
+static int modinfo_##field##_exists(struct module *mod)               \
+{                                                                     \
+	return mod->field != NULL;                                    \
+}                                                                     \
+static const struct module_attribute modinfo_##field = {              \
+	.attr = { .name = __stringify(field), .mode = 0444 },         \
+	.show = show_modinfo_##field,                                 \
+	.test = modinfo_##field##_exists,                             \
+};
+
+MODINFO_ATTR(version);
+MODINFO_ATTR(srcversion);
+
+static ssize_t show_initstate(const struct module_attribute *mattr,
+			      struct module_kobject *mk, char *buffer)
+{
+	const char *state = "unknown";
+
+	switch (mk->mod->state) {
+	case MODULE_STATE_LIVE:
+		state = "live";
+		break;
+	case MODULE_STATE_COMING:
+		state = "coming";
+		break;
+	case MODULE_STATE_GOING:
+		state = "going";
+		break;
+	default:
+		BUG();
+	}
+	return sprintf(buffer, "%s\n", state);
+}
+
+static const struct module_attribute modinfo_initstate =
+	__ATTR(initstate, 0444, show_initstate, NULL);
+
+static ssize_t store_uevent(const struct module_attribute *mattr,
+			    struct module_kobject *mk,
+			    const char *buffer, size_t count)
+{
+	int rc;
+
+	rc = kobject_synth_uevent(&mk->kobj, buffer, count);
+	return rc ? rc : count;
+}
+
+const struct module_attribute module_uevent =
+	__ATTR(uevent, 0200, NULL, store_uevent);
+
+static ssize_t show_coresize(const struct module_attribute *mattr,
+			     struct module_kobject *mk, char *buffer)
+{
+	unsigned int size = mk->mod->mem[MOD_TEXT].size;
+
+	if (!IS_ENABLED(CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC)) {
+		for_class_mod_mem_type(type, core_data)
+			size += mk->mod->mem[type].size;
+	}
+	return sprintf(buffer, "%u\n", size);
+}
+
+static const struct module_attribute modinfo_coresize =
+	__ATTR(coresize, 0444, show_coresize, NULL);
+
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+static ssize_t show_datasize(const struct module_attribute *mattr,
+			     struct module_kobject *mk, char *buffer)
+{
+	unsigned int size = 0;
+
+	for_class_mod_mem_type(type, core_data)
+		size += mk->mod->mem[type].size;
+	return sprintf(buffer, "%u\n", size);
+}
+
+static const struct module_attribute modinfo_datasize =
+	__ATTR(datasize, 0444, show_datasize, NULL);
+#endif
+
+static ssize_t show_initsize(const struct module_attribute *mattr,
+			     struct module_kobject *mk, char *buffer)
+{
+	unsigned int size = 0;
+
+	for_class_mod_mem_type(type, init)
+		size += mk->mod->mem[type].size;
+	return sprintf(buffer, "%u\n", size);
+}
+
+static const struct module_attribute modinfo_initsize =
+	__ATTR(initsize, 0444, show_initsize, NULL);
+
+static ssize_t show_taint(const struct module_attribute *mattr,
+			  struct module_kobject *mk, char *buffer)
+{
+	size_t l;
+
+	l = module_flags_taint(mk->mod->taints, buffer);
+	buffer[l++] = '\n';
+	return l;
+}
+
+static const struct module_attribute modinfo_taint =
+	__ATTR(taint, 0444, show_taint, NULL);
+
+static ssize_t show_refcnt(const struct module_attribute *mattr,
+			   struct module_kobject *mk, char *buffer)
+{
+	return sprintf(buffer, "%i\n", module_refcount(mk->mod));
+}
+
+static const struct module_attribute modinfo_refcnt =
+	__ATTR(refcnt, 0444, show_refcnt, NULL);
+
+static const struct module_attribute *const modinfo_attrs[] = {
+	&module_uevent,
+	&modinfo_version,
+	&modinfo_srcversion,
+	&modinfo_initstate,
+	&modinfo_coresize,
+#ifdef CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC
+	&modinfo_datasize,
+#endif
+	&modinfo_initsize,
+	&modinfo_taint,
+#ifdef CONFIG_MODULE_UNLOAD
+	&modinfo_refcnt,
+#endif
+	NULL,
+};
+
 /*
  * /sys/module/foo/sections stuff
  * J. Corbet <corbet@lwn.net>
@@ -278,8 +416,6 @@ static void module_remove_modinfo_attrs(struct module *mod, int end)
 		if (!attr->attr.name)
 			break;
 		sysfs_remove_file(&mod->mkobj.kobj, &attr->attr);
-		if (attr->free)
-			attr->free(mod);
 	}
 	kfree(mod->modinfo_attrs);
 }
@@ -292,7 +428,7 @@ static int module_add_modinfo_attrs(struct module *mod)
 	int i;
 
 	mod->modinfo_attrs = kzalloc((sizeof(struct module_attribute) *
-					(modinfo_attrs_count + 1)),
+					(ARRAY_SIZE(modinfo_attrs) + 1)),
 					GFP_KERNEL);
 	if (!mod->modinfo_attrs)
 		return -ENOMEM;