* [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() @ 2025-06-02 19:39 Clément Léger 2025-06-02 19:39 ` [PATCH v2 1/3] riscv: make unsafe user copy routines use existing assembly routines Clément Léger ` (4 more replies) 0 siblings, 5 replies; 9+ messages in thread From: Clément Léger @ 2025-06-02 19:39 UTC (permalink / raw) To: linux-riscv, linux-kernel Cc: Clément Léger, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Maciej W . Rozycki, David Laight 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. --- v2: - Use __inttype instead of unsigned long for pointer cast - Add Alex patch to make unsafe func use existing assembly. - Add missing EXPORT_SYMBOL(__asm_copy_to_user_sum_enabled) Alexandre Ghiti (1): riscv: make unsafe user copy routines use existing assembly routines Clément Léger (2): riscv: process: use unsigned int instead of unsigned long for put_user() riscv: uaccess: do not do misaligned accesses in get/put_user() arch/riscv/include/asm/asm-prototypes.h | 2 +- arch/riscv/include/asm/uaccess.h | 48 +++++++++++------------- arch/riscv/kernel/process.c | 2 +- arch/riscv/lib/riscv_v_helpers.c | 11 ++++-- arch/riscv/lib/uaccess.S | 50 +++++++++++++++++-------- arch/riscv/lib/uaccess_vector.S | 15 ++++++-- 6 files changed, 78 insertions(+), 50 deletions(-) -- 2.49.0 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 1/3] riscv: make unsafe user copy routines use existing assembly routines 2025-06-02 19:39 [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() Clément Léger @ 2025-06-02 19:39 ` Clément Léger 2025-06-02 19:39 ` [PATCH v2 2/3] riscv: process: use unsigned int instead of unsigned long for put_user() Clément Léger ` (3 subsequent siblings) 4 siblings, 0 replies; 9+ messages in thread From: Clément Léger @ 2025-06-02 19:39 UTC (permalink / raw) To: linux-riscv, linux-kernel Cc: Clément Léger, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Maciej W . Rozycki, David Laight, Alexandre Ghiti From: Alexandre Ghiti <alexghiti@rivosinc.com> The current implementation is underperforming and in addition, it triggers misaligned access traps on platforms which do not handle misaligned accesses in hardware. Use the existing assembly routines to solve both problems at once. Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com> --- arch/riscv/include/asm/asm-prototypes.h | 2 +- arch/riscv/include/asm/uaccess.h | 33 ++++------------ arch/riscv/lib/riscv_v_helpers.c | 11 ++++-- arch/riscv/lib/uaccess.S | 50 +++++++++++++++++-------- arch/riscv/lib/uaccess_vector.S | 15 ++++++-- 5 files changed, 63 insertions(+), 48 deletions(-) diff --git a/arch/riscv/include/asm/asm-prototypes.h b/arch/riscv/include/asm/asm-prototypes.h index cd627ec289f1..5d10edde6d17 100644 --- a/arch/riscv/include/asm/asm-prototypes.h +++ b/arch/riscv/include/asm/asm-prototypes.h @@ -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, diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h index 87d01168f80a..046de7ced09c 100644 --- a/arch/riscv/include/asm/uaccess.h +++ b/arch/riscv/include/asm/uaccess.h @@ -450,35 +450,18 @@ 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); \ - } +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 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> diff --git a/arch/riscv/lib/riscv_v_helpers.c b/arch/riscv/lib/riscv_v_helpers.c index be38a93cedae..7bbdfc6d4552 100644 --- a/arch/riscv/lib/riscv_v_helpers.c +++ b/arch/riscv/lib/riscv_v_helpers.c @@ -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 diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S index 6a9f116bb545..4efea1b3326c 100644 --- a/arch/riscv/lib/uaccess.S +++ b/arch/riscv/lib/uaccess.S @@ -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 - tail enter_vector_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 + 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) diff --git a/arch/riscv/lib/uaccess_vector.S b/arch/riscv/lib/uaccess_vector.S index 7c45f26de4f7..03b5560609a2 100644 --- a/arch/riscv/lib/uaccess_vector.S +++ b/arch/riscv/lib/uaccess_vector.S @@ -24,7 +24,18 @@ SYM_FUNC_START(__asm_vector_usercopy) /* Enable access to user memory */ li t6, SR_SUM csrs CSR_STATUS, t6 + mv t6, ra + call __asm_vector_usercopy_sum_enabled + + /* Disable access to user memory */ + mv ra, t6 + li t6, SR_SUM + csrc CSR_STATUS, t6 + ret +SYM_FUNC_END(__asm_vector_usercopy) + +SYM_FUNC_START(__asm_vector_usercopy_sum_enabled) loop: vsetvli iVL, iNum, e8, ELEM_LMUL_SETTING, ta, ma fixup vle8.v vData, (pSrc), 10f @@ -36,8 +47,6 @@ loop: /* Exception fixup for vector load is shared with normal exit */ 10: - /* Disable access to user memory */ - csrc CSR_STATUS, t6 mv a0, iNum ret @@ -49,4 +58,4 @@ loop: csrr t2, CSR_VSTART sub iNum, iNum, t2 j 10b -SYM_FUNC_END(__asm_vector_usercopy) +SYM_FUNC_END(__asm_vector_usercopy_sum_enabled) -- 2.49.0 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] riscv: process: use unsigned int instead of unsigned long for put_user() 2025-06-02 19:39 [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() Clément Léger 2025-06-02 19:39 ` [PATCH v2 1/3] riscv: make unsafe user copy routines use existing assembly routines Clément Léger @ 2025-06-02 19:39 ` Clément Léger 2025-06-04 11:55 ` Alexandre Ghiti 2025-06-02 19:39 ` [PATCH v2 3/3] riscv: uaccess: do not do misaligned accesses in get/put_user() Clément Léger ` (2 subsequent siblings) 4 siblings, 1 reply; 9+ messages in thread From: Clément Léger @ 2025-06-02 19:39 UTC (permalink / raw) To: linux-riscv, linux-kernel Cc: Clément Léger, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Maciej W . Rozycki, David Laight The specification of prctl() for GET_UNALIGN_CTL states that the value is returned in an unsigned int * address passed as an unsigned long. Change the type to match that and avoid an unaligned access as well. Signed-off-by: Clément Léger <cleger@rivosinc.com> --- arch/riscv/kernel/process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c index 15d8f75902f8..9ee6d816b98b 100644 --- a/arch/riscv/kernel/process.c +++ b/arch/riscv/kernel/process.c @@ -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) -- 2.49.0 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] riscv: process: use unsigned int instead of unsigned long for put_user() 2025-06-02 19:39 ` [PATCH v2 2/3] riscv: process: use unsigned int instead of unsigned long for put_user() Clément Léger @ 2025-06-04 11:55 ` Alexandre Ghiti 0 siblings, 0 replies; 9+ messages in thread From: Alexandre Ghiti @ 2025-06-04 11:55 UTC (permalink / raw) To: Clément Léger, linux-riscv, linux-kernel Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Maciej W . Rozycki, David Laight Hi Clément, On 6/2/25 21:39, Clément Léger wrote: > The specification of prctl() for GET_UNALIGN_CTL states that the value is > returned in an unsigned int * address passed as an unsigned long. Change > the type to match that and avoid an unaligned access as well. > > Signed-off-by: Clément Léger <cleger@rivosinc.com> > --- > arch/riscv/kernel/process.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c > index 15d8f75902f8..9ee6d816b98b 100644 > --- a/arch/riscv/kernel/process.c > +++ b/arch/riscv/kernel/process.c > @@ -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) Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> Thanks, Alex _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] riscv: uaccess: do not do misaligned accesses in get/put_user() 2025-06-02 19:39 [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() Clément Léger 2025-06-02 19:39 ` [PATCH v2 1/3] riscv: make unsafe user copy routines use existing assembly routines Clément Léger 2025-06-02 19:39 ` [PATCH v2 2/3] riscv: process: use unsigned int instead of unsigned long for put_user() Clément Léger @ 2025-06-02 19:39 ` Clément Léger 2025-06-04 11:56 ` Alexandre Ghiti 2025-06-02 21:08 ` [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() David Laight 2025-06-05 18:50 ` patchwork-bot+linux-riscv 4 siblings, 1 reply; 9+ messages in thread From: Clément Léger @ 2025-06-02 19:39 UTC (permalink / raw) To: linux-riscv, linux-kernel Cc: Clément Léger, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Maciej W . Rozycki, David Laight Doing misaligned access to userspace memory would make a trap on platform where it is emulated. Latest fixes removed the kernel capability to do unaligned accesses to userspace memory safely since interrupts are kept disabled at all time during that. Thus doing so would crash the kernel. Such behavior was detected with GET_UNALIGN_CTL() that was doing a put_user() with an unsigned long* address that should have been an unsigned int*. Reenabling kernel misaligned access emulation is a bit risky and it would also degrade performances. Rather than doing that, we will try to avoid any misaligned accessed by using copy_from/to_user() which does not do any misaligned accesses. This can be done only for !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS and thus allows to only generate a bit more code for this config. Signed-off-by: Clément Léger <cleger@rivosinc.com> --- arch/riscv/include/asm/uaccess.h | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h index 046de7ced09c..d472da4450e6 100644 --- a/arch/riscv/include/asm/uaccess.h +++ b/arch/riscv/include/asm/uaccess.h @@ -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,11 +468,6 @@ static inline void user_access_restore(unsigned long enabled) { } (x) = (__force __typeof__(*(ptr)))__gu_val; \ } while (0) -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 unsafe_copy_to_user(_dst, _src, _len, label) \ if (__asm_copy_to_user_sum_enabled(_dst, _src, _len)) \ goto label; -- 2.49.0 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 3/3] riscv: uaccess: do not do misaligned accesses in get/put_user() 2025-06-02 19:39 ` [PATCH v2 3/3] riscv: uaccess: do not do misaligned accesses in get/put_user() Clément Léger @ 2025-06-04 11:56 ` Alexandre Ghiti 0 siblings, 0 replies; 9+ messages in thread From: Alexandre Ghiti @ 2025-06-04 11:56 UTC (permalink / raw) To: Clément Léger, linux-riscv, linux-kernel Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Maciej W . Rozycki, David Laight On 6/2/25 21:39, Clément Léger wrote: > Doing misaligned access to userspace memory would make a trap on > platform where it is emulated. Latest fixes removed the kernel > capability to do unaligned accesses to userspace memory safely since > interrupts are kept disabled at all time during that. Thus doing so > would crash the kernel. > > Such behavior was detected with GET_UNALIGN_CTL() that was doing > a put_user() with an unsigned long* address that should have been an > unsigned int*. Reenabling kernel misaligned access emulation is a bit > risky and it would also degrade performances. Rather than doing that, > we will try to avoid any misaligned accessed by using copy_from/to_user() > which does not do any misaligned accesses. This can be done only for > !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS and thus allows to only generate > a bit more code for this config. > > Signed-off-by: Clément Léger <cleger@rivosinc.com> > --- > arch/riscv/include/asm/uaccess.h | 23 ++++++++++++++++++----- > 1 file changed, 18 insertions(+), 5 deletions(-) > > diff --git a/arch/riscv/include/asm/uaccess.h b/arch/riscv/include/asm/uaccess.h > index 046de7ced09c..d472da4450e6 100644 > --- a/arch/riscv/include/asm/uaccess.h > +++ b/arch/riscv/include/asm/uaccess.h > @@ -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,11 +468,6 @@ static inline void user_access_restore(unsigned long enabled) { } > (x) = (__force __typeof__(*(ptr)))__gu_val; \ > } while (0) > > -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 unsafe_copy_to_user(_dst, _src, _len, label) \ > if (__asm_copy_to_user_sum_enabled(_dst, _src, _len)) \ > goto label; Even if as noted by David, there is room for improvement, for now the simplicity of this is good enough to me, so: Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> Thanks, Alex _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() 2025-06-02 19:39 [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() Clément Léger ` (2 preceding siblings ...) 2025-06-02 19:39 ` [PATCH v2 3/3] riscv: uaccess: do not do misaligned accesses in get/put_user() Clément Léger @ 2025-06-02 21:08 ` David Laight 2025-06-03 7:32 ` Clément Léger 2025-06-05 18:50 ` patchwork-bot+linux-riscv 4 siblings, 1 reply; 9+ messages in thread From: David Laight @ 2025-06-02 21:08 UTC (permalink / raw) To: Clément Léger Cc: linux-riscv, linux-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Maciej W . Rozycki On Mon, 2 Jun 2025 21:39:13 +0200 Clément Léger <cleger@rivosinc.com> wrote: ... > 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()) You can't do RMW because it is visible to other threads. David _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() 2025-06-02 21:08 ` [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() David Laight @ 2025-06-03 7:32 ` Clément Léger 0 siblings, 0 replies; 9+ messages in thread From: Clément Léger @ 2025-06-03 7:32 UTC (permalink / raw) To: David Laight Cc: linux-riscv, linux-kernel, Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti, Maciej W . Rozycki On 02/06/2025 23:08, David Laight wrote: > On Mon, 2 Jun 2025 21:39:13 +0200 > Clément Léger <cleger@rivosinc.com> wrote: > > ... >> 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()) > > You can't do RMW because it is visible to other threads. Ahem yeah indeed, so the only way was to do individual lb at least up to some alignment. Thanks, Clément > > David _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() 2025-06-02 19:39 [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() Clément Léger ` (3 preceding siblings ...) 2025-06-02 21:08 ` [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() David Laight @ 2025-06-05 18:50 ` patchwork-bot+linux-riscv 4 siblings, 0 replies; 9+ messages in thread From: patchwork-bot+linux-riscv @ 2025-06-05 18:50 UTC (permalink / raw) To: =?utf-8?b?Q2zDqW1lbnQgTMOpZ2VyIDxjbGVnZXJAcml2b3NpbmMuY29tPg==?= Cc: linux-riscv, linux-kernel, paul.walmsley, palmer, aou, alex, macro, david.laight.linux Hello: This series was applied to riscv/linux.git (for-next) by Palmer Dabbelt <palmer@dabbelt.com>: On Mon, 2 Jun 2025 21:39:13 +0200 you wrote: > 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. > > [...] Here is the summary with links: - [v2,1/3] riscv: make unsafe user copy routines use existing assembly routines https://git.kernel.org/riscv/c/a4348546332c - [v2,2/3] riscv: process: use unsigned int instead of unsigned long for put_user() https://git.kernel.org/riscv/c/020667d661f9 - [v2,3/3] riscv: uaccess: do not do misaligned accesses in get/put_user() https://git.kernel.org/riscv/c/ca1a66cdd685 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-06-05 18:50 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-06-02 19:39 [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() Clément Léger 2025-06-02 19:39 ` [PATCH v2 1/3] riscv: make unsafe user copy routines use existing assembly routines Clément Léger 2025-06-02 19:39 ` [PATCH v2 2/3] riscv: process: use unsigned int instead of unsigned long for put_user() Clément Léger 2025-06-04 11:55 ` Alexandre Ghiti 2025-06-02 19:39 ` [PATCH v2 3/3] riscv: uaccess: do not do misaligned accesses in get/put_user() Clément Léger 2025-06-04 11:56 ` Alexandre Ghiti 2025-06-02 21:08 ` [PATCH v2 0/3] riscv: misaligned: fix misaligned accesses handling in put/get_user() David Laight 2025-06-03 7:32 ` Clément Léger 2025-06-05 18:50 ` patchwork-bot+linux-riscv
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).