| Message ID | 20210818112203.24863-1-tiwai@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | modinfo: don't parse built-in for explicitly given module files | expand |
On Wed, Aug 18, 2021 at 01:22:03PM +0200, Takashi Iwai wrote: > A recent bug report showed that modinfo doesn't give the signature > information for certain modules, and it turned out to happen only on > the modules that are built-in on the running kernel; then modinfo > skips the signature check, as if the target module file never exists. > The behavior is, however, inconsistent when modinfo is performed for > external modules (no matter which kernel version is) and the module > file path is explicitly given by a command-line argument, which > guarantees the presence of the module file itself. > > This patch addresses the regression by checking the presence of the > module path at first before checking the built-in module. > > Fixes: e7e2cb61fa9f ("modinfo: Show information about built-in modules") > BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1189537 > Signed-off-by: Takashi Iwai <tiwai@suse.de> Reviewed-by: Michal Suchánek <msuchanek@suse.de> LGTM Thanks > --- > libkmod/libkmod-module.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c > index 6e0ff1a99604..9e878a5345a1 100644 > --- a/libkmod/libkmod-module.c > +++ b/libkmod/libkmod-module.c > @@ -2292,7 +2292,8 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_ > assert(*list == NULL); > > /* remove const: this can only change internal state */ > - if (kmod_module_is_builtin((struct kmod_module *)mod)) { > + if (!kmod_module_get_path(mod) && > + kmod_module_is_builtin((struct kmod_module *)mod)) { > count = kmod_builtin_get_modinfo(mod->ctx, > kmod_module_get_name(mod), > &strings); > -- > 2.26.2 >
Hi,
Reviewed-by: Petr Vorel <pvorel@suse.cz>
Kind regards,
Petr
On Wed, Aug 18, 2021 at 4:23 AM Takashi Iwai <tiwai@suse.de> wrote: > > A recent bug report showed that modinfo doesn't give the signature > information for certain modules, and it turned out to happen only on > the modules that are built-in on the running kernel; then modinfo > skips the signature check, as if the target module file never exists. > The behavior is, however, inconsistent when modinfo is performed for > external modules (no matter which kernel version is) and the module > file path is explicitly given by a command-line argument, which > guarantees the presence of the module file itself. > > This patch addresses the regression by checking the presence of the > module path at first before checking the built-in module. > > Fixes: e7e2cb61fa9f ("modinfo: Show information about built-in modules") > BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1189537 > Signed-off-by: Takashi Iwai <tiwai@suse.de> > --- > libkmod/libkmod-module.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c > index 6e0ff1a99604..9e878a5345a1 100644 > --- a/libkmod/libkmod-module.c > +++ b/libkmod/libkmod-module.c > @@ -2292,7 +2292,8 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_ > assert(*list == NULL); > > /* remove const: this can only change internal state */ > - if (kmod_module_is_builtin((struct kmod_module *)mod)) { > + if (!kmod_module_get_path(mod) && > + kmod_module_is_builtin((struct kmod_module *)mod)) { thanks for spotting this... but I'm not sure this is the right fix. kmod_module_is_builtin() shouldn't return true if the module was created by path rather than lookup. It seems we should rather set mod->builtin = KMOD_MODULE_BUILTIN_NO in kmod_module_new_from_path(). This would also fix the use of kmod_module_is_builtin() directly by applications. Since the builtin check uses lazy initialization, the default is KMOD_MODULE_BUILTIN_UNKNOWN, hence the bug here. Lucas De Marchi > count = kmod_builtin_get_modinfo(mod->ctx, > kmod_module_get_name(mod), > &strings); > -- > 2.26.2 >
diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c index 6e0ff1a99604..9e878a5345a1 100644 --- a/libkmod/libkmod-module.c +++ b/libkmod/libkmod-module.c @@ -2292,7 +2292,8 @@ KMOD_EXPORT int kmod_module_get_info(const struct kmod_module *mod, struct kmod_ assert(*list == NULL); /* remove const: this can only change internal state */ - if (kmod_module_is_builtin((struct kmod_module *)mod)) { + if (!kmod_module_get_path(mod) && + kmod_module_is_builtin((struct kmod_module *)mod)) { count = kmod_builtin_get_modinfo(mod->ctx, kmod_module_get_name(mod), &strings);
A recent bug report showed that modinfo doesn't give the signature information for certain modules, and it turned out to happen only on the modules that are built-in on the running kernel; then modinfo skips the signature check, as if the target module file never exists. The behavior is, however, inconsistent when modinfo is performed for external modules (no matter which kernel version is) and the module file path is explicitly given by a command-line argument, which guarantees the presence of the module file itself. This patch addresses the regression by checking the presence of the module path at first before checking the built-in module. Fixes: e7e2cb61fa9f ("modinfo: Show information about built-in modules") BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1189537 Signed-off-by: Takashi Iwai <tiwai@suse.de> --- libkmod/libkmod-module.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)