Kernel module version support.

Kernel module version support.

[Wang, Stanley]

Hi, Rusty
I am interested in your module version support implementation. I've read
your 
description about it.
(http://www.kernel.org/pub/linux/kernel/people/rusty/modversions_support.htm
l)
And I have some questions about the implementation details. Would you like
to help me to 
clarify them?

1. How do you plan to store the version information of a kernel module that
will export some symbols?
(In the version table of "bzImage"? In a specified section in this kernel
module? In other place? Or don't
store?)

2. You mentioned that "modules which want to export symbols place their full
path name 
in the .needmodversion section. Just before the kernel is linked, these
names are extracted, 
and genksyms scans those files to create a version table. This table is then
linked into the kernel". 
And I think we must recalculate all version informaiton every time when we
re built the kernel in this way. 
Why don't we place all the module version information in some files just
like old days.

3. You mentioned that "these symbol versions are then checked on insmod". I
wanna whether it means
you would like to restore the "/proc/ksyms" file or QUERY_MODULE SYSCALL to
export the kernel version
table to the user space application. 

Thanks a lot.

Your Sincerely,
Stanley Wang 

SW Engineer, Intel Corporation.
Intel China Software Lab. 
Tel: 021-52574545 ext. 1171 
iNet: 8-752-1171 
 
Opinions expressed are those of the author and do not represent Intel
Corporation

Re: Kernel module version support.

[Rusty Russell]

In message <957BD1C2BF3CD411B6C500A0C944CA2602AFADC0@pdsmsx32.pd.intel.com> you
 write:
> Hi, Rusty
> I am interested in your module version support implementation. I've read
> your description about it.

Cool.  That reminds me.

> 1. How do you plan to store the version information of a kernel module that
> will export some symbols?
> (In the version table of "bzImage"? In a specified section in this kernel
> module? In other place? Or don't
> store?)

Currently I don't do this.  I discussed solutions briefly with Kai
some time ago, but I'll have to revisit it.  Getting the core kernel
symbols versioned covers 90% of the problem though, and shows the
validity of the approach.

> 2. You mentioned that "modules which want to export symbols place
> their full path name in the .needmodversion section. Just before the
> kernel is linked, these names are extracted, and genksyms scans
> those files to create a version table. This table is then linked
> into the kernel".  And I think we must recalculate all version
> informaiton every time when we re built the kernel in this way.  Why
> don't we place all the module version information in some files just
> like old days.

That's a build system detail: it can use that list to generate the
versions on demand of course.

> 3. You mentioned that "these symbol versions are then checked on
> insmod". I wanna whether it means you would like to restore the
> "/proc/ksyms" file or QUERY_MODULE SYSCALL to export the kernel
> version table to the user space application.

The language is a little loose.  The kernel does the actual checking:

#ifdef CONFIG_MODVERSIONING
static inline int check_version(const char *name,
				const char *symname,
				const struct modversion_info *versions,
				unsigned int num)
{
	unsigned int i, k;

	/* First search kernel (unversioned symbols not listed). */
	for (k = 0; !streq(modversions[k].symbol, symname); k++)
		if (!modversions[k].symbol[0])
			return 0;

	/* Now see if module has matching symbol. */
	for (i = 0; i < num; i++) {
		if (streq(versions[i].symbol, symname)) {
			if (versions[i].checksum == modversions[k].checksum)
				return 0;
			printk("%s: disagrees about version of symbol %s\n",
			       name, symname);
			DEBUGP("Kernel checksum %lX vs module %lX\n",
			       modversions[k].checksum, versions[i].checksum);
			return -ESRCH;
		}
	}

	/* Not in module's version table.  OK, but that taints the kernel. */
	printk("%s: no version for %s found: kernel tainted.\n",
	       name, symname);
	tainted |= TAINT_FORCED_MODULE;
	return 0;
}

Hope that clarifies,
Rusty.
PS.  I'll update the patch soon, I promise...
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

RE: Kernel module version support.

[Wang, Stanley]

Hi Rusty
Thanks for your rapid responding.

And as you are the maintainer of kernel module support, I would like to know
how
you think about export some APIs for geting a specified module structure's
pointer.
Just like:
struct module *get_module(const char *name)
{
	struct module *mod;
	down(&module_mutex);
	mod = find_module(name);
	up(&module_mutex);
	return mod;
}
EXPORT_SYMBOL_GPL(get_module);

This function will be useful when we use Kprobes to place probes into a
kernel module.
We could get the base address of the module's .text section througn this
module structure's
pointer, hence we could place the kernel probe on any instruction we wanted.
  
Thanks a lot.

Your Sincerely,
Stanley Wang 

SW Engineer, Intel Corporation.
Intel China Software Lab. 
Tel: 021-52574545 ext. 1171 
iNet: 8-752-1171 
 
Opinions expressed are those of the author and do not represent Intel
Corporation

Re: Kernel module version support.

[Rusty Russell]

In message <957BD1C2BF3CD411B6C500A0C944CA2601F1170F@pdsmsx32.pd.intel.com> you
 write:
