Unverified Commit a921f075 authored by Palmer Dabbelt's avatar Palmer Dabbelt
Browse files

Merge patch series "riscv: misaligned: fix misaligned accesses handling in put/get_user()" 

Clément Léger <cleger@rivosinc.com> says:

While debugging a few problems with the misaligned access kselftest,
Alexandre discovered some crash with the current code. Indeed, some
misaligned access was done by the kernel using put_user(). This
was resulting in trap and a kernel crash since. The path was the
following:
user -> kernel -> access to user memory -> misaligned trap -> trap ->
kernel -> misaligned handling -> memcpy -> crash due to failed page fault
while in interrupt disabled section.

Last discussion about kernel misaligned handling and interrupt reenabling
were actually not to reenable interrupt when handling misaligned access
being done by kernel. The best solution being not to do any misaligned
accesses to userspace memory, we considered a few options:

- Remove any call to put/get_user() potentially doing misaligned
  accesses
- Do not do any misaligned accesses in put/get_user() itself

The second solution was the one chosen as there are too many callsites to
put/get_user() that could potentially do misaligned accesses. We tried
two approaches for that, either split access in two aligned accesses
(and do RMW for put_user()) or call copy_from/to_user() which does not
do any misaligned accesses. The later one was the simpler to implement
(although the performances are probably lower than split aligned
accesses but still way better than doing misaligned access emulation)
and allows to support what we wanted.

These commits are based on top of Alex dev/alex/get_user_misaligned_v1
branch.

[Palmer: No idea what that branch is, so I'm basing it on the uaccess
optimizations patch series which is the last thing to touch these.]

* b4-shazam-merge
  riscv: uaccess: do not do misaligned accesses in get/put_user()
  riscv: process: use unsigned int instead of unsigned long for put_user()
  riscv: make unsafe user copy routines use existing assembly routines

Link: https://lore.kernel.org/r/20250602193918.868962-1-cleger@rivosinc.com


Signed-off-by: default avatarPalmer Dabbelt <palmer@dabbelt.com>
parents 265d6aba ca1a66cd

arch/riscv/include/asm/asm-prototypes.h

+1 −1
Original line number Diff line number Diff line
@@ -12,7 +12,7 @@ long long __ashlti3(long long a, int b); 
#ifdef CONFIG_RISCV_ISA_V



#ifdef CONFIG_MMU

asmlinkage int enter_vector_usercopy(void *dst, void *src, size_t n);

asmlinkage int enter_vector_usercopy(void *dst, void *src, size_t n, bool enable_sum);

#endif /* CONFIG_MMU  */



