[RFC][PATCH]fix search_extable() to find correct entry

[RFC][PATCH]fix search_extable() to find correct entry

[Masami Hiramatsu]

Hi, Tony

I found a suspicious buggy code in the linux kernel on IA64 arch.
As far as I can see, search_extable() doesn't work correctly, because
the lookup routine expects that the address format of the
exception_table_entry is "IP + slot", but the compiler (gcc-3.4.5)
generates it as "IP + (slot << 2)". Thus the lookup routine always
fails to find the corresponding entry.
You can check it by dumping __ex_table section of vmlinux.

I made a patch to fix this bug attached in this mail. This patch is
against 2.6.17-rc6-mm2.
Please review it.

Description:
 Fix search_extable() and ia64_handle_exception() to handle the
address format of exception_table_entry correctly.

Thanks,

-- 
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: hiramatu@sdl.hitachi.co.jp

Signed-off-by: Masami Hiramatsu <hiramatu@sdl.hitachi.co.jp>

 arch/ia64/mm/extable.c     |    7 ++++---
 include/asm-ia64/uaccess.h |    4 ++--
 2 files changed, 6 insertions(+), 5 deletions(-)
diff --exclude=CVS -Narup a/arch/ia64/mm/extable.c b/arch/ia64/mm/extable.c
--- a/arch/ia64/mm/extable.c	2005-10-28 09:02:08.000000000 +0900
+++ b/arch/ia64/mm/extable.c	2006-06-14 14:59:11.000000000 +0900
@@ -63,9 +63,10 @@ search_extable (const struct exception_t
 	unsigned long mid_ip;
 	long diff;

+	ip = (ip & ~0xf) + ((ip & 0x3) << 2);
         while (first <= last) {
 		mid = &first[(last - first)/2];
-		mid_ip = (u64) &mid->addr + mid->addr;
+		mid_ip = ((u64) &mid->addr + mid->addr) & ~0x3;
 		diff = mid_ip - ip;
                 if (diff = 0)
                         return mid;
@@ -83,8 +84,8 @@ ia64_handle_exception (struct pt_regs *r
 	long fix = (u64) &e->cont + e->cont;

 	regs->r8 = -EFAULT;
-	if (fix & 4)
+	if (fix & 2)
 		regs->r9 = 0;
 	regs->cr_iip = fix & ~0xf;
-	ia64_psr(regs)->ri = fix & 0x3;		/* set continuation slot number */
+	ia64_psr(regs)->ri = (fix & 0xc) >> 2;		/* set continuation slot number */
 }
diff --exclude=CVS -Narup a/include/asm-ia64/uaccess.h b/include/asm-ia64/uaccess.h
--- a/include/asm-ia64/uaccess.h	2005-10-28 09:02:08.000000000 +0900
+++ b/include/asm-ia64/uaccess.h	2006-06-14 23:54:55.000000000 +0900
@@ -139,7 +139,7 @@ do {												\
 	register long __gu_r8 asm ("r8") = 0;							\
 	register long __gu_r9 asm ("r9");							\
 	asm ("\n[1:]\tld"#n" %0=%2%P2\t// %0 and %1 get overwritten by exception handler\n"	\
-	     "\t.xdata4 \"__ex_table\", 1b-., 1f-.+4\n"						\
+	     "\t.xdata4 \"__ex_table\", 1b-., 1f-.+2\n"						\
 	     "[1:]"										\
 	     : "=r"(__gu_r9), "=r"(__gu_r8) : "m"(__m(addr)), "1"(__gu_r8));			\
 	(err) = __gu_r8;									\
@@ -346,7 +346,7 @@ extern unsigned long __strnlen_user (con

 struct exception_table_entry {
 	int addr;	/* location-relative address of insn this fixup is for */
-	int cont;	/* location-relative continuation addr.; if bit 2 is set, r9 is set to 0 */
+	int cont;	/* location-relative continuation addr.; if bit 1 is set, r9 is set to 0 */
 };

 extern void ia64_handle_exception (struct pt_regs *regs, const struct exception_table_entry *e);

Re: [RFC][PATCH]fix search_extable() to find correct entry

[David Mosberger-Tang]

What about EXCLR() in asmmacro.h?  Doesn't that need to be updated as
well?  Did you verify that the equivalent isn't open-coded in any
assembly-file?

  --david

On 6/15/06, Masami Hiramatsu <hiramatu@sdl.hitachi.co.jp> wrote:
> Hi, Tony
>
> I found a suspicious buggy code in the linux kernel on IA64 arch.
> As far as I can see, search_extable() doesn't work correctly, because
> the lookup routine expects that the address format of the
> exception_table_entry is "IP + slot", but the compiler (gcc-3.4.5)
> generates it as "IP + (slot << 2)". Thus the lookup routine always
> fails to find the corresponding entry.
> You can check it by dumping __ex_table section of vmlinux.
>
> I made a patch to fix this bug attached in this mail. This patch is
> against 2.6.17-rc6-mm2.
> Please review it.
>
> Description:
>  Fix search_extable() and ia64_handle_exception() to handle the
> address format of exception_table_entry correctly.
>
> Thanks,
>
> --
> Masami HIRAMATSU
> 2nd Research Dept.
> Hitachi, Ltd., Systems Development Laboratory
> E-mail: hiramatu@sdl.hitachi.co.jp
>
> Signed-off-by: Masami Hiramatsu <hiramatu@sdl.hitachi.co.jp>
>
>  arch/ia64/mm/extable.c     |    7 ++++---
>  include/asm-ia64/uaccess.h |    4 ++--
>  2 files changed, 6 insertions(+), 5 deletions(-)
> diff --exclude=CVS -Narup a/arch/ia64/mm/extable.c b/arch/ia64/mm/extable.c
> --- a/arch/ia64/mm/extable.c    2005-10-28 09:02:08.000000000 +0900
> +++ b/arch/ia64/mm/extable.c    2006-06-14 14:59:11.000000000 +0900
> @@ -63,9 +63,10 @@ search_extable (const struct exception_t
>         unsigned long mid_ip;
>         long diff;
>
> +       ip = (ip & ~0xf) + ((ip & 0x3) << 2);
>          while (first <= last) {
>                 mid = &first[(last - first)/2];
> -               mid_ip = (u64) &mid->addr + mid->addr;
> +               mid_ip = ((u64) &mid->addr + mid->addr) & ~0x3;
>                 diff = mid_ip - ip;
>                  if (diff = 0)
>                          return mid;
> @@ -83,8 +84,8 @@ ia64_handle_exception (struct pt_regs *r
>         long fix = (u64) &e->cont + e->cont;
>
>         regs->r8 = -EFAULT;
> -       if (fix & 4)
> +       if (fix & 2)
>                 regs->r9 = 0;
>         regs->cr_iip = fix & ~0xf;
> -       ia64_psr(regs)->ri = fix & 0x3;         /* set continuation slot number */
> +       ia64_psr(regs)->ri = (fix & 0xc) >> 2;          /* set continuation slot number */
>  }
> diff --exclude=CVS -Narup a/include/asm-ia64/uaccess.h b/include/asm-ia64/uaccess.h
> --- a/include/asm-ia64/uaccess.h        2005-10-28 09:02:08.000000000 +0900
> +++ b/include/asm-ia64/uaccess.h        2006-06-14 23:54:55.000000000 +0900
> @@ -139,7 +139,7 @@ do {                                                                                                \
>         register long __gu_r8 asm ("r8") = 0;                                                   \
>         register long __gu_r9 asm ("r9");                                                       \
>         asm ("\n[1:]\tld"#n" %0=%2%P2\t// %0 and %1 get overwritten by exception handler\n"     \
> -            "\t.xdata4 \"__ex_table\", 1b-., 1f-.+4\n"                                         \
> +            "\t.xdata4 \"__ex_table\", 1b-., 1f-.+2\n"                                         \
>              "[1:]"                                                                             \
>              : "=r"(__gu_r9), "=r"(__gu_r8) : "m"(__m(addr)), "1"(__gu_r8));                    \
>         (err) = __gu_r8;                                                                        \
> @@ -346,7 +346,7 @@ extern unsigned long __strnlen_user (con
>
>  struct exception_table_entry {
>         int addr;       /* location-relative address of insn this fixup is for */
> -       int cont;       /* location-relative continuation addr.; if bit 2 is set, r9 is set to 0 */
> +       int cont;       /* location-relative continuation addr.; if bit 1 is set, r9 is set to 0 */
>  };
>
>  extern void ia64_handle_exception (struct pt_regs *regs, const struct exception_table_entry *e);
>
>
>
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Mosberger Consulting LLC, http://www.mosberger-consulting.com/

RE: [RFC][PATCH]fix search_extable() to find correct entry

[Chen, Kenneth W]

Masami Hiramatsu wrote on Thursday, June 15, 2006 6:27 AM
> I found a suspicious buggy code in the linux kernel on IA64 arch.
> As far as I can see, search_extable() doesn't work correctly, because
> the lookup routine expects that the address format of the
> exception_table_entry is "IP + slot", but the compiler (gcc-3.4.5)
> generates it as "IP + (slot << 2)". Thus the lookup routine always
> fails to find the corresponding entry.
> You can check it by dumping __ex_table section of vmlinux.


Are you sure about the exception table being wrong?  A quick look on my
system indicates that the compiler only generate bundle address, there
is no inter-bundle address in the exception table.

I can explain the data in continuation address. Since there is no exception
code that starts at inter-bundle address, it should always be bundle aligned,
i.e., slot will always be zero.

But the tag address in the table is also bundle aligned, which I will look
a bit more.


# gcc -v
Reading specs from /usr/lib/gcc/ia64-redhat-linux/3.4.5/specs
Configured with: ../configure --prefix=/usr --mandir=/usr/share/man --infodir=/usr/share/info --enable-shared --enable-threads=posix
--disable-checking --with-system-zlib --enable-__cxa_atexit --disable-libunwind-exceptions --enable-java-awt=gtk
--host=ia64-redhat-linux
Thread model: posix
gcc version 3.4.5 20051201 (Red Hat 3.4.5-2)

RE: [RFC][PATCH]fix search_extable() to find correct entry

[Chen, Kenneth W]

Masami Hiramatsu wrote on Thursday, June 15, 2006 6:27 AM
> I found a suspicious buggy code in the linux kernel on IA64 arch.
> As far as I can see, search_extable() doesn't work correctly, because
> the lookup routine expects that the address format of the
> exception_table_entry is "IP + slot", but the compiler (gcc-3.4.5)
> generates it as "IP + (slot << 2)". Thus the lookup routine always
> fails to find the corresponding entry.
> You can check it by dumping __ex_table section of vmlinux.

Chen, Kenneth W wrote on Thursday, June 15, 2006 2:22 PM
> But the tag address in the table is also bundle aligned, which I will
> look a bit more.


I've double checked (triple checked with sample test code below).  It's a
false alarm. Everyone should rest assure that exception table and compiler
do match and generates correct code.  There is no bug AFAICT.


Test code:

#include <linux/config.h>
#include <asm/asmmacro.h>

GLOBAL_ENTRY(ken)
        EX(eh0, ld1 r32 = [r0])
        EX(eh1, ld1 r33 = [r0])
        nop 0
        ;;
[eh0:] br.ret.sptk b0
[eh1:] br.ret.sptk b0

END(ken)


Exception table entry for eh0 and eh1:

0xa0000001004bc828   ffff7518 ffff7524 ffff7511 ffff751d


objdump of test code:

a0000001004b3d40:    ld1 r32=[r0]
a0000001004b3d46:    ld1 r33=[r0]
a0000001004b3d4c:    nop.i 0x0;;
a0000001004b3d50:    br.ret.sptk.few b0
a0000001004b3d56:    br.ret.sptk.few b0
a0000001004b3d5c:    nop.b 0x0;;

Re: [RFC][PATCH]fix search_extable() to find correct entry

[Masami Hiramatsu]

Hi,

Chen, Kenneth W wrote:
> Masami Hiramatsu wrote on Thursday, June 15, 2006 6:27 AM
>> I found a suspicious buggy code in the linux kernel on IA64 arch.
>> As far as I can see, search_extable() doesn't work correctly, because
>> the lookup routine expects that the address format of the
>> exception_table_entry is "IP + slot", but the compiler (gcc-3.4.5)
>> generates it as "IP + (slot << 2)". Thus the lookup routine always
>> fails to find the corresponding entry.
>> You can check it by dumping __ex_table section of vmlinux.
> 
> Chen, Kenneth W wrote on Thursday, June 15, 2006 2:22 PM
>> But the tag address in the table is also bundle aligned, which I will
>> look a bit more.
> 
> I've double checked (triple checked with sample test code below).  It's a
> false alarm. Everyone should rest assure that exception table and compiler
> do match and generates correct code.  There is no bug AFAICT.

Thank you for your comments.
I had just checked the dump of __ex_table section of vmlinux.
This time, I checked the __ex_table of running kernel.
It is true that those format is "IP + slot". So you are right.
I admit my mistake.
Please dispose the previous patch.

Thanks again.

-- 
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: hiramatu@sdl.hitachi.co.jp