diff mbox series

modinfo must show real module info, not context if filename set

Message ID dda23def071a8d087cca3538289de1e0@bspu.by (mailing list archive)
State New, archived
Headers show
Series modinfo must show real module info, not context if filename set | expand

Commit Message

Denis Kaganovich Feb. 26, 2020, 2:53 a.m. UTC
After commit e7e2cb61fa9f1db3429d91ef6accff549500d268, even if real 
filename
passed - modinfo show info from context (so, I got built-in info from 
running
kernel, but asking for new kernel's external module). This behaviour 
unobvious
and incompatible with pre-v27. Simple use fake context for filename - 
IMHO
much less ugly then current results.

Signed-off-by: Dzianis Kahanovich <mahatma@eu.by>

Comments

Alexey Gladkov Feb. 26, 2020, 6:31 p.m. UTC | #1
On Wed, Feb 26, 2020 at 04:53:45AM +0200, Denis Kaganovich wrote:
> After commit e7e2cb61fa9f1db3429d91ef6accff549500d268, even if real filename
> passed - modinfo show info from context (so, I got built-in info from
> running
> kernel, but asking for new kernel's external module). This behaviour
> unobvious
> and incompatible with pre-v27. Simple use fake context for filename - IMHO
> much less ugly then current results.

Can you give an example of this misbehavior ?

I have a two kernel directories (generic and current) with
modules.builtin.modinfo:

$ ls -1 /lib/modules/{5.2.0-generic,`uname -r`}/modules.builtin.modinfo 
/lib/modules/5.2.0-generic/modules.builtin.modinfo
/lib/modules/5.2.0-current/modules.builtin.modinfo

The ext4 module is built into both:

$ tr '\0' '\n' < /lib/modules/`uname -r`/modules.builtin.modinfo |grep ^ext4.description
ext4.description=Fourth Extended Filesystem

$ tr '\0' '\n' < /lib/modules/5.2.0-generic/modules.builtin.modinfo |grep ^ext4.description
ext4.description=Fourth Extended Filesystem

Now I have build this module separately and put it into the tree:

$ find /lib/modules/{5.2.0-generic,`uname -r`} -name 'ext4.*'
/lib/modules/5.2.0-generic/kernel/fs/ext4/ext4.ko.gz

For current kernel:

$ tools/modinfo ext4 |grep filename
filename: (builtin)

For the other kernel:

$ tools/modinfo -k 5.2.0-generic ext4 |grep filename
filename: /lib/modules/5.2.0-generic/kernel/fs/ext4/ext4.ko.gz

Only the external module is displayed, even if there is a module with the
same name in the modules.builtin.modinfo.

I also have a very old kernel without modules.builtin.modinfo:

$ tools/modinfo -k 4.14.96-generic ext4 |grep filename
filename: /lib/modules/4.14.96-generic/kernel/fs/ext4/ext4.ko.gz

> Signed-off-by: Dzianis Kahanovich <mahatma@eu.by>
> 
> --- a/tools/modinfo.c	2020-02-25 13:46:38.181693570 +0300
> +++ b/tools/modinfo.c	2020-02-26 05:18:39.393790919 +0300
> @@ -359,7 +359,7 @@ static bool is_module_filename(const cha
> 
>  static int do_modinfo(int argc, char *argv[])
>  {
> -	struct kmod_ctx *ctx;
> +	struct kmod_ctx *ctx, *ctx0;
>  	char dirname_buf[PATH_MAX];
>  	const char *dirname = NULL;
>  	const char *kversion = NULL;
> @@ -437,7 +437,8 @@ static int do_modinfo(int argc, char *ar
>  	}
> 
>  	ctx = kmod_new(dirname, &null_config);
> -	if (!ctx) {
> +	ctx0 = kmod_new("/dev/null", &null_config);
> +	if (!ctx || !ctx0) {
>  		ERR("kmod_new() failed!\n");
>  		return EXIT_FAILURE;
>  	}
> @@ -448,7 +449,7 @@ static int do_modinfo(int argc, char *ar
>  		int r;
> 
>  		if (is_module_filename(name))
> -			r = modinfo_path_do(ctx, name);
> +			r = modinfo_path_do(ctx0, name);
>  		else
>  			r = modinfo_alias_do(ctx, name);
> 
> @@ -456,6 +457,7 @@ static int do_modinfo(int argc, char *ar
>  			err = r;
>  	}
> 
> +	kmod_unref(ctx0);
>  	kmod_unref(ctx);
>  	return err >= 0 ? EXIT_SUCCESS : EXIT_FAILURE;
>  }
>
Lucas De Marchi Feb. 26, 2020, 7:36 p.m. UTC | #2
On Wed, Feb 26, 2020 at 10:33 AM Alexey Gladkov
<gladkov.alexey@gmail.com> wrote:
>
> On Wed, Feb 26, 2020 at 04:53:45AM +0200, Denis Kaganovich wrote:
> > After commit e7e2cb61fa9f1db3429d91ef6accff549500d268, even if real filename
> > passed - modinfo show info from context (so, I got built-in info from
> > running
> > kernel, but asking for new kernel's external module). This behaviour
> > unobvious
> > and incompatible with pre-v27. Simple use fake context for filename - IMHO
> > much less ugly then current results.
>
> Can you give an example of this misbehavior ?
>
> I have a two kernel directories (generic and current) with
> modules.builtin.modinfo:
>
> $ ls -1 /lib/modules/{5.2.0-generic,`uname -r`}/modules.builtin.modinfo
> /lib/modules/5.2.0-generic/modules.builtin.modinfo
> /lib/modules/5.2.0-current/modules.builtin.modinfo
>
> The ext4 module is built into both:
>
> $ tr '\0' '\n' < /lib/modules/`uname -r`/modules.builtin.modinfo |grep ^ext4.description
> ext4.description=Fourth Extended Filesystem
>
> $ tr '\0' '\n' < /lib/modules/5.2.0-generic/modules.builtin.modinfo |grep ^ext4.description
> ext4.description=Fourth Extended Filesystem
>
> Now I have build this module separately and put it into the tree:

that's why you don't have this problem. If you just build a new
ext4.ko module and give it
in the command line without putting it into the tree, then this
problem would be triggered.

$ ~/p/kmod/tools/modinfo fs/ext4/ext4.ko
filename:       /home/ldmartin/p/gfx-internal/drm-intel-internal/fs/ext4/ext4.ko
modinfo: ERROR: could not get modinfo from 'ext4': No such file or directory

$ modinfo fs/ext4/ext4.ko
filename:       /home/ldmartin/p/gfx-internal/drm-intel-internal/fs/ext4/ext4.ko
softdep:        pre: crc32c
license:        GPL
description:    Fourth Extended Filesystem
author:         Remy Card, Stephen Tweedie, Andrew Morton, Andreas
Dilger, Theodore Ts'o and others
alias:          fs-ext4
alias:          ext3
alias:          fs-ext3
alias:          ext2
alias:          fs-ext2
depends:        mbcache,jbd2
retpoline:      Y
intree:         Y
name:           ext4
vermagic:       5.4.17+ SMP preempt mod_unload

Lucas De Marchi

>
> $ find /lib/modules/{5.2.0-generic,`uname -r`} -name 'ext4.*'
> /lib/modules/5.2.0-generic/kernel/fs/ext4/ext4.ko.gz
>
> For current kernel:
>
> $ tools/modinfo ext4 |grep filename
> filename: (builtin)
>
> For the other kernel:
>
> $ tools/modinfo -k 5.2.0-generic ext4 |grep filename
> filename: /lib/modules/5.2.0-generic/kernel/fs/ext4/ext4.ko.gz
>
> Only the external module is displayed, even if there is a module with the
> same name in the modules.builtin.modinfo.
>
> I also have a very old kernel without modules.builtin.modinfo:
>
> $ tools/modinfo -k 4.14.96-generic ext4 |grep filename
> filename: /lib/modules/4.14.96-generic/kernel/fs/ext4/ext4.ko.gz
>
> > Signed-off-by: Dzianis Kahanovich <mahatma@eu.by>
> >
> > --- a/tools/modinfo.c 2020-02-25 13:46:38.181693570 +0300
> > +++ b/tools/modinfo.c 2020-02-26 05:18:39.393790919 +0300
> > @@ -359,7 +359,7 @@ static bool is_module_filename(const cha
> >
> >  static int do_modinfo(int argc, char *argv[])
> >  {
> > -     struct kmod_ctx *ctx;
> > +     struct kmod_ctx *ctx, *ctx0;
> >       char dirname_buf[PATH_MAX];
> >       const char *dirname = NULL;
> >       const char *kversion = NULL;
> > @@ -437,7 +437,8 @@ static int do_modinfo(int argc, char *ar
> >       }
> >
> >       ctx = kmod_new(dirname, &null_config);
> > -     if (!ctx) {
> > +     ctx0 = kmod_new("/dev/null", &null_config);
> > +     if (!ctx || !ctx0) {
> >               ERR("kmod_new() failed!\n");
> >               return EXIT_FAILURE;
> >       }
> > @@ -448,7 +449,7 @@ static int do_modinfo(int argc, char *ar
> >               int r;
> >
> >               if (is_module_filename(name))
> > -                     r = modinfo_path_do(ctx, name);
> > +                     r = modinfo_path_do(ctx0, name);
> >               else
> >                       r = modinfo_alias_do(ctx, name);
> >
> > @@ -456,6 +457,7 @@ static int do_modinfo(int argc, char *ar
> >                       err = r;
> >       }
> >
> > +     kmod_unref(ctx0);
> >       kmod_unref(ctx);
> >       return err >= 0 ? EXIT_SUCCESS : EXIT_FAILURE;
> >  }
> >
>
> --
> Rgrds, legion
>
Alexey Gladkov Feb. 26, 2020, 10:36 p.m. UTC | #3
On Wed, Feb 26, 2020 at 11:36:00AM -0800, Lucas De Marchi wrote:
> On Wed, Feb 26, 2020 at 10:33 AM Alexey Gladkov
> <gladkov.alexey@gmail.com> wrote:
> >
> > On Wed, Feb 26, 2020 at 04:53:45AM +0200, Denis Kaganovich wrote:
> > > After commit e7e2cb61fa9f1db3429d91ef6accff549500d268, even if real filename
> > > passed - modinfo show info from context (so, I got built-in info from
> > > running
> > > kernel, but asking for new kernel's external module). This behaviour
> > > unobvious
> > > and incompatible with pre-v27. Simple use fake context for filename - IMHO
> > > much less ugly then current results.
> >
> > Can you give an example of this misbehavior ?
> >
> > I have a two kernel directories (generic and current) with
> > modules.builtin.modinfo:
> >
> > $ ls -1 /lib/modules/{5.2.0-generic,`uname -r`}/modules.builtin.modinfo
> > /lib/modules/5.2.0-generic/modules.builtin.modinfo
> > /lib/modules/5.2.0-current/modules.builtin.modinfo
> >
> > The ext4 module is built into both:
> >
> > $ tr '\0' '\n' < /lib/modules/`uname -r`/modules.builtin.modinfo |grep ^ext4.description
> > ext4.description=Fourth Extended Filesystem
> >
> > $ tr '\0' '\n' < /lib/modules/5.2.0-generic/modules.builtin.modinfo |grep ^ext4.description
> > ext4.description=Fourth Extended Filesystem
> >
> > Now I have build this module separately and put it into the tree:
> 
> that's why you don't have this problem. If you just build a new
> ext4.ko module and give it
> in the command line without putting it into the tree, then this
> problem would be triggered.

Ouch, I see. The phrase "new kernel's external module" confused me because
usually the modules for the new kernel are in the tree. My bad.

> $ ~/p/kmod/tools/modinfo fs/ext4/ext4.ko
> filename:       /home/ldmartin/p/gfx-internal/drm-intel-internal/fs/ext4/ext4.ko
> modinfo: ERROR: could not get modinfo from 'ext4': No such file or directory
> 
> $ modinfo fs/ext4/ext4.ko
> filename:       /home/ldmartin/p/gfx-internal/drm-intel-internal/fs/ext4/ext4.ko
> softdep:        pre: crc32c
> license:        GPL
> description:    Fourth Extended Filesystem
> author:         Remy Card, Stephen Tweedie, Andrew Morton, Andreas
> Dilger, Theodore Ts'o and others
> alias:          fs-ext4
> alias:          ext3
> alias:          fs-ext3
> alias:          ext2
> alias:          fs-ext2
> depends:        mbcache,jbd2
> retpoline:      Y
> intree:         Y
> name:           ext4
> vermagic:       5.4.17+ SMP preempt mod_unload
> 
> Lucas De Marchi
diff mbox series

Patch

--- a/tools/modinfo.c	2020-02-25 13:46:38.181693570 +0300
+++ b/tools/modinfo.c	2020-02-26 05:18:39.393790919 +0300
@@ -359,7 +359,7 @@  static bool is_module_filename(const cha

  static int do_modinfo(int argc, char *argv[])
  {
-	struct kmod_ctx *ctx;
+	struct kmod_ctx *ctx, *ctx0;
  	char dirname_buf[PATH_MAX];
  	const char *dirname = NULL;
  	const char *kversion = NULL;
@@ -437,7 +437,8 @@  static int do_modinfo(int argc, char *ar
  	}

  	ctx = kmod_new(dirname, &null_config);
-	if (!ctx) {
+	ctx0 = kmod_new("/dev/null", &null_config);
+	if (!ctx || !ctx0) {
  		ERR("kmod_new() failed!\n");
  		return EXIT_FAILURE;
  	}
@@ -448,7 +449,7 @@  static int do_modinfo(int argc, char *ar
  		int r;

  		if (is_module_filename(name))
-			r = modinfo_path_do(ctx, name);
+			r = modinfo_path_do(ctx0, name);
  		else
  			r = modinfo_alias_do(ctx, name);

@@ -456,6 +457,7 @@  static int do_modinfo(int argc, char *ar
  			err = r;
  	}

+	kmod_unref(ctx0);
  	kmod_unref(ctx);
  	return err >= 0 ? EXIT_SUCCESS : EXIT_FAILURE;
  }