void xor_regs_2_(unsigned long bytes, unsigned long *__restrict p1,

arch/riscv/include/asm/uaccess.h

+22 −26
Original line number Diff line number Diff line
@@ -169,8 +169,19 @@ do { \ 


#endif /* CONFIG_64BIT */



unsigned long __must_check __asm_copy_to_user_sum_enabled(void __user *to,

	const void *from, unsigned long n);

unsigned long __must_check __asm_copy_from_user_sum_enabled(void *to,

	const void __user *from, unsigned long n);



#define __get_user_nocheck(x, __gu_ptr, label)			\

do {								\

	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&	\

	    !IS_ALIGNED((uintptr_t)__gu_ptr, sizeof(*__gu_ptr))) {	\

		if (__asm_copy_from_user_sum_enabled(&(x), __gu_ptr, sizeof(*__gu_ptr))) \

			goto label;				\

		break;						\

	}							\

	switch (sizeof(*__gu_ptr)) {				\

	case 1:							\

		__get_user_asm("lb", (x), __gu_ptr, label);	\
@@ -297,6 +308,13 @@ do { \ 


#define __put_user_nocheck(x, __gu_ptr, label)			\

do {								\

	if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&	\

	    !IS_ALIGNED((uintptr_t)__gu_ptr, sizeof(*__gu_ptr))) {	\

		__inttype(x) val = (__inttype(x))x;			\

		if (__asm_copy_to_user_sum_enabled(__gu_ptr, &(val), sizeof(*__gu_ptr))) \

			goto label;				\

		break;						\

	}							\

	switch (sizeof(*__gu_ptr)) {				\

	case 1:							\

		__put_user_asm("sb", (x), __gu_ptr, label);	\
@@ -450,35 +468,13 @@ static inline void user_access_restore(unsigned long enabled) { } 
	(x) = (__force __typeof__(*(ptr)))__gu_val;			\

} while (0)



#define unsafe_copy_loop(dst, src, len, type, op, label)		\

	while (len >= sizeof(type)) {					\

		op(*(type *)(src), (type __user *)(dst), label);	\

		dst += sizeof(type);					\

		src += sizeof(type);					\

		len -= sizeof(type);					\

	}



#define unsafe_copy_to_user(_dst, _src, _len, label)			\

do {									\

	char __user *__ucu_dst = (_dst);				\

	const char *__ucu_src = (_src);					\

	size_t __ucu_len = (_len);					\

	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u64, unsafe_put_user, label);	\

	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u32, unsafe_put_user, label);	\

	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u16, unsafe_put_user, label);	\

	unsafe_copy_loop(__ucu_dst, __ucu_src, __ucu_len, u8, unsafe_put_user, label);	\

} while (0)

	if (__asm_copy_to_user_sum_enabled(_dst, _src, _len))		\

		goto label;



#define unsafe_copy_from_user(_dst, _src, _len, label)			\

do {									\

	char *__ucu_dst = (_dst);					\

	const char __user *__ucu_src = (_src);				\

	size_t __ucu_len = (_len);					\

	unsafe_copy_loop(__ucu_src, __ucu_dst, __ucu_len, u64, unsafe_get_user, label);	\

	unsafe_copy_loop(__ucu_src, __ucu_dst, __ucu_len, u32, unsafe_get_user, label);	\

	unsafe_copy_loop(__ucu_src, __ucu_dst, __ucu_len, u16, unsafe_get_user, label);	\

	unsafe_copy_loop(__ucu_src, __ucu_dst, __ucu_len, u8, unsafe_get_user, label);	\

} while (0)

	if (__asm_copy_from_user_sum_enabled(_dst, _src, _len))		\

		goto label;



#else /* CONFIG_MMU */

#include <asm-generic/uaccess.h>

arch/riscv/kernel/process.c

+1 −1
Original line number Diff line number Diff line
@@ -57,7 +57,7 @@ int get_unalign_ctl(struct task_struct *tsk, unsigned long adr) 
	if (!unaligned_ctl_available())

		return -EINVAL;



	return put_user(tsk->thread.align_ctl, (unsigned long __user *)adr);

	return put_user(tsk->thread.align_ctl, (unsigned int __user *)adr);

}



void __show_regs(struct pt_regs *regs)

arch/riscv/lib/riscv_v_helpers.c

+8 −3
Original line number Diff line number Diff line
@@ -16,8 +16,11 @@ 
#ifdef CONFIG_MMU

size_t riscv_v_usercopy_threshold = CONFIG_RISCV_ISA_V_UCOPY_THRESHOLD;

int __asm_vector_usercopy(void *dst, void *src, size_t n);

int __asm_vector_usercopy_sum_enabled(void *dst, void *src, size_t n);

int fallback_scalar_usercopy(void *dst, void *src, size_t n);

asmlinkage int enter_vector_usercopy(void *dst, void *src, size_t n)

int fallback_scalar_usercopy_sum_enabled(void *dst, void *src, size_t n);

asmlinkage int enter_vector_usercopy(void *dst, void *src, size_t n,

				     bool enable_sum)

