Re: Early crash (was: Re: module: show version information for built-in modules in sysfs)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Feb 02, 2011 at 06:48:50AM -0800, Geert Uytterhoeven wrote:
On Tue, Feb 1, 2011 at 23:26, Dmitry Torokhov <dtor@xxxxxxxxxx> wrote:
On Tue, Feb 01, 2011 at 02:03:23PM -0800, Geert Uytterhoeven wrote:
On Tue, Feb 1, 2011 at 22:09, Dmitry Torokhov <dtor@xxxxxxxxxx> wrote:
On Tue, Feb 01, 2011 at 12:33:29PM -0800, Geert Uytterhoeven wrote:
On Mon, Jan 24, 2011 at 11:59, Linux Kernel Mailing List
<linux-kernel@xxxxxxxxxxxxxxx> wrote:
Gitweb:     http://git.kernel.org/linus/e94965ed5beb23c6fabf7ed31f625e66d7ff28de

   module: show version information for built-in modules in sysfs

   Currently only drivers that are built as modules have their versions
   shown in /sys/module/<module_name>/version, but this information might
   also be useful for built-in drivers as well. This especially important
   for drivers that do not define any parameters - such drivers, if
   built-in, are completely invisible from userspace.

   This patch changes MODULE_VERSION() macro so that in case when we are
   compiling built-in module, version information is stored in a separate
   section. Kernel then uses this data to create 'version' sysfs attribute
   in the same fashion it creates attributes for module parameters.

This commit causes the crash below on m68k (ARAnyM).
Reverting this commit and its dependency
3b90a5b292321b2acac3921f77046ae195aef53f
("module: fix linker error for MODULE_VERSION when !MODULE and CONFIG_SYSFS=n")
makes it boot again.


Hi Geert,

Does the follwing help by any chance?

From d6fd4a6e0fc2d3f0a74962d4a6f663a46d230ecd Mon Sep 17 00:00:00 2001
diff --git a/arch/m68knommu/kernel/vmlinux.lds.S b/arch/m68knommu/kernel/vmlinux.lds.S
index ef33213..47e15eb 100644
--- a/arch/m68knommu/kernel/vmlinux.lds.S
+++ b/arch/m68knommu/kernel/vmlinux.lds.S

The crash happened on m68k with MMU, not m68knommu.


Hmm, OK then. Could you please see if the crash happens if you return
early in kernel/params.c::version_sysfs_builtin() ? Also, do you see

It does not crash if version_sysfs_builtin() returns early.

anything in __modev section of your build?

"objdump -h" says:

| Sections:
| Idx Name          Size      VMA       LMA       File off  Algn
|   7 __modver      0000007c  002e0f84  002e0f84  002e0f84  2**2
|                   CONTENTS, ALLOC, LOAD, DATA

"nm vmlinux | grep __modver" says:

| 002e0f84 d __modver_version_attr
| 002e0fa8 d __modver_version_attr
| 00039026 T __modver_version_show
| 002e0f84 D __start___modver
| 002e0fca D __stop___modver

The section size (0x7c) is larger than __stop___modver -
__start___modver (0x46)?

Adding some debugging code (which increases the section size even more?) shows:

vattr = 002e1004
vattr->module_name = xz_dec
    mk = 00c2ee50
    err = 0
    kobject_uevent done
    kobject_put done
vattr = 002e1026
vattr->module_name = (null)
Unable to handle kernel NULL pointer dereference at virtual address 0000002c

So the second module in the list has no name. Why?
Aha, it's not NULL, but just < PAGE_SIZE (0x2c).

sizeof(struct module_version_attribute) = 34, which you can see from
the 2 consecutive vattr
pointers above. But the "aligned(sizeof(void *))" in the definition of
MODULE_VERSION() puts
the next module_version_attribute struct in the array at offset 36,
not offset 34!
On m68k, the alignment of 32-bit integrals is 2 bytes, not 4.

But why is it aligned on 2-byte boundary and why m64k is not happy with
module_version_attribute but is happy with kernel_param which is also
aligned similarly?

If we unroll module_version_attribute it woud look like this:

struct module_version_attribute {

	struct module_attribute {

		struct attribute {
			const char *name;
			mode_t mode;
		} attr;
		...

	} mattr;

	const char *module_name;
	const char *version;
};

So I would expect it be aligned on (char *) boundary which should be the
same as (void *).

Will it help if we rearrange module_version_attribute definition to
explicitly have first field being a pointer so it is more like
kernel_param, like this:

struct module_version_attribute {
	const char *module_name;
	const char *version;
	struct module_attribute mattr;
};

Thanks,
Dmitry

--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux