[PATCH] fix modpost segfault for 64bit mipsel kernel

[PATCH] fix modpost segfault for 64bit mipsel kernel

[Atsushi Nemoto]

64bit mips has different r_info layout.  This patch fixes modpost
segfault for 64bit little endian mips kernel.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index cd00e9f..7846600 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -712,7 +712,13 @@ static void check_sec_ref(struct module 
 			r.r_offset = TO_NATIVE(rela->r_offset);
 			r.r_info   = TO_NATIVE(rela->r_info);
 			r.r_addend = TO_NATIVE(rela->r_addend);
+#if KERNEL_ELFCLASS == ELFCLASS64 && KERNEL_ELFDATA == ELFDATA2LSB
+			sym = elf->symtab_start +
+				(hdr->e_machine == EM_MIPS ?
+				 (Elf32_Word)r.r_info : ELF_R_SYM(r.r_info));
+#else
 			sym = elf->symtab_start + ELF_R_SYM(r.r_info);
+#endif
 			/* Skip special sections */
 			if (sym->st_shndx >= SHN_LORESERVE)
 				continue;

Re: [PATCH] fix modpost segfault for 64bit mipsel kernel

[Thiemo Seufer]

Atsushi Nemoto wrote:
> 64bit mips has different r_info layout.  This patch fixes modpost
> segfault for 64bit little endian mips kernel.
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index cd00e9f..7846600 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -712,7 +712,13 @@ static void check_sec_ref(struct module 
>  			r.r_offset = TO_NATIVE(rela->r_offset);
>  			r.r_info   = TO_NATIVE(rela->r_info);
>  			r.r_addend = TO_NATIVE(rela->r_addend);
> +#if KERNEL_ELFCLASS == ELFCLASS64 && KERNEL_ELFDATA == ELFDATA2LSB
> +			sym = elf->symtab_start +
> +				(hdr->e_machine == EM_MIPS ?
> +				 (Elf32_Word)r.r_info : ELF_R_SYM(r.r_info));
> +#else
>  			sym = elf->symtab_start + ELF_R_SYM(r.r_info);
> +#endif

This doesn't look right. ELF64_R_SYM/ELF64_R_TYPE should be fixed for
mips64 instead.


Thiemo

Re: [PATCH] fix modpost segfault for 64bit mipsel kernel

[Sam Ravnborg]

On Mon, Apr 17, 2006 at 09:00:39PM +0900, Atsushi Nemoto wrote:
> 64bit mips has different r_info layout.  This patch fixes modpost
> segfault for 64bit little endian mips kernel.
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index cd00e9f..7846600 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -712,7 +712,13 @@ static void check_sec_ref(struct module 
>  			r.r_offset = TO_NATIVE(rela->r_offset);
>  			r.r_info   = TO_NATIVE(rela->r_info);
>  			r.r_addend = TO_NATIVE(rela->r_addend);
> +#if KERNEL_ELFCLASS == ELFCLASS64 && KERNEL_ELFDATA == ELFDATA2LSB
> +			sym = elf->symtab_start +
> +				(hdr->e_machine == EM_MIPS ?
> +				 (Elf32_Word)r.r_info : ELF_R_SYM(r.r_info));
> +#else
>  			sym = elf->symtab_start + ELF_R_SYM(r.r_info);
> +#endif
>  			/* Skip special sections */
>  			if (sym->st_shndx >= SHN_LORESERVE)
>  				continue;

Hi Atsushi

So with 64bit mips with the above we read r.r_info with no
modifications. But the elf.h I have on my system looks like this:

/* How to extract and insert information held in the r_info field.  */

#define ELF32_R_SYM(val)                ((val) >> 8)
#define ELF32_R_TYPE(val)               ((val) & 0xff)
#define ELF32_R_INFO(sym, type)         (((sym) << 8) + ((type) & 0xff))

#define ELF64_R_SYM(i)                  ((i) >> 32)
#define ELF64_R_TYPE(i)                 ((i) & 0xffffffff)
#define ELF64_R_INFO(sym,type)          ((((Elf64_Xword) (sym)) << 32) + (type))

So the difference between 64bit and 32bit is only the number of right
shifts.

Maybe we need to do some simple range check also so it becomes:
	if ((sym->st_shndx >= SHN_LORESERVE) || (sym > elf->symtab_stop))
		continue;

Could you try this - and if you still see the SEGVAL then try to print
out the value of all members in 'r'.

You can also try to add '-g' to HOSTCFLAGS in toplevel makefile.
Then use make V=1 to see what arguments modpost is caleld with and call
it from the commandline or from a debugger.

	Sam

Re: [PATCH] fix modpost segfault for 64bit mipsel kernel

[Thiemo Seufer]

Thiemo Seufer wrote:
> Atsushi Nemoto wrote:
> > 64bit mips has different r_info layout.  This patch fixes modpost
> > segfault for 64bit little endian mips kernel.
> > 
> > Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> > 
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index cd00e9f..7846600 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -712,7 +712,13 @@ static void check_sec_ref(struct module 
> >  			r.r_offset = TO_NATIVE(rela->r_offset);
> >  			r.r_info   = TO_NATIVE(rela->r_info);
> >  			r.r_addend = TO_NATIVE(rela->r_addend);
> > +#if KERNEL_ELFCLASS == ELFCLASS64 && KERNEL_ELFDATA == ELFDATA2LSB
> > +			sym = elf->symtab_start +
> > +				(hdr->e_machine == EM_MIPS ?
> > +				 (Elf32_Word)r.r_info : ELF_R_SYM(r.r_info));
> > +#else
> >  			sym = elf->symtab_start + ELF_R_SYM(r.r_info);
> > +#endif
> 
> This doesn't look right. ELF64_R_SYM/ELF64_R_TYPE should be fixed for
> mips64 instead.

I should have read more carefully. The ELF_R_SYM seems to be correct, if
this patch makes it work fo you then the toolchain you use creates broken
(word-swapped ?) relocation entries for mips64el.


Thiemo

Re: [PATCH] fix modpost segfault for 64bit mipsel kernel

[Atsushi Nemoto]

On Mon, 17 Apr 2006 15:07:35 +0100, Thiemo Seufer <ths@networkno.de> wrote:
> I should have read more carefully. The ELF_R_SYM seems to be correct, if
> this patch makes it work fo you then the toolchain you use creates broken
> (word-swapped ?) relocation entries for mips64el.

Looking at following codes in glibc source
(sysdeps/mips/elf/ldsodefs.h), I thought r_info on 64bit mips needs
special handling.  Is not this structure used for 64bit kernel
modules?

typedef struct
{
  Elf32_Word    r_sym;		/* Symbol index */
  unsigned char r_ssym;		/* Special symbol for 2nd relocation */
  unsigned char r_type3;	/* 3rd relocation type */
  unsigned char r_type2;	/* 2nd relocation type */
  unsigned char r_type1;	/* 1st relocation type */
} _Elf64_Mips_R_Info;

typedef union
{
  Elf64_Xword	r_info_number;
  _Elf64_Mips_R_Info r_info_fields;
} _Elf64_Mips_R_Info_union;
...
typedef struct
{
  Elf64_Addr	r_offset;		/* Address */
  _Elf64_Mips_R_Info_union r_info;	/* Relocation type and symbol index */
  Elf64_Sxword	r_addend;		/* Addend */
} Elf64_Mips_Rela;

#define ELF64_MIPS_R_SYM(i) \
  ((__extension__ (_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_sym)
...
#undef ELF64_R_SYM
#define ELF64_R_SYM(i) ELF64_MIPS_R_SYM (i)

---
Atsushi Nemoto

Re: [PATCH] fix modpost segfault for 64bit mipsel kernel

[Atsushi Nemoto]

On Mon, 17 Apr 2006 15:40:05 +0200, Sam Ravnborg <sam@ravnborg.org> wrote:
> Maybe we need to do some simple range check also so it becomes:
> 	if ((sym->st_shndx >= SHN_LORESERVE) || (sym > elf->symtab_stop))
> 		continue;
> 
> Could you try this - and if you still see the SEGVAL then try to print
> out the value of all members in 'r'.
> 
> You can also try to add '-g' to HOSTCFLAGS in toplevel makefile.
> Then use make V=1 to see what arguments modpost is caleld with and call
> it from the commandline or from a debugger.

When segfault happened, 'sym' has an invalid value.  Here is gdb session log:

$ gdb scripts/mod/modpost
GNU gdb 6.3-debian
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "i386-linux"...Using host libthread_db library "/lib/tls/libthread_db.so.1".

(gdb) run -m -a -o /home/git/build-swarm-le/Module.symvers   vmlinux crypto/aes.o crypto/anubis.o crypto/arc4.o crypto/blowfish.o crypto/cast5.o crypto/cast6.o crypto/crc32c.o crypto/crypto_null.o crypto/deflate.o crypto/des.o crypto/khazad.o crypto/md4.o crypto/md5.o crypto/michael_mic.o crypto/serpent.o crypto/sha1.o crypto/sha256.o crypto/sha512.o crypto/tea.o crypto/tgr192.o crypto/twofish.o crypto/wp512.o drivers/base/firmware_class.o drivers/block/aoe/aoe.o drivers/block/pktcdvd.o drivers/connector/cn.o drivers/input/serio/serio_raw.o drivers/net/phy/cicada.o drivers/net/phy/davicom.o drivers/net/phy/libphy.o drivers/net/phy/lxt.o drivers/net/phy/marvell.o drivers/net/phy/qsemi.o fs/fuse/fuse.o lib/crc16.o lib/libcrc32c.o lib/zlib_deflate/zlib_deflate.o lib/zlib_inflate/zlib_inflate.o net/ieee80211/ieee80211.o net/ieee80211/ieee80211_crypt.o net/ieee80211/ieee80211_crypt_ccmp.o net/ieee80211/ieee80211_crypt_wep.o net/xfrm/xfrm_user.o
Starting program: /home/git/build-swarm-le/scripts/mod/modpost -m -a -o /home/git/build-swarm-le/Module.symvers   vmlinux crypto/aes.o crypto/anubis.o crypto/arc4.o crypto/blowfish.o crypto/cast5.o crypto/cast6.o crypto/crc32c.o crypto/crypto_null.o crypto/deflate.o crypto/des.o crypto/khazad.o crypto/md4.o crypto/md5.o crypto/michael_mic.o crypto/serpent.o crypto/sha1.o crypto/sha256.o crypto/sha512.o crypto/tea.o crypto/tgr192.o crypto/twofish.o crypto/wp512.o drivers/base/firmware_class.o drivers/block/aoe/aoe.o drivers/block/pktcdvd.o drivers/connector/cn.o drivers/input/serio/serio_raw.o drivers/net/phy/cicada.o drivers/net/phy/davicom.o drivers/net/phy/libphy.o drivers/net/phy/lxt.o drivers/net/phy/marvell.o drivers/net/phy/qsemi.o fs/fuse/fuse.o lib/crc16.o lib/libcrc32c.o lib/zlib_deflate/zlib_deflate.o lib/zlib_inflate/zlib_inflate.o net/ieee80211/ieee80211.o net/ieee80211/ieee80211_crypt.o net/ieee80211/ieee80211_crypt_ccmp.o net/ieee80211/ieee80211_crypt_wep.o net/xfrm/xfrm_user.o

Program received signal SIGSEGV, Segmentation fault.
check_sec_ref (mod=0x806a2d8, modname=0xbffff79a "crypto/aes.o",
    elf=0xbffff230, section=0x804a050 <init_section>,
    section_ref_ok=0x804a0b0 <init_section_ref_ok>)
    at /home/git/linux-mips/scripts/mod/modpost.c:717
717                             if (sym->st_shndx >= SHN_LORESERVE)
(gdb) p/x sym
$1 = 0xf8161a58
(gdb) p/x elf->symtab_start
$2 = 0x40161a58
(gdb) p/x r
$3 = {r_offset = 0x1a4, r_info = 0x1d00000000000004, r_addend = 0x2028}


I suppose r_info have is following layout on 64bit mips:

typedef struct
{
  Elf32_Word    r_sym;		/* Symbol index */
  unsigned char r_ssym;		/* Special symbol for 2nd relocation */
  unsigned char r_type3;	/* 3rd relocation type */
  unsigned char r_type2;	/* 2nd relocation type */
  unsigned char r_type1;	/* 1st relocation type */
} _Elf64_Mips_R_Info;

For 64bit big-endian mips, r_info was byte-swapped by TO_NATIVE, so I
can use standard ELF_R_SYM().

---
Atsushi Nemoto

Re: [PATCH] fix modpost segfault for 64bit mipsel kernel

[Thiemo Seufer]

Atsushi Nemoto wrote:
> On Mon, 17 Apr 2006 15:07:35 +0100, Thiemo Seufer <ths@networkno.de> wrote:
> > I should have read more carefully. The ELF_R_SYM seems to be correct, if
> > this patch makes it work fo you then the toolchain you use creates broken
> > (word-swapped ?) relocation entries for mips64el.
> 
> Looking at following codes in glibc source
> (sysdeps/mips/elf/ldsodefs.h), I thought r_info on 64bit mips needs
> special handling.  Is not this structure used for 64bit kernel
> modules?
> 
> typedef struct
> {
>   Elf32_Word    r_sym;		/* Symbol index */
>   unsigned char r_ssym;		/* Special symbol for 2nd relocation */
>   unsigned char r_type3;	/* 3rd relocation type */
>   unsigned char r_type2;	/* 2nd relocation type */
>   unsigned char r_type1;	/* 1st relocation type */
> } _Elf64_Mips_R_Info;


Hm, binutils uses generically 64bit quantities:

#define ELF32_R_SYM(i)		((i) >> 8)
#define ELF32_R_TYPE(i)		((i) & 0xff)
#define ELF32_R_INFO(s,t)	(((s) << 8) + ((t) & 0xff))

#define ELF64_R_SYM(i)		((i) >> 32)
#define ELF64_R_TYPE(i)		((i) & 0xffffffff)
#define ELF64_R_INFO(s,t)	(((bfd_vma) (s) << 31 << 1) + (bfd_vma) (t))


But for MIPS64 the same as glibc:

typedef struct
{
  /* Address of relocation.  */
  unsigned char r_offset[8];
  /* Symbol index.  */
  unsigned char r_sym[4];
  /* Special symbol.  */
  unsigned char r_ssym[1];
  /* Third relocation.  */
  unsigned char r_type3[1];
  /* Second relocation.  */
  unsigned char r_type2[1];
  /* First relocation.  */
  unsigned char r_type[1];
  /* Addend.  */
  unsigned char r_addend[8];
} Elf64_Mips_External_Rela;

/* MIPS ELF 64 relocation info access macros.  */
#define ELF64_MIPS_R_SSYM(i) (((i) >> 24) & 0xff)
#define ELF64_MIPS_R_TYPE3(i) (((i) >> 16) & 0xff)
#define ELF64_MIPS_R_TYPE2(i) (((i) >> 8) & 0xff)
#define ELF64_MIPS_R_TYPE(i) ((i) & 0xff)


So it is the

      r.r_info   = TO_NATIVE(rela->r_info);

in modpost.c which breaks both SYM and TYPE because it assumes a
64bit integer. The proper solution would be to add a Elf64_Mips_Rela
structure (with lots of nearly identical duplicated code), the hack
would be to cast r_info to a 32bit integer for mips, before feeding
it to TO_NATIVE (which works until somebody asks for the TYPE, then
a separate mips64 version becomes inevitable.)


Thiemo

Re: [PATCH] fix modpost segfault for 64bit mipsel kernel

[Atsushi Nemoto]

On Mon, 17 Apr 2006 17:27:42 +0100, Thiemo Seufer <ths@networkno.de> wrote:
> So it is the
> 
>       r.r_info   = TO_NATIVE(rela->r_info);
> 
> in modpost.c which breaks both SYM and TYPE because it assumes a
> 64bit integer. The proper solution would be to add a Elf64_Mips_Rela
> structure (with lots of nearly identical duplicated code), the hack
> would be to cast r_info to a 32bit integer for mips, before feeding
> it to TO_NATIVE (which works until somebody asks for the TYPE, then
> a separate mips64 version becomes inevitable.)

I'd like to fix in _proper_ way.  Please review.  Thanks.


64bit mips has different r_info layout.  This patch fixes modpost
segfault for 64bit little endian mips kernel.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index cd00e9f..4ce95c6 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -710,7 +710,20 @@ static void check_sec_ref(struct module 
 			Elf_Rela r;
 			const char *secname;
 			r.r_offset = TO_NATIVE(rela->r_offset);
+#if KERNEL_ELFCLASS == ELFCLASS64
+			if (hdr->e_machine == EM_MIPS) {
+				unsigned int r_sym =
+					ELF64_MIPS_R_SYM(rela->r_info);
+				unsigned int r_type =
+					ELF64_MIPS_R_TYPE(rela->r_info);
+				r.r_info = ELF_R_INFO(TO_NATIVE(r_sym),
+						      TO_NATIVE(r_type));
+			} else {
+				r.r_info = TO_NATIVE(rela->r_info);
+			}
+#else
 			r.r_info   = TO_NATIVE(rela->r_info);
+#endif
 			r.r_addend = TO_NATIVE(rela->r_addend);
 			sym = elf->symtab_start + ELF_R_SYM(r.r_info);
 			/* Skip special sections */
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index b14255c..7d1c04d 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -24,6 +24,7 @@
 #define Elf_Rela    Elf32_Rela
 #define ELF_R_SYM   ELF32_R_SYM
 #define ELF_R_TYPE  ELF32_R_TYPE
+#define ELF_R_INFO  ELF32_R_INFO
 #else
 
 #define Elf_Ehdr    Elf64_Ehdr
@@ -37,8 +38,43 @@
 #define Elf_Rela    Elf64_Rela
 #define ELF_R_SYM   ELF64_R_SYM
 #define ELF_R_TYPE  ELF64_R_TYPE
+#define ELF_R_INFO  ELF64_R_INFO
 #endif
 
+/* The 64-bit MIPS ELF ABI uses an unusual reloc format. */
+typedef struct
+{
+  Elf32_Word    r_sym;		/* Symbol index */
+  unsigned char r_ssym;		/* Special symbol for 2nd relocation */
+  unsigned char r_type3;	/* 3rd relocation type */
+  unsigned char r_type2;	/* 2nd relocation type */
+  unsigned char r_type1;	/* 1st relocation type */
+} _Elf64_Mips_R_Info;
+
+typedef union
+{
+  Elf64_Xword	r_info_number;
+  _Elf64_Mips_R_Info r_info_fields;
+} _Elf64_Mips_R_Info_union;
+
+typedef struct
+{
+  Elf64_Addr	r_offset;		/* Address */
+  _Elf64_Mips_R_Info_union r_info;	/* Relocation type and symbol index */
+  Elf64_Sxword	r_addend;		/* Addend */
+} Elf64_Mips_Rela;
+
+#define ELF64_MIPS_R_SYM(i) \
+  ((__extension__ (_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_sym)
+#define ELF64_MIPS_R_TYPE(i) \
+  (((_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_type1 \
+   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
+		   ).r_info_fields.r_type2 << 8) \
+   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
+		   ).r_info_fields.r_type3 << 16) \
+   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
+		   ).r_info_fields.r_ssym << 24))
+
 #if KERNEL_ELFDATA != HOST_ELFDATA
 
 static inline void __endian(const void *src, void *dest, unsigned int size)

Re: [PATCH] fix modpost segfault for 64bit mipsel kernel

[Thiemo Seufer]

Atsushi Nemoto wrote:
> On Mon, 17 Apr 2006 17:27:42 +0100, Thiemo Seufer <ths@networkno.de> wrote:
> > So it is the
> > 
> >       r.r_info   = TO_NATIVE(rela->r_info);
> > 
> > in modpost.c which breaks both SYM and TYPE because it assumes a
> > 64bit integer. The proper solution would be to add a Elf64_Mips_Rela
> > structure (with lots of nearly identical duplicated code), the hack
> > would be to cast r_info to a 32bit integer for mips, before feeding
> > it to TO_NATIVE (which works until somebody asks for the TYPE, then
> > a separate mips64 version becomes inevitable.)
> 
> I'd like to fix in _proper_ way.  Please review.  Thanks.
> 
> 
> 64bit mips has different r_info layout.  This patch fixes modpost
> segfault for 64bit little endian mips kernel.
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> 
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index cd00e9f..4ce95c6 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -710,7 +710,20 @@ static void check_sec_ref(struct module 
>  			Elf_Rela r;
>  			const char *secname;
>  			r.r_offset = TO_NATIVE(rela->r_offset);
> +#if KERNEL_ELFCLASS == ELFCLASS64
> +			if (hdr->e_machine == EM_MIPS) {
> +				unsigned int r_sym =
> +					ELF64_MIPS_R_SYM(rela->r_info);
> +				unsigned int r_type =
> +					ELF64_MIPS_R_TYPE(rela->r_info);
> +				r.r_info = ELF_R_INFO(TO_NATIVE(r_sym),
> +						      TO_NATIVE(r_type));
[snip]
> +/* The 64-bit MIPS ELF ABI uses an unusual reloc format. */
> +typedef struct
> +{
> +  Elf32_Word    r_sym;		/* Symbol index */
> +  unsigned char r_ssym;		/* Special symbol for 2nd relocation */
> +  unsigned char r_type3;	/* 3rd relocation type */
> +  unsigned char r_type2;	/* 2nd relocation type */
> +  unsigned char r_type1;	/* 1st relocation type */
> +} _Elf64_Mips_R_Info;
[snip]
> +#define ELF64_MIPS_R_TYPE(i) \
> +  (((_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_type1 \
> +   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
> +		   ).r_info_fields.r_type2 << 8) \
> +   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
> +		   ).r_info_fields.r_type3 << 16) \
> +   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
> +		   ).r_info_fields.r_ssym << 24))

Why is it the right thing to combine the type info into a 32bit word?
It will never get used as such for MIPS ELF64. I would have expected
something like:

#define ELF64_MIPS_R_INFO(sym,ssym,t3,t2,t1)		\
{(							\
	_Elf64_Mips_R_Info info = {			\
		.r_sym = sym,				\
		.r_ssym = ssym,				\
		.r_type3 = t3,				\
		.r_type2 = t2,				\
		.r_type1 = t1,				\
	}						\
	(Elf64_Xword)info;				\
)}

without a corresponding ELF64_MIPS_R_TYPE, and then:

	if (hdr->e_ident[EI_CLASS] == ELFCLASS64
	    && hdr->e_machine == EM_MIPS) {
		_Elf64_Mips_R_Info info = (_Elf64_Mips_R_Info)r.r_info;
		r.r_info = ELF64_MIPS_R_INFO(TO_NATIVE(info.r_sym),
					     info.r_ssym, info.r_type3,
					     info.r_type2, info.r_type1);
	}


Thiemo

Re: [PATCH] fix modpost segfault for 64bit mipsel kernel

[Atsushi Nemoto]

On Thu, 20 Apr 2006 01:19:00 +0100, Thiemo Seufer <ths@networkno.de> wrote:
> > +#define ELF64_MIPS_R_TYPE(i) \
> > +  (((_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_type1 \
> > +   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
> > +		   ).r_info_fields.r_type2 << 8) \
> > +   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
> > +		   ).r_info_fields.r_type3 << 16) \
> > +   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
> > +		   ).r_info_fields.r_ssym << 24))
> 
> Why is it the right thing to combine the type info into a 32bit word?

