[PATCH] [MIPS] Fix order of BRK_BUG in case

[PATCH] [MIPS] Fix order of BRK_BUG in case

[Ilpo Järvinen]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 914 bytes --]


BRK bug is defined as 512, which is smaller than 1 << 10
resulting in left shifting bcode by 10 earlier. Without
this the code block can't ever be reached.

Problem was added in commit 63dc68a8cf ([MIPS] Use conditional
traps for BUG_ON on MIPS II and better).

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
---

I'm by no means familiar with MIPS but this seems
an appropriate fix.

 arch/mips/kernel/traps.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
index 984c0d0..4dfcd61 100644
--- a/arch/mips/kernel/traps.c
+++ b/arch/mips/kernel/traps.c
@@ -694,7 +694,7 @@ asmlinkage void do_bp(struct pt_regs *regs)
 		info.si_addr = (void __user *) regs->cp0_epc;
 		force_sig_info(SIGFPE, &info, current);
 		break;
-	case BRK_BUG:
+	case BRK_BUG << 10:
 		die("Kernel bug detected", regs);
 		break;
 	default:
-- 
1.5.4.4

Re: [PATCH] [MIPS] Fix order of BRK_BUG in case

[Ralf Baechle]

On Sat, Apr 19, 2008 at 11:16:36PM +0300, Ilpo Järvinen wrote:

> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> index 984c0d0..4dfcd61 100644
> --- a/arch/mips/kernel/traps.c
> +++ b/arch/mips/kernel/traps.c
> @@ -694,7 +694,7 @@ asmlinkage void do_bp(struct pt_regs *regs)
>  		info.si_addr = (void __user *) regs->cp0_epc;
>  		force_sig_info(SIGFPE, &info, current);
>  		break;
> -	case BRK_BUG:
> +	case BRK_BUG << 10:
>  		die("Kernel bug detected", regs);
                ^^^

Now what will be happening if with your patch applied a "break BRK_BUG"
instruction is executed in userspace?

  Ralf

Re: [PATCH] [MIPS] Fix order of BRK_BUG in case

[Ilpo Järvinen]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1129 bytes --]

On Sun, 20 Apr 2008, Ralf Baechle wrote:

> On Sat, Apr 19, 2008 at 11:16:36PM +0300, Ilpo Järvinen wrote:
> 
> > diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> > index 984c0d0..4dfcd61 100644
> > --- a/arch/mips/kernel/traps.c
> > +++ b/arch/mips/kernel/traps.c
> > @@ -694,7 +694,7 @@ asmlinkage void do_bp(struct pt_regs *regs)
> >  		info.si_addr = (void __user *) regs->cp0_epc;
> >  		force_sig_info(SIGFPE, &info, current);
> >  		break;
> > -	case BRK_BUG:
> > +	case BRK_BUG << 10:
> >  		die("Kernel bug detected", regs);
>                 ^^^
> 
> Now what will be happening if with your patch applied a "break BRK_BUG"
> instruction is executed in userspace?

I suppose you know better, I can only guess :-). Like I said, I have no 
idea about mips code, maybe one would want die_if_kernel() kernel 
instead...? My main point was that the conversion made by commit 63dc68a8 
here seems questionable (resulted in dead code and would that not have 
been dead code the question you asked would apply to that code fragment), 
what is the right thing to do, you sure know better than I do :-).

-- 
 i.