Skip to content

Commit 5e5be3a

Browse files
committed
powerpc/mm: Detect bad KUAP faults
When KUAP is enabled we have logic to detect page faults that occur outside of a valid user access region and are blocked by the AMR. What we don't have at the moment is logic to detect a fault *within* a valid user access region, that has been incorrectly blocked by AMR. This is not meant to ever happen, but it can if we incorrectly save/restore the AMR, or if the AMR was overwritten for some other reason. Currently if that happens we assume it's just a regular fault that will be corrected by handling the fault normally, so we just return. But there is nothing the fault handling code can do to fix it, so the fault just happens again and we spin forever, leading to soft lockups. So add some logic to detect that case and WARN() if we ever see it. Arguably it should be a BUG(), but it's more polite to fail the access and let the kernel continue, rather than taking down the box. There should be no data integrity issue with failing the fault rather than BUG'ing, as we're just going to disallow an access that should have been allowed. To make the code a little easier to follow, unroll the condition at the end of bad_kernel_fault() and comment each case, before adding the call to bad_kuap_fault(). Signed-off-by: Michael Ellerman <[email protected]>
1 parent 890274c commit 5e5be3a

File tree

3 files changed

+29
-3
lines changed

3 files changed

+29
-3
lines changed

arch/powerpc/include/asm/book3s/64/kup-radix.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,12 @@ static inline void prevent_user_access(void __user *to, const void __user *from,
9595
set_kuap(AMR_KUAP_BLOCKED);
9696
}
9797

98+
static inline bool bad_kuap_fault(struct pt_regs *regs, bool is_write)
99+
{
100+
return WARN(mmu_has_feature(MMU_FTR_RADIX_KUAP) &&
101+
(regs->kuap & (is_write ? AMR_KUAP_BLOCK_WRITE : AMR_KUAP_BLOCK_READ)),
102+
"Bug: %s fault blocked by AMR!", is_write ? "Write" : "Read");
103+
}
98104
#endif /* CONFIG_PPC_KUAP */
99105

100106
#endif /* __ASSEMBLY__ */

arch/powerpc/include/asm/kup.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ static inline void allow_user_access(void __user *to, const void __user *from,
2626
unsigned long size) { }
2727
static inline void prevent_user_access(void __user *to, const void __user *from,
2828
unsigned long size) { }
29+
static inline bool bad_kuap_fault(struct pt_regs *regs, bool is_write) { return false; }
2930
#endif /* CONFIG_PPC_KUAP */
3031

3132
static inline void allow_read_from_user(const void __user *from, unsigned long size)

arch/powerpc/mm/fault.c

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
#include <asm/mmu_context.h>
4545
#include <asm/siginfo.h>
4646
#include <asm/debug.h>
47+
#include <asm/kup.h>
4748

4849
static inline bool notify_page_fault(struct pt_regs *regs)
4950
{
@@ -224,7 +225,7 @@ static int mm_fault_error(struct pt_regs *regs, unsigned long addr,
224225

225226
/* Is this a bad kernel fault ? */
226227
static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code,
227-
unsigned long address)
228+
unsigned long address, bool is_write)
228229
{
229230
int is_exec = TRAP(regs) == 0x400;
230231

@@ -235,6 +236,9 @@ static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code,
235236
address >= TASK_SIZE ? "exec-protected" : "user",
236237
address,
237238
from_kuid(&init_user_ns, current_uid()));
239+
240+
// Kernel exec fault is always bad
241+
return true;
238242
}
239243

240244
if (!is_exec && address < TASK_SIZE && (error_code & DSISR_PROTFAULT) &&
@@ -244,7 +248,22 @@ static bool bad_kernel_fault(struct pt_regs *regs, unsigned long error_code,
244248
from_kuid(&init_user_ns, current_uid()));
245249
}
246250

247-
return is_exec || (address >= TASK_SIZE) || !search_exception_tables(regs->nip);
251+
// Kernel fault on kernel address is bad
252+
if (address >= TASK_SIZE)
253+
return true;
254+
255+
// Fault on user outside of certain regions (eg. copy_tofrom_user()) is bad
256+
if (!search_exception_tables(regs->nip))
257+
return true;
258+
259+
// Read/write fault in a valid region (the exception table search passed
260+
// above), but blocked by KUAP is bad, it can never succeed.
261+
if (bad_kuap_fault(regs, is_write))
262+
return true;
263+
264+
// What's left? Kernel fault on user in well defined regions (extable
265+
// matched), and allowed by KUAP in the faulting context.
266+
return false;
248267
}
249268

250269
static bool bad_stack_expansion(struct pt_regs *regs, unsigned long address,
@@ -467,7 +486,7 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
467486
* take a page fault to a kernel address or a page fault to a user
468487
* address outside of dedicated places
469488
*/
470-
if (unlikely(!is_user && bad_kernel_fault(regs, error_code, address)))
489+
if (unlikely(!is_user && bad_kernel_fault(regs, error_code, address, is_write)))
471490
return SIGSEGV;
472491

473492
/*

0 commit comments

Comments
 (0)