Well, I just take ELF64_MIPS_R_TYPE() from glibc source.

> It will never get used as such for MIPS ELF64. I would have expected
> something like:
> 
> #define ELF64_MIPS_R_INFO(sym,ssym,t3,t2,t1)		\
> {(							\
> 	_Elf64_Mips_R_Info info = {			\
> 		.r_sym = sym,				\
> 		.r_ssym = ssym,				\
> 		.r_type3 = t3,				\
> 		.r_type2 = t2,				\
> 		.r_type1 = t1,				\
> 	}						\
> 	(Elf64_Xword)info;				\
> )}
> 
> without a corresponding ELF64_MIPS_R_TYPE, and then:
> 
> 	if (hdr->e_ident[EI_CLASS] == ELFCLASS64
> 	    && hdr->e_machine == EM_MIPS) {
> 		_Elf64_Mips_R_Info info = (_Elf64_Mips_R_Info)r.r_info;
> 		r.r_info = ELF64_MIPS_R_INFO(TO_NATIVE(info.r_sym),
> 					     info.r_ssym, info.r_type3,
> 					     info.r_type2, info.r_type1);
> 	}

Sorry, I can not see what you mean ... it just does byte-swap only
r_sym part, doesn't it?  It is not enough because a position of r_sym
in MIPS ELF64 r_info is different from standard ELF64 r_info.

