Skip to content

Commit 97ab824

Browse files
aikgregkh
authored andcommitted
powerpc/uaccess: Avoid might_fault() when user access is enabled
[ Upstream commit 7d506ca ] The amount of code executed with enabled user space access (unlocked KUAP) should be minimal. However with CONFIG_PROVE_LOCKING or CONFIG_DEBUG_ATOMIC_SLEEP enabled, might_fault() calls into various parts of the kernel, and may even end up replaying interrupts which in turn may access user space and forget to restore the KUAP state. The problem places are: 1. strncpy_from_user (and similar) which unlock KUAP and call unsafe_get_user -> __get_user_allowed -> __get_user_nocheck() with do_allow=false to skip KUAP as the caller took care of it. 2. __unsafe_put_user_goto() which is called with unlocked KUAP. eg: WARNING: CPU: 30 PID: 1 at arch/powerpc/include/asm/book3s/64/kup.h:324 arch_local_irq_restore+0x160/0x190 NIP arch_local_irq_restore+0x160/0x190 LR lock_is_held_type+0x140/0x200 Call Trace: 0xc00000007f392ff8 (unreliable) ___might_sleep+0x180/0x320 __might_fault+0x50/0xe0 filldir64+0x2d0/0x5d0 call_filldir+0xc8/0x180 ext4_readdir+0x948/0xb40 iterate_dir+0x1ec/0x240 sys_getdents64+0x80/0x290 system_call_exception+0x160/0x280 system_call_common+0xf0/0x27c Change __get_user_nocheck() to look at `do_allow` to decide whether to skip might_fault(). Since strncpy_from_user/etc call might_fault() anyway before unlocking KUAP, there should be no visible change. Drop might_fault() in __unsafe_put_user_goto() as it is only called from unsafe_put_user(), which already has KUAP unlocked. Since keeping might_fault() is still desirable for debugging, add calls to it in user_[read|write]_access_begin(). That also allows us to drop the is_kernel_addr() test, because there should be no code using user_[read|write]_access_begin() in order to access a kernel address. Fixes: de78a9c ("powerpc: Add a framework for Kernel Userspace Access Protection") Signed-off-by: Alexey Kardashevskiy <[email protected]> [mpe: Combine with related patch from myself, merge change logs] Signed-off-by: Michael Ellerman <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Sasha Levin <[email protected]>
1 parent 3aa4af4 commit 97ab824

File tree

1 file changed

+10
-3
lines changed

1 file changed

+10
-3
lines changed

arch/powerpc/include/asm/uaccess.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -216,8 +216,6 @@ do { \
216216
#define __put_user_nocheck_goto(x, ptr, size, label) \
217217
do { \
218218
__typeof__(*(ptr)) __user *__pu_addr = (ptr); \
219-
if (!is_kernel_addr((unsigned long)__pu_addr)) \
220-
might_fault(); \
221219
__chk_user_ptr(ptr); \
222220
__put_user_size_goto((x), __pu_addr, (size), label); \
223221
} while (0)
@@ -313,7 +311,7 @@ do { \
313311
__typeof__(size) __gu_size = (size); \
314312
\
315313
__chk_user_ptr(__gu_addr); \
316-
if (!is_kernel_addr((unsigned long)__gu_addr)) \
314+
if (do_allow && !is_kernel_addr((unsigned long)__gu_addr)) \
317315
might_fault(); \
318316
barrier_nospec(); \
319317
if (do_allow) \
@@ -508,6 +506,9 @@ static __must_check inline bool user_access_begin(const void __user *ptr, size_t
508506
{
509507
if (unlikely(!access_ok(ptr, len)))
510508
return false;
509+
510+
might_fault();
511+
511512
allow_read_write_user((void __user *)ptr, ptr, len);
512513
return true;
513514
}
@@ -521,6 +522,9 @@ user_read_access_begin(const void __user *ptr, size_t len)
521522
{
522523
if (unlikely(!access_ok(ptr, len)))
523524
return false;
525+
526+
might_fault();
527+
524528
allow_read_from_user(ptr, len);
525529
return true;
526530
}
@@ -532,6 +536,9 @@ user_write_access_begin(const void __user *ptr, size_t len)
532536
{
533537
if (unlikely(!access_ok(ptr, len)))
534538
return false;
539+
540+
might_fault();
541+
535542
allow_write_to_user((void __user *)ptr, len);
536543
return true;
537544
}

0 commit comments

Comments
 (0)