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

[Helge Deller <deller@gmx.de>]

On 04.07.19 21:58, Helge Deller wrote:
> 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.

I'm wrong.
I see crashes with a 64bit kernel too. 32bit kernel worked nicely.
Will analyze.

Helge