The helpers that are used to implement copy_from_kernel_nofault() and copy_to_kernel_nofault() cast a void* to a pointer to a wider type, which may result in alignment faults on ARM if the compiler decides to use double-word or multiple-word load/store instructions.
So use the unaligned accessors where needed: when the type's size > 1 and the input was not aligned already by the caller.
Cc: stable@vger.kernel.org Fixes: 2df4c9a741a0 ("ARM: 9112/1: uaccess: add __{get,put}_kernel_nofault") Signed-off-by: Ard Biesheuvel ardb@kernel.org --- arch/arm/include/asm/uaccess.h | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h index 36fbc3329252..32dbfd81f42a 100644 --- a/arch/arm/include/asm/uaccess.h +++ b/arch/arm/include/asm/uaccess.h @@ -11,6 +11,7 @@ #include <linux/string.h> #include <asm/memory.h> #include <asm/domain.h> +#include <asm/unaligned.h> #include <asm/unified.h> #include <asm/compiler.h>
@@ -497,7 +498,10 @@ do { \ } \ default: __err = __get_user_bad(); break; \ } \ - *(type *)(dst) = __val; \ + if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)) \ + put_unaligned(__val, (type *)(dst)); \ + else \ + *(type *)(dst) = __val; /* aligned by caller */ \ if (__err) \ goto err_label; \ } while (0) @@ -507,7 +511,9 @@ do { \ const type *__pk_ptr = (dst); \ unsigned long __dst = (unsigned long)__pk_ptr; \ int __err = 0; \ - type __val = *(type *)src; \ + type __val = IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) \ + ? get_unaligned((type *)(src)) \ + : *(type *)(src); /* aligned by caller */ \ switch (sizeof(type)) { \ case 1: __put_user_asm_byte(__val, __dst, __err, ""); break; \ case 2: __put_user_asm_half(__val, __dst, __err, ""); break; \
On Tue, Jan 18, 2022 at 9:28 AM Ard Biesheuvel ardb@kernel.org wrote:
The helpers that are used to implement copy_from_kernel_nofault() and copy_to_kernel_nofault() cast a void* to a pointer to a wider type, which may result in alignment faults on ARM if the compiler decides to use double-word or multiple-word load/store instructions.
So use the unaligned accessors where needed: when the type's size > 1 and the input was not aligned already by the caller.
Cc: stable@vger.kernel.org Fixes: 2df4c9a741a0 ("ARM: 9112/1: uaccess: add __{get,put}_kernel_nofault") Signed-off-by: Ard Biesheuvel ardb@kernel.org
Reviewed-by: Arnd Bergmann arnd@arndb.de
It took me a bit to see whythis works, maybe mention commit 2423de2e6f4d ("ARM: 9115/1: mm/maccess: fix unaligned copy_{from,to}_kernel_nofault") in the description for clarification.
Did you run into actual faults, or did you find this problem by reading the code?
Arnd
On Tue, 18 Jan 2022 at 09:38, Arnd Bergmann arnd@arndb.de wrote:
On Tue, Jan 18, 2022 at 9:28 AM Ard Biesheuvel ardb@kernel.org wrote:
The helpers that are used to implement copy_from_kernel_nofault() and copy_to_kernel_nofault() cast a void* to a pointer to a wider type, which may result in alignment faults on ARM if the compiler decides to use double-word or multiple-word load/store instructions.
So use the unaligned accessors where needed: when the type's size > 1 and the input was not aligned already by the caller.
Cc: stable@vger.kernel.org Fixes: 2df4c9a741a0 ("ARM: 9112/1: uaccess: add __{get,put}_kernel_nofault") Signed-off-by: Ard Biesheuvel ardb@kernel.org
Reviewed-by: Arnd Bergmann arnd@arndb.de
It took me a bit to see whythis works, maybe mention commit 2423de2e6f4d ("ARM: 9115/1: mm/maccess: fix unaligned copy_{from,to}_kernel_nofault") in the description for clarification.
Ack.
Did you run into actual faults, or did you find this problem by reading the code?
I was seeing actual faults:
[ 4.447003] copy_from_kernel_nofault from prepend+0x3c/0xb4 [ 4.453085] prepend from prepend_path+0x118/0x34c [ 4.457930] prepend_path from d_path+0x11c/0x184 [ 4.462656] d_path from proc_pid_readlink+0xbc/0x1d4 [ 4.467928] proc_pid_readlink from vfs_readlink+0xfc/0x110 [ 4.473740] vfs_readlink from do_readlinkat+0xb0/0x110 [ 4.479024] do_readlinkat from ret_fast_syscall+0x0/0x54
On Tue, 18 Jan 2022 at 09:41, Ard Biesheuvel ardb@kernel.org wrote:
On Tue, 18 Jan 2022 at 09:38, Arnd Bergmann arnd@arndb.de wrote:
On Tue, Jan 18, 2022 at 9:28 AM Ard Biesheuvel ardb@kernel.org wrote:
The helpers that are used to implement copy_from_kernel_nofault() and copy_to_kernel_nofault() cast a void* to a pointer to a wider type, which may result in alignment faults on ARM if the compiler decides to use double-word or multiple-word load/store instructions.
So use the unaligned accessors where needed: when the type's size > 1 and the input was not aligned already by the caller.
Cc: stable@vger.kernel.org Fixes: 2df4c9a741a0 ("ARM: 9112/1: uaccess: add __{get,put}_kernel_nofault") Signed-off-by: Ard Biesheuvel ardb@kernel.org
Reviewed-by: Arnd Bergmann arnd@arndb.de
It took me a bit to see whythis works, maybe mention commit 2423de2e6f4d ("ARM: 9115/1: mm/maccess: fix unaligned copy_{from,to}_kernel_nofault") in the description for clarification.
Ack.
I've dropped this into the patch system as #9719/1, with the above suggestions incorporated into the commit log.
Thanks,
Did you run into actual faults, or did you find this problem by reading the code?
I was seeing actual faults:
[ 4.447003] copy_from_kernel_nofault from prepend+0x3c/0xb4 [ 4.453085] prepend from prepend_path+0x118/0x34c [ 4.457930] prepend_path from d_path+0x11c/0x184 [ 4.462656] d_path from proc_pid_readlink+0xbc/0x1d4 [ 4.467928] proc_pid_readlink from vfs_readlink+0xfc/0x110 [ 4.473740] vfs_readlink from do_readlinkat+0xb0/0x110 [ 4.479024] do_readlinkat from ret_fast_syscall+0x0/0x54
linux-stable-mirror@lists.linaro.org
-
Ard Biesheuvel -
Arnd Bergmann