And I found my previous patch does unnecessary byte-swap on r_type.
This is a take 3.  It also checks hdr->e_ident[EI_CLASS] instead of
KERNEL_ELFCLASS.


64bit mips has different r_info layout.  This patch fixes modpost
segfault for 64bit little endian mips kernel.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index cd00e9f..8f5e814 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -710,7 +710,17 @@ static void check_sec_ref(struct module 
 			Elf_Rela r;
 			const char *secname;
 			r.r_offset = TO_NATIVE(rela->r_offset);
-			r.r_info   = TO_NATIVE(rela->r_info);
+			if (hdr->e_ident[EI_CLASS] == ELFCLASS64 &&
+			    hdr->e_machine == EM_MIPS) {
+				unsigned int r_sym =
+					ELF64_MIPS_R_SYM(rela->r_info);
+				unsigned int r_type =
+					ELF64_MIPS_R_TYPE(rela->r_info);
+				r.r_info = ELF_R_INFO(TO_NATIVE(r_sym),
+						      r_type);
+			} else {
+				r.r_info = TO_NATIVE(rela->r_info);
+			}
 			r.r_addend = TO_NATIVE(rela->r_addend);
 			sym = elf->symtab_start + ELF_R_SYM(r.r_info);
 			/* Skip special sections */
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index b14255c..7d1c04d 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -24,6 +24,7 @@
 #define Elf_Rela    Elf32_Rela
 #define ELF_R_SYM   ELF32_R_SYM
 #define ELF_R_TYPE  ELF32_R_TYPE
+#define ELF_R_INFO  ELF32_R_INFO
 #else
 
 #define Elf_Ehdr    Elf64_Ehdr
@@ -37,8 +38,43 @@
 #define Elf_Rela    Elf64_Rela
 #define ELF_R_SYM   ELF64_R_SYM
 #define ELF_R_TYPE  ELF64_R_TYPE
+#define ELF_R_INFO  ELF64_R_INFO
 #endif
 
+/* The 64-bit MIPS ELF ABI uses an unusual reloc format. */
+typedef struct
+{
+  Elf32_Word    r_sym;		/* Symbol index */
+  unsigned char r_ssym;		/* Special symbol for 2nd relocation */
+  unsigned char r_type3;	/* 3rd relocation type */
+  unsigned char r_type2;	/* 2nd relocation type */
+  unsigned char r_type1;	/* 1st relocation type */
+} _Elf64_Mips_R_Info;
+
+typedef union
+{
+  Elf64_Xword	r_info_number;
+  _Elf64_Mips_R_Info r_info_fields;
+} _Elf64_Mips_R_Info_union;
+
+typedef struct
+{
+  Elf64_Addr	r_offset;		/* Address */
+  _Elf64_Mips_R_Info_union r_info;	/* Relocation type and symbol index */
+  Elf64_Sxword	r_addend;		/* Addend */
+} Elf64_Mips_Rela;
+
+#define ELF64_MIPS_R_SYM(i) \
+  ((__extension__ (_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_sym)
+#define ELF64_MIPS_R_TYPE(i) \
+  (((_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_type1 \
+   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
+		   ).r_info_fields.r_type2 << 8) \
+   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
+		   ).r_info_fields.r_type3 << 16) \
+   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
+		   ).r_info_fields.r_ssym << 24))
+
 #if KERNEL_ELFDATA != HOST_ELFDATA
 
 static inline void __endian(const void *src, void *dest, unsigned int size)