{

	size_t remain, copied;
@@ -26,7 +29,8 @@ asmlinkage int enter_vector_usercopy(void *dst, void *src, size_t n) 
		goto fallback;



	kernel_vector_begin();

	remain = __asm_vector_usercopy(dst, src, n);

	remain = enable_sum ? __asm_vector_usercopy(dst, src, n) :

			      __asm_vector_usercopy_sum_enabled(dst, src, n);

	kernel_vector_end();



	if (remain) {
@@ -40,6 +44,7 @@ asmlinkage int enter_vector_usercopy(void *dst, void *src, size_t n) 
	return remain;



fallback:

	return fallback_scalar_usercopy(dst, src, n);

	return enable_sum ? fallback_scalar_usercopy(dst, src, n) :

			    fallback_scalar_usercopy_sum_enabled(dst, src, n);

}

#endif

arch/riscv/lib/uaccess.S

+34 −16
Original line number Diff line number Diff line
@@ -17,14 +17,43 @@ SYM_FUNC_START(__asm_copy_to_user) 
	ALTERNATIVE("j fallback_scalar_usercopy", "nop", 0, RISCV_ISA_EXT_ZVE32X, CONFIG_RISCV_ISA_V)

	REG_L	t0, riscv_v_usercopy_threshold

	bltu	a2, t0, fallback_scalar_usercopy

	li	a3, 1

	tail 	enter_vector_usercopy

#endif

SYM_FUNC_START(fallback_scalar_usercopy)

SYM_FUNC_END(__asm_copy_to_user)

EXPORT_SYMBOL(__asm_copy_to_user)

SYM_FUNC_ALIAS(__asm_copy_from_user, __asm_copy_to_user)

EXPORT_SYMBOL(__asm_copy_from_user)



SYM_FUNC_START(fallback_scalar_usercopy)

	/* Enable access to user memory */

	li	t6, SR_SUM

	csrs 	CSR_STATUS, t6

	mv 	t6, ra



	call 	fallback_scalar_usercopy_sum_enabled



	/* Disable access to user memory */

	mv 	ra, t6

	li 	t6, SR_SUM

	csrc 	CSR_STATUS, t6

	ret

SYM_FUNC_END(fallback_scalar_usercopy)



SYM_FUNC_START(__asm_copy_to_user_sum_enabled)

#ifdef CONFIG_RISCV_ISA_V

	ALTERNATIVE("j fallback_scalar_usercopy_sum_enabled", "nop", 0, RISCV_ISA_EXT_ZVE32X, CONFIG_RISCV_ISA_V)

	REG_L	t0, riscv_v_usercopy_threshold

	bltu	a2, t0, fallback_scalar_usercopy_sum_enabled

	li	a3, 0

	tail 	enter_vector_usercopy

#endif

SYM_FUNC_END(__asm_copy_to_user_sum_enabled)

SYM_FUNC_ALIAS(__asm_copy_from_user_sum_enabled, __asm_copy_to_user_sum_enabled)

EXPORT_SYMBOL(__asm_copy_from_user_sum_enabled)

EXPORT_SYMBOL(__asm_copy_to_user_sum_enabled)



SYM_FUNC_START(fallback_scalar_usercopy_sum_enabled)

	/*

	 * Save the terminal address which will be used to compute the number

	 * of bytes copied in case of a fixup exception.
@@ -178,23 +207,12 @@ SYM_FUNC_START(fallback_scalar_usercopy) 
	bltu	a0, t0, 4b	/* t0 - end of dst */



.Lout_copy_user:

	/* Disable access to user memory */

	csrc CSR_STATUS, t6

	li	a0, 0

	ret



	/* Exception fixup code */

10:

	/* Disable access to user memory */

	csrc CSR_STATUS, t6

	sub a0, t5, a0

	ret

SYM_FUNC_END(__asm_copy_to_user)

SYM_FUNC_END(fallback_scalar_usercopy)

EXPORT_SYMBOL(__asm_copy_to_user)

SYM_FUNC_ALIAS(__asm_copy_from_user, __asm_copy_to_user)

EXPORT_SYMBOL(__asm_copy_from_user)



SYM_FUNC_END(fallback_scalar_usercopy_sum_enabled)



SYM_FUNC_START(__clear_user)