Kyle McMartin wrote:
> Honestly, I can't decide whether to apply this. It really should never
> happen in the kernel, since the kernel can guarantee it won't get the
> access rights failure (highest privilege level, and can set %sr and
> %protid to whatever it wants.)
Yes, that's the problem.
We catch a bug in the kernel which shouldn't be there (and which we
can't find yet).
If it wouldn't be there, we wouldn't need this patch..
Chicken and egg problem.
> It really genuinely is a bug that probably should panic the kernel. The
> only precedent I can easily see is x86 fixing up a bad iret with a
> general protection fault, which is more or less analogous to code 27
> here.
>
> On the other hand, taking the exception on a userspace access really
> isn't all that critical, and there's fundamentally little reason for the
> kernel not to SIGSEGV the process, and continue...
>
> Argh.
Argh. Argh :-)
> (btw, I've instrumented my do_sys_poll with a pile of assertions that
> %cr8 << 1 == %sr3 == current->mm.context... let's see if where we're
> getting corrupted is deterministic, though, I would guess that it won't
> be.)
I'll continue to debug as well. I have some more ideas left to try...
Thanks for cleaning up my patch and splitting out the fixup_exception()
code.
Your patch is fine and I think you should apply it (with one minor
addition - see below).
The reason I think why you should apply it is, that we shouldn't let the
kernel crash on (correct) userspace apps.
Maybe adding a comment that this check in trap #27 should be removed
when the bug is fixed would be good though.
> diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
> index 4878b95..1c6dbb6 100644
> --- a/arch/parisc/include/asm/uaccess.h
> +++ b/arch/parisc/include/asm/uaccess.h
> @@ -745,6 +745,10 @@ void handle_interruption(int code, struct pt_regs *regs)
> /* Fall Through */
> case 27:
> /* Data memory protection ID trap */
> + if (code == 27 && !user_mode(regs) &&
> + fixup_exception(regs))
> + return;
I think you should add a self-explaining printk(KERN_CRIT,..) here, so
that we can track when this bug appeared in userspace.
Something like:
printk("strange data protection ID trap happened to <procname> PID: xxx.
(this is a bug in the kernel)\n");
(you probably find a better wording)
Having a correctly-coded userspace process which suddenly just return
failures in syscalls is not good and would make people wonder and
unnecessarily try to debug their (correct) userspace apps...
Helge
--
To unsubscribe from this list: send the line "unsubscribe linux-parisc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