Re: [PATCH] fix modpost segfault for 64bit mipsel kernel

[Thiemo Seufer]

Atsushi Nemoto wrote:
> On Thu, 20 Apr 2006 01:19:00 +0100, Thiemo Seufer <ths@networkno.de> wrote:
> > > +#define ELF64_MIPS_R_TYPE(i) \
> > > +  (((_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_type1 \
> > > +   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
> > > +		   ).r_info_fields.r_type2 << 8) \
> > > +   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
> > > +		   ).r_info_fields.r_type3 << 16) \
> > > +   | ((Elf32_Word)(__extension__ (_Elf64_Mips_R_Info_union)(i) \
> > > +		   ).r_info_fields.r_ssym << 24))
> > 
> > Why is it the right thing to combine the type info into a 32bit word?
> 
> Well, I just take ELF64_MIPS_R_TYPE() from glibc source.

It is not more useful in glibc. :-)  Any use of the TYPE data will have
to take the MIPS64 specifics in account, and thus split it up again
into single characters.

> > It will never get used as such for MIPS ELF64. I would have expected
> > something like:
> > 
> > #define ELF64_MIPS_R_INFO(sym,ssym,t3,t2,t1)		\
> > {(							\
> > 	_Elf64_Mips_R_Info info = {			\
> > 		.r_sym = sym,				\
> > 		.r_ssym = ssym,				\
> > 		.r_type3 = t3,				\
> > 		.r_type2 = t2,				\
> > 		.r_type1 = t1,				\
> > 	}						\
> > 	(Elf64_Xword)info;				\
> > )}
> > 
> > without a corresponding ELF64_MIPS_R_TYPE, and then:
> > 
> > 	if (hdr->e_ident[EI_CLASS] == ELFCLASS64
> > 	    && hdr->e_machine == EM_MIPS) {
> > 		_Elf64_Mips_R_Info info = (_Elf64_Mips_R_Info)r.r_info;
> > 		r.r_info = ELF64_MIPS_R_INFO(TO_NATIVE(info.r_sym),
> > 					     info.r_ssym, info.r_type3,
> > 					     info.r_type2, info.r_type1);
> > 	}
> 
> Sorry, I can not see what you mean ... it just does byte-swap only
> r_sym part, doesn't it?  It is not enough because a position of r_sym
> in MIPS ELF64 r_info is different from standard ELF64 r_info.

