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

[Helge Deller <deller@gmx.de>]

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.

Helge