Re: [PATCH] powerpc/signal32: Fix Oops on sigreturn with unmapped VDSO
Christophe Leroy writes:
> PPC32 encounters a KUAP fault when trying to handle a signal with
> VDSO unmapped.
>
> Kernel attempted to read user page (7fc07ec0) - exploit attempt? (uid:
> 0)
> BUG: Unable to handle kernel data access on read at 0x7fc07ec0
> Faulting instruction address: 0xc00111d4
> Oops: Kernel access of bad area, sig: 11 [#1]
> BE PAGE_SIZE=16K PREEMPT CMPC885
> CPU: 0 PID: 353 Comm: sigreturn_vdso Not tainted
> 5.12.0-rc4-s3k-dev-01553-gb30c310ea220 #4814
> NIP: c00111d4 LR: c0005a28 CTR:
> REGS: cadb3dd0 TRAP: 0300 Not tainted
> (5.12.0-rc4-s3k-dev-01553-gb30c310ea220)
> MSR: 9032 CR: 48000884 XER: 2000
> DAR: 7fc07ec0 DSISR: 8800
> GPR00: c0007788 cadb3e90 c28d4a40 7fc07ec0 7fc07ed0 04e0 7fc07ce0
>
> GPR08: 0001 0001 7fc07ec0 28000282 1001b828 100a0920
>
> GPR16: 100cac0c 100b 105c43a4 105c5685 100d 100d 100d
> 100b2e9e
> GPR24: 105c43c8 7fc07ec8 cadb3f40 cadb3ec8 c28d4a40
>
> NIP [c00111d4] flush_icache_range+0x90/0xb4
> LR [c0005a28] handle_signal32+0x1bc/0x1c4
> Call Trace:
> [cadb3e90] [100d] 0x100d (unreliable)
> [cadb3ec0] [c0007788] do_notify_resume+0x260/0x314
> [cadb3f20] [c000c764] syscall_exit_prepare+0x120/0x184
> [cadb3f30] [c00100b4] ret_from_syscall+0xc/0x28
> --- interrupt: c00 at 0xfe807f8
> NIP: 0fe807f8 LR: 10001060 CTR: c0139378
> REGS: cadb3f40 TRAP: 0c00 Not tainted
> (5.12.0-rc4-s3k-dev-01553-gb30c310ea220)
> MSR: d032 CR: 28000482 XER: 2000
>
> GPR00: 0025 7fc081c0 77bb1690 000a 28000482 0001
> 0ff03a38
> GPR08: d032 6de5 c28d4a40 0009 88000482 1001b828 100a0920
>
> GPR16: 100cac0c 100b 105c43a4 105c5685 100d 100d 100d
> 100b2e9e
> GPR24: 105c43c8 77ba7628 10002398 1001 10002124
> 00024000
> NIP [0fe807f8] 0xfe807f8
> LR [10001060] 0x10001060
> --- interrupt: c00
> Instruction dump:
> 38630010 7c001fac 38630010 4200fff0 7c0004ac 4c00012c 4e800020 7c001fac
> 2c0a 38630010 4082ffcc 4be4 <7c00186c> 2c07 39430010
> 4082ff8c
> ---[ end trace 3973fb72b049cb06 ]---
>
> This is because flush_icache_range() is called on user addresses.
>
> The same problem was detected some time ago on PPC64. It was fixed by
> enabling KUAP in commit 59bee45b9712 ("powerpc/mm: Fix missing KUAP
> disable in flush_coherent_icache()").
>
> PPC32 doesn't use flush_coherent_icache() and fallbacks on
> clean_dcache_range() and invalidate_icache_range().
But this code is also used for compat tasks on 64-bit.
> We could fix it similarly by enabling user access in those functions,
> but this is overkill for just flushing two instructions.
>
> The two instructions are 8 bytes aligned, so a single dcbst/icbi is
> enough to flush them. Do like __patch_instruction() and inline
> a dcbst followed by an icbi just after the write of the instructions,
> while user access is still allowed. The isync is not required because
> rfi will be used to return to user.
>
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/kernel/signal_32.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
> index 75ee918a120a..5b2ba2731957 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -809,6 +809,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t
> *oldset,
> unsafe_put_user(PPC_INST_ADDI + __NR_rt_sigreturn,
> >mc_pad[0],
> failed);
> unsafe_put_user(PPC_INST_SC, >mc_pad[1], failed);
> + asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
If I'm reading that right you're pointing the icbi at the user address.
That's going to cause a KUAP fault just like we fixed in commit
59bee45b9712 ("powerpc/mm: Fix missing KUAP disable in
flush_coherent_icache()").
We have user write access enabled, but the icbi is treated as a load.
So I don't think that's going to work.
cheers
[PATCH] powerpc/signal32: Fix Oops on sigreturn with unmapped VDSO
PPC32 encounters a KUAP fault when trying to handle a signal with
VDSO unmapped.
Kernel attempted to read user page (7fc07ec0) - exploit attempt? (uid:
0)
BUG: Unable to handle kernel data access on read at 0x7fc07ec0
Faulting instruction address: 0xc00111d4
Oops: Kernel access of bad area, sig: 11 [#1]
BE PAGE_SIZE=16K PREEMPT CMPC885
CPU: 0 PID: 353 Comm: sigreturn_vdso Not tainted
5.12.0-rc4-s3k-dev-01553-gb30c310ea220 #4814
NIP: c00111d4 LR: c0005a28 CTR:
REGS: cadb3dd0 TRAP: 0300 Not tainted
(5.12.0-rc4-s3k-dev-01553-gb30c310ea220)
MSR: 9032 CR: 48000884 XER: 2000
DAR: 7fc07ec0 DSISR: 8800
GPR00: c0007788 cadb3e90 c28d4a40 7fc07ec0 7fc07ed0 04e0 7fc07ce0
GPR08: 0001 0001 7fc07ec0 28000282 1001b828 100a0920
GPR16: 100cac0c 100b 105c43a4 105c5685 100d 100d 100d
100b2e9e
GPR24: 105c43c8 7fc07ec8 cadb3f40 cadb3ec8 c28d4a40
NIP [c00111d4] flush_icache_range+0x90/0xb4
LR [c0005a28] handle_signal32+0x1bc/0x1c4
Call Trace:
[cadb3e90] [100d] 0x100d (unreliable)
[cadb3ec0] [c0007788] do_notify_resume+0x260/0x314
[cadb3f20] [c000c764] syscall_exit_prepare+0x120/0x184
[cadb3f30] [c00100b4] ret_from_syscall+0xc/0x28
--- interrupt: c00 at 0xfe807f8
NIP: 0fe807f8 LR: 10001060 CTR: c0139378
REGS: cadb3f40 TRAP: 0c00 Not tainted
(5.12.0-rc4-s3k-dev-01553-gb30c310ea220)
MSR: d032 CR: 28000482 XER: 2000
GPR00: 0025 7fc081c0 77bb1690 000a 28000482 0001
0ff03a38
GPR08: d032 6de5 c28d4a40 0009 88000482 1001b828 100a0920
GPR16: 100cac0c 100b 105c43a4 105c5685 100d 100d 100d
100b2e9e
GPR24: 105c43c8 77ba7628 10002398 1001 10002124
00024000
NIP [0fe807f8] 0xfe807f8
LR [10001060] 0x10001060
--- interrupt: c00
Instruction dump:
38630010 7c001fac 38630010 4200fff0 7c0004ac 4c00012c 4e800020 7c001fac
2c0a 38630010 4082ffcc 4be4 <7c00186c> 2c07 39430010
4082ff8c
---[ end trace 3973fb72b049cb06 ]---
This is because flush_icache_range() is called on user addresses.
The same problem was detected some time ago on PPC64. It was fixed by
enabling KUAP in commit 59bee45b9712 ("powerpc/mm: Fix missing KUAP
disable in flush_coherent_icache()").
PPC32 doesn't use flush_coherent_icache() and fallbacks on
clean_dcache_range() and invalidate_icache_range().
We could fix it similarly by enabling user access in those functions,
but this is overkill for just flushing two instructions.
The two instructions are 8 bytes aligned, so a single dcbst/icbi is
enough to flush them. Do like __patch_instruction() and inline
a dcbst followed by an icbi just after the write of the instructions,
while user access is still allowed. The isync is not required because
rfi will be used to return to user.
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/signal_32.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 75ee918a120a..5b2ba2731957 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -809,6 +809,7 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t
*oldset,
unsafe_put_user(PPC_INST_ADDI + __NR_rt_sigreturn,
>mc_pad[0],
failed);
unsafe_put_user(PPC_INST_SC, >mc_pad[1], failed);
+ asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
}
unsafe_put_sigset_t(>uc.uc_sigmask, oldset, failed);
@@ -817,9 +818,6 @@ int handle_rt_signal32(struct ksignal *ksig, sigset_t
*oldset,
if (copy_siginfo_to_user(>info, >info))
goto badframe;
- if (tramp == (unsigned long)mctx->mc_pad)
- flush_icache_range(tramp, tramp + 2 * sizeof(unsigned long));
-
regs->link = tramp;
#ifdef CONFIG_PPC_FPU_REGS
@@ -908,12 +906,10 @@ int handle_signal32(struct ksignal *ksig, sigset_t
*oldset,
/* Set up the sigreturn trampoline: li r0,sigret; sc */
unsafe_put_user(PPC_INST_ADDI + __NR_sigreturn,
>mc_pad[0], failed);
unsafe_put_user(PPC_INST_SC, >mc_pad[1], failed);
+ asm("dcbst %y0; sync; icbi %y0; sync" :: "Z" (mctx->mc_pad[0]));
}
user_write_access_end();
- if (tramp == (unsigned long)mctx->mc_pad)
- flush_icache_range(tramp, tramp + 2 * sizeof(unsigned long));
-
regs->link = tramp;
#ifdef CONFIG_PPC_FPU_REGS
--
2.25.0