But it does so for a _Elf64_Mips_R_Info struct, which already keeps
the position of its r_sym field endianness independent.


Thiemo

Re: [PATCH] fix modpost segfault for 64bit mipsel kernel

[Atsushi Nemoto]

On Thu, 20 Apr 2006 17:23:19 +0100, Thiemo Seufer <ths@networkno.de> wrote:
> > Well, I just take ELF64_MIPS_R_TYPE() from glibc source.
> 
> It is not more useful in glibc. :-)  Any use of the TYPE data will have
> to take the MIPS64 specifics in account, and thus split it up again
> into single characters.

OK, then I drop TYPE data and simplify the patch.  Take 4.


64bit mips has different r_info layout.  This patch fixes modpost
segfault for 64bit little endian mips kernel.

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index cd00e9f..fcd4306 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -709,10 +709,17 @@ static void check_sec_ref(struct module 
 		for (rela = start; rela < stop; rela++) {
 			Elf_Rela r;
 			const char *secname;
+			unsigned int r_sym;
 			r.r_offset = TO_NATIVE(rela->r_offset);
-			r.r_info   = TO_NATIVE(rela->r_info);
+			if (hdr->e_ident[EI_CLASS] == ELFCLASS64 &&
+			    hdr->e_machine == EM_MIPS) {
+				r_sym = ELF64_MIPS_R_SYM(rela->r_info);
+				r_sym = TO_NATIVE(r_sym);
+			} else {
+				r_sym = ELF_R_SYM(TO_NATIVE(rela->r_info));
+			}
 			r.r_addend = TO_NATIVE(rela->r_addend);
-			sym = elf->symtab_start + ELF_R_SYM(r.r_info);
+			sym = elf->symtab_start + r_sym;
 			/* Skip special sections */
 			if (sym->st_shndx >= SHN_LORESERVE)
 				continue;
diff --git a/scripts/mod/modpost.h b/scripts/mod/modpost.h
index b14255c..89b96c6 100644
--- a/scripts/mod/modpost.h
+++ b/scripts/mod/modpost.h
@@ -39,6 +39,25 @@
 #define ELF_R_TYPE  ELF64_R_TYPE
 #endif
 
+/* The 64-bit MIPS ELF ABI uses an unusual reloc format. */
+typedef struct
+{
+  Elf32_Word    r_sym;		/* Symbol index */
+  unsigned char r_ssym;		/* Special symbol for 2nd relocation */
+  unsigned char r_type3;	/* 3rd relocation type */
+  unsigned char r_type2;	/* 2nd relocation type */
+  unsigned char r_type1;	/* 1st relocation type */
+} _Elf64_Mips_R_Info;
+
+typedef union
+{
+  Elf64_Xword	r_info_number;
+  _Elf64_Mips_R_Info r_info_fields;
+} _Elf64_Mips_R_Info_union;
+
+#define ELF64_MIPS_R_SYM(i) \
+  ((__extension__ (_Elf64_Mips_R_Info_union)(i)).r_info_fields.r_sym)
+
 #if KERNEL_ELFDATA != HOST_ELFDATA
 
 static inline void __endian(const void *src, void *dest, unsigned int size)

Re: [PATCH] fix modpost segfault for 64bit mipsel kernel

[Thiemo Seufer]

Atsushi Nemoto wrote:
> On Thu, 20 Apr 2006 17:23:19 +0100, Thiemo Seufer <ths@networkno.de> wrote:
> > > Well, I just take ELF64_MIPS_R_TYPE() from glibc source.
> > 
> > It is not more useful in glibc. :-)  Any use of the TYPE data will have
> > to take the MIPS64 specifics in account, and thus split it up again
> > into single characters.
> 
> OK, then I drop TYPE data and simplify the patch.  Take 4.

Looks good to me, FWIW.


Thiemo