Received-SPF: pass (google.com: domain of linux-kernel+bounces-121652-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Feedback-ID: i84414492:Fastmail
User-Agent: Cyrus-JMAP/3.11.0-alpha0-328-gc998c829b7-fm-20240325.002-gc998c829
Precedence: bulk
MIME-Version: 1.0
Message-Id: <234c458c-15f1-423f-a2fd-0abfc6232c66@app.fastmail.com>
In-Reply-To: 
 <CAEEQ3w=yNEktgUucmKuUvqfwDYYztVX+jofi5Q7hG-yQWcLbTA@mail.gmail.com>
References: <20240327061258.2370291-1-sorear@fastmail.com>
 <CAEEQ3w=yNEktgUucmKuUvqfwDYYztVX+jofi5Q7hG-yQWcLbTA@mail.gmail.com>
Date: Wed, 27 Mar 2024 12:53:33 -0400
From: "Stefan O'Rear" <sorear@fastmail.com>
To: "yunhui cui" <cuiyunhui@bytedance.com>
Cc: linux-riscv@lists.infradead.org,
 "Paul Walmsley" <paul.walmsley@sifive.com>,
 "Palmer Dabbelt" <palmer@dabbelt.com>, "Albert Ou" <aou@eecs.berkeley.edu>,
 linux-kernel@vger.kernel.org
Subject: Re: [External] [PATCH] riscv: process: Fix kernel gp leakage
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable

On Wed, Mar 27, 2024, at 4:43 AM, yunhui cui wrote:
> Hi Stefan,
>
> On Wed, Mar 27, 2024 at 2:14=E2=80=AFPM Stefan O'Rear <sorear@fastmail=
com> wrote:
>>
>> childregs represents the registers which are active for the new thread
>> in user context. For a kernel thread, childregs->gp is never used sin=
ce
>> the kernel gp is not touched by switch_to. For a user mode helper, the
>> gp value can be observed in user space after execve or possibly by ot=
her
>> means.
>>
>> Fixes: 7db91e57a0ac ("RISC-V: Task implementation")
>> Signed-off-by: Stefan O'Rear <sorear@fastmail.com>
>> ---
>>  arch/riscv/kernel/process.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
>> index 92922dbd5b5c..51042f48da17 100644
>> --- a/arch/riscv/kernel/process.c
>> +++ b/arch/riscv/kernel/process.c
>> @@ -27,8 +27,6 @@
>>  #include <asm/vector.h>
>>  #include <asm/cpufeature.h>
>>
>> -register unsigned long gp_in_global __asm__("gp");
>> -
>>  #if defined(CONFIG_STACKPROTECTOR) && !defined(CONFIG_STACKPROTECTOR=
_PER_TASK)
>>  #include <linux/stackprotector.h>
>>  unsigned long __stack_chk_guard __read_mostly;
>> @@ -207,7 +205,6 @@ int copy_thread(struct task_struct *p, const stru=
ct kernel_clone_args *args)
>>         if (unlikely(args->fn)) {
>>                 /* Kernel thread */
>>                 memset(childregs, 0, sizeof(struct pt_regs));
>> -               childregs->gp =3D gp_in_global;
>>                 /* Supervisor/Machine, irqs on: */
>>                 childregs->status =3D SR_PP | SR_PIE;
>>
>> --
>> 2.40.1
>>
>>
> Can you help express in more detail what the problem was before fixing=
 it?

It's a KASLR bypass, since gp_in_global is the address of the kernel sym=
bol
__global_pointer$.

The /* Kernel thread */ comment is somewhat inaccurate in that it is als=
o used
for user_mode_helper threads, which exec a user process, e.g. /sbin/init=
 or
when /proc/sys/kernel/core_pattern is a pipe. Such threads do not have
PF_KTHREAD set and are valid targets for ptrace etc. even before they ex=
ec.

childregs is the *user* context during syscall execution and it is obser=
vable
from userspace in at least five ways:

1. kernel_execve does not currently clear integer registers, so the star=
ting
   register state for PID 1 and other user processes started by the kern=
el has
   sp =3D user stack, gp =3D kernel __global_pointer$, all other integer=
 registers
   zeroed by the memset in the patch comment.

   This is a bug in its own right, but I'm unwilling to bet that it is t=
he only
   way to exploit the issue addressed by this patch.

2. ptrace(PTRACE_GETREGSET): you can PTRACE_ATTACH to a user_mode_helper=
 thread
   before it execs, but ptrace requires SIGSTOP to be delivered which ca=
n only
   happen at user/kernel boundaries.

3. /proc/*/task/*/syscall: this is perfectly happy to read pt_regs for
   user_mode_helpers before the exec completes, but gp is not one of the
   registers it returns.

4. PERF_SAMPLE_REGS_USER: LOCKDOWN_PERF normally prevents access to kern=
el
   addresses via PERF_SAMPLE_REGS_INTR, but due to this bug kernel addre=
sses
   are also exposed via PERF_SAMPLE_REGS_USER which is permitted under
   LOCKDOWN_PERF. I have not attempted to write exploit code.

5. Much of the tracing infrastructure allows access to user registers. I=
 have
   not attempted to determine which forms of tracing allow access to user
   registers without already allowing access to kernel registers.

Does this help? How much of this should be in the commit message?

-s

> Thanks,
> Yunhui
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv