Re: [PATCH] parisc: Fix kernel panic due invalid values of IAOQ0 or IAOQ1

[Helge Deller <deller@gmx.de>]

On 04.07.19 21:54, Helge Deller wrote:
> Hi Jeroen,
>
> On 04.07.19 21:23, Jeroen Roovers wrote:
>> On Wed, 3 Jul 2019 08:35:24 +0200
>> Helge Deller <deller@gmx.de> wrote:
>>
>>> On parisc the privilege level of a process is stored in the lowest
>>> two bits of the instruction pointers (IAOQ0 and IAOQ1). On Linux we
>>> use privilege level 0 for the kernel and privilege level 3 for
>>> user-space. So userspace should not be allowed to modify IAOQ0 or
>>> IAOQ1 of a ptraced process to change it's privilege level to e.g. 0
>>> to try to gain kernel privileges.
>>>
>>> This patch prevents such modifications by always setting the two
>>> lowest bits to one (which relates to privilege level 3 for
>>> user-space) if IAOQ0 or IAOQ1 are modified via ptrace calls.
>>>
>>> Fixes: https://bugs.gentoo.org/481768
>>> Reported-by: Jeroen Roovers <jer@gentoo.org>
>>> Cc: <stable@vger.kernel.org>
>>>
>>> diff --git a/arch/parisc/kernel/ptrace.c b/arch/parisc/kernel/ptrace.c
>>> index a3d2fb4e6dd2..8ecd41938709 100644
>>> --- a/arch/parisc/kernel/ptrace.c
>>> +++ b/arch/parisc/kernel/ptrace.c
>>> @@ -167,6 +175,9 @@ long arch_ptrace(struct task_struct *child, long
>>> request, if ((addr & (sizeof(unsigned long)-1)) ||
>>>                addr >= sizeof(struct pt_regs))
>>>               break;
>>> +        if (addr == PT_IAOQ0 || addr == PT_IAOQ1) {
>>> +            data |= 3; /* ensure userspace privilege */
>>> +        }
>>>           if ((addr >= PT_GR1 && addr <= PT_GR31) ||
>>>                   addr == PT_IAOQ0 || addr == PT_IAOQ1
>>> || (addr >= PT_FR0 && addr <= PT_FR31 + 4) ||
>>> @@ -281,6 +292,9 @@ long compat_arch_ptrace(struct task_struct
>>> *child, compat_long_t request, addr = translate_usr_offset(addr);
>>>               if (addr >= sizeof(struct pt_regs))
>>>                   break;
>>> +            if (addr == PT_IAOQ0 || addr == PT_IAOQ1) {
>>> +                data |= 3; /* ensure userspace
>>> privilege */
>>> +            }
>>>               if (addr >= PT_FR0 && addr <= PT_FR31 + 4) {
>>>                   /* Special case, fp regs are 64 bits
>>> anyway */ *(__u64 *) ((char *) task_regs(child) + addr) = data;
>>
>> That may fix some problem, but it sadly does not fix the problem
>> reported in https://bugs.gentoo.org/481768 . Both root and unprivileged
>> users can still trigger the same kernel panic with a kernel patches
>> thusly. How can we help you reproduce the issue?
>
> Thanks for testing, but are you sure?
> It does fix exactly that kernel panic for me.
> Instead of triggering a kernel panic, the ptraced process now gets
> killed by a segfault, which is exactly what should happen:
>
> Breakpoint 1, main () at gdb-crash.c:24
> 24      gdb-crash.c: No such file or directory.
> (gdb) set tp = { 3,4 }
> Python Exception <type 'exceptions.NameError'> Installation error: gdb.execute_unwinders function is missing:
> Program received signal SIGSEGV, Segmentation fault.
> 0x00000000 in ?? ()
> The program being debugged was signaled while in a function called from GDB.
> GDB remains in the frame where the signal was received.
> To change this behavior use "set unwindonsignal on".
> Evaluation of the expression containing the function
> (malloc) will be abandoned.
> When the function is done executing, GDB will silently stop.
> (gdb)
>
> Of course this patch does not fix gdb in such a way that
> it handles the "set tp = { 5,3 }" correctly. That's a gdb issue.
>
> The above log is from a 32bit-kernel. Did you maybe tested a 64bit kernel (I didn't tested it).
> Or maybe you didn't booted a kernel with that patch?
> I'm pretty sure the patch is correct.

In case you still get a kernel panic, please verify the value of "IAOQ" in the panic you see:
IASQ: 0000000000334000 0000000000334000 IAOQ: 0000000000000000 0000000000000004

If the value is still "0000000000000000" instead of "0000000000000003",
then my patch wasn't applied correctly.

Helge