Jesper Juhl wrote:
> arch/i386/kernel/traps.c: In function `do_general_protection':
> arch/i386/kernel/traps.c:506: warning: empty body in an if-statement
>
> upon inspecting the code I see what looks like a mistakenly placed ";"
>
> if (!fixup_exception(regs)) {
> if (notify_die(DIE_GPF, "general protection fault", regs,
> error_code, 13, SIGSEGV) == NOTIFY_STOP);
> return;
> die("general protection fault", regs, error_code);
> }
Ouch. No matter what the notifier chain returns it will be treated
as if it returned NOTIFY_STOP, and no kernel-mode GPF will ever reach
the die().
This bug was introduced 31 Aug 04 by [email protected] during a
kprobes update. The comments say it was ported from x86_64, so I had
a look:
/* kernel gp */
{
const struct exception_table_entry *fixup;
fixup = search_exception_tables(regs->rip);
if (fixup) {
regs->rip = fixup->fixup;
return;
}
notify_die(DIE_GPF, "general protection fault", regs, error_code,
13, SIGSEGV);
die("general protection fault", regs, error_code);
}
x86_64 never checks the result of notify_die() and unconditionally does a die().
I don't know if this is a bug or not...
Andi, if this is not a bug could you explain why not?
--Chuck Ebbert 24-Nov-04 00:23:50
On Wed, Nov 24, 2004 at 12:23:23AM -0500, Chuck Ebbert wrote:
> Jesper Juhl wrote:
>
> > arch/i386/kernel/traps.c: In function `do_general_protection':
> > arch/i386/kernel/traps.c:506: warning: empty body in an if-statement
> >
> > upon inspecting the code I see what looks like a mistakenly placed ";"
> >
> > if (!fixup_exception(regs)) {
> > if (notify_die(DIE_GPF, "general protection fault", regs,
> > error_code, 13, SIGSEGV) == NOTIFY_STOP);
> > return;
> > die("general protection fault", regs, error_code);
> > }
>
>
> Ouch. No matter what the notifier chain returns it will be treated
> as if it returned NOTIFY_STOP, and no kernel-mode GPF will ever reach
> the die().
>
> This bug was introduced 31 Aug 04 by [email protected] during a
> kprobes update. The comments say it was ported from x86_64, so I had
> a look:
>
> /* kernel gp */
> {
> const struct exception_table_entry *fixup;
> fixup = search_exception_tables(regs->rip);
> if (fixup) {
> regs->rip = fixup->fixup;
> return;
> }
> notify_die(DIE_GPF, "general protection fault", regs, error_code,
> 13, SIGSEGV);
> die("general protection fault", regs, error_code);
> }
>
> x86_64 never checks the result of notify_die() and unconditionally does a die().
> I don't know if this is a bug or not...
>
> Andi, if this is not a bug could you explain why not?
It depends on what the debugger (or kprobes) wants. These checks
are added based on their needs. Perhaps he didn't consider it
necessary on x86-64. But why don't you ask Prasanna directly? (cc'ed)
-Andi
> > x86_64 never checks the result of notify_die() and unconditionally does a die().
> > I don't know if this is a bug or not...
> >
> > Andi, if this is not a bug could you explain why not?
>
> It depends on what the debugger (or kprobes) wants. These checks
> are added based on their needs. Perhaps he didn't consider it
> necessary on x86-64. But why don't you ask Prasanna directly? (cc'ed)
>
I agree with Andi that it depends on the specific debugger. On handling
the general protection fault notification, kprobe handler returns NOTIFY_STOP.
This check missed out in x86_64 kprobes patch. Below patch should fix this,
please let me know if you have any issues.
Thanks
Prasanna
This patch adds the return value check for the exception notifiers at
do_general_protection as pointed out by Chuck Ebbert.
Signed-off-by: Prasanna S Panchamukhi <[email protected]>
---
linux-2.6.10-rc2-prasanna/arch/x86_64/kernel/traps.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff -puN arch/x86_64/kernel/traps.c~notifier-fix arch/x86_64/kernel/traps.c
--- linux-2.6.10-rc2/arch/x86_64/kernel/traps.c~notifier-fix 2004-11-24 17:06:41.000000000 +0530
+++ linux-2.6.10-rc2-prasanna/arch/x86_64/kernel/traps.c 2004-11-24 17:08:18.000000000 +0530
@@ -556,8 +556,9 @@ asmlinkage void do_general_protection(st
regs->rip = fixup->fixup;
return;
}
- notify_die(DIE_GPF, "general protection fault", regs, error_code,
- 13, SIGSEGV);
+ if (notify_die(DIE_GPF, "general protection fault", regs,
+ error_code, 13, SIGSEGV) == NOTIFY_STOP)
+ return;
die("general protection fault", regs, error_code);
}
}
_
--
Prasanna S Panchamukhi
Linux Technology Center
India Software Labs, IBM Bangalore
Ph: 91-80-25044636
<[email protected]>