> Hi Rusty
> Thanks for your rapid responding.
> 
> And as you are the maintainer of kernel module support, I would like to know
> how
> you think about export some APIs for geting a specified module structure's
> pointer.
> Just like:
> struct module *get_module(const char *name)
> {
> 	struct module *mod;
> 	down(&module_mutex);
> 	mod = find_module(name);
> 	up(&module_mutex);
> 	return mod;
> }
> EXPORT_SYMBOL_GPL(get_module);

Well, you also need:
      if (mod && !try_module_get(mod))
		mod = NULL; /* Not ready, or vanishing.  Ignore. */

Just before the "up(&module_mutex);".

I'm not convinced you don't want some other, more powerful interface,
though, like "get_ksymbol_address()" or something.

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: Kernel module version support.

[Rusty Russell]

In message <957BD1C2BF3CD411B6C500A0C944CA2601F11711@pdsmsx32.pd.intel.com> you
 write:
> Hi, Rusty

> There is a example that could explain why I want the module structure's
> pointer.

> If we want to place kernel probes on all PIO instrcutions of a
> device driver for debuging purpose, only knowing symbol's address is
> not enough. We need the base address of .text section.  How do you
> think about this example ?

I don't know where the .text section is anymore, once the module is
loaded.  And just the .text section might not be enough on some archs.

I think it would be cleaner to have a userspace program which parses
the module, figures out how it is laid out in memory (this will be
arch specific!) and then (using the base address from /proc/modules)
tells the kernel "insert a probe at address 0xc1234567".  This should
be far more flexible, I think.

Thoughts?
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: Kernel module version support.

[Rusty Russell]

In message <957BD1C2BF3CD411B6C500A0C944CA2601F11712@pdsmsx32.pd.intel.com> you
> I agree with you. It's a more elegant way:)
> But we couldn't get the module's base address from /proc/modules.=20
> And could you print "mod->core" in /proc/modules?(Just like the =
> following
> patch)

You need to following patch first: the current /proc/modules is not
extensible, so adding a field breaks things.  This changes the
/proc/modules format, and then your patch is as I planned.

Cheers!
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Name: /proc/modules rework
Author: Rusty Russell
Status: Tested on 2.5.52

D: This changes the dependency list in /proc/modules to be one field,
D: rather than a space-separated set of dependencies, and makes sure
D: the contents are there even if CONFIG_MODULE_UNLOAD is off.
D:
D: This allows us to append fields (eg. the module address for
D: oprofile, and the module status) without breaking userspace.
D: module-init-tools already handles this format (which you can detect
D: by the presence of "-" or "," in the fourth field).

diff -urpN --exclude TAGS -X /home/rusty/devel/kernel/kernel-patches/current-dontdiff --minimal .17749-linux-2.5-bk/kernel/module.c .17749-linux-2.5-bk.updated/kernel/module.c
--- .17749-linux-2.5-bk/kernel/module.c	2003-01-02 14:48:01.000000000 +1100
+++ .17749-linux-2.5-bk.updated/kernel/module.c	2003-01-03 19:12:01.000000000 +1100
@@ -481,19 +481,29 @@ sys_delete_module(const char *name_user,
 static void print_unload_info(struct seq_file *m, struct module *mod)
 {
 	struct module_use *use;
+	int printed_something = 0;
 
-	seq_printf(m, " %u", module_refcount(mod));
+	seq_printf(m, " %u ", module_refcount(mod));
 
-	list_for_each_entry(use, &mod->modules_which_use_me, list)
-		seq_printf(m, " %s", use->module_which_uses->name);
+	/* Always include a trailing , so userspace can differentiate
+           between this and the old multi-field proc format. */
+	list_for_each_entry(use, &mod->modules_which_use_me, list) {
+		printed_something = 1;
+		seq_printf(m, "%s,", use->module_which_uses->name);
+	}
 
-	if (mod->unsafe)
-		seq_printf(m, " [unsafe]");
+	if (mod->unsafe) {
+		printed_something = 1;
+		seq_printf(m, "[unsafe],");
+	}
 
 	if (mod->init != init_module && mod->exit == cleanup_module)
-		seq_printf(m, " [permanent]");
+		printed_something = 1;
+		seq_printf(m, "[permanent],");
+	}
 
-	seq_printf(m, "\n");
+	if (!printed_something)
+		seq_printf(m, "-");
 }
 
 void __symbol_put(const char *symbol)
@@ -534,7 +544,8 @@ EXPORT_SYMBOL_GPL(symbol_put_addr);
 #else /* !CONFIG_MODULE_UNLOAD */
 static void print_unload_info(struct seq_file *m, struct module *mod)
 {
-	seq_printf(m, "\n");
+	/* We don't know the usage count, or what modules are using. */
+	seq_printf(m, " - -");
 }
 
 static inline void module_unload_free(struct module *mod)
@@ -1370,6 +1381,7 @@ static int m_show(struct seq_file *m, vo
 	seq_printf(m, "%s %lu",
 		   mod->name, mod->init_size + mod->core_size);
 	print_unload_info(m, mod);
+	seq_printf(m, "\n");
 	return 0;
 }
 struct seq_operations modules_op = {