From 52449c17bdd1540940e21511612b58acebc49c06 Mon Sep 17 00:00:00 2001 From: Celeste Liu Date: Tue, 1 Aug 2023 22:15:16 +0800 Subject: [PATCH 1/6] riscv: entry: set a0 = -ENOSYS only when syscall != -1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When we test seccomp with 6.4 kernel, we found errno has wrong value. If we deny NETLINK_AUDIT with EAFNOSUPPORT, after f0bddf50586d, we will get ENOSYS instead. We got same result with commit 9c2598d43510 ("riscv: entry: Save a0 prior syscall_enter_from_user_mode()"). After analysing code, we think that regs->a0 = -ENOSYS should only be executed when syscall != -1. In __seccomp_filter, when seccomp rejected this syscall with specified errno, they will set a0 to return number as syscall ABI, and then return -1. This return number is finally pass as return number of syscall_enter_from_user_mode, and then is compared with NR_syscalls after converted to ulong (so it will be ULONG_MAX). The condition syscall < NR_syscalls will always be false, so regs->a0 = -ENOSYS is always executed. It covered a0 set by seccomp, so we always get ENOSYS when match seccomp RET_ERRNO rule. Fixes: f0bddf50586d ("riscv: entry: Convert to generic entry") Reported-by: Felix Yan Co-developed-by: Ruizhe Pan Signed-off-by: Ruizhe Pan Co-developed-by: Shiqi Zhang Signed-off-by: Shiqi Zhang Signed-off-by: Celeste Liu Tested-by: Felix Yan Tested-by: Emil Renner Berthing Reviewed-by: Björn Töpel Reviewed-by: Guo Ren Link: https://lore.kernel.org/r/20230801141607.435192-1-CoelacanthusHex@gmail.com Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/traps.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index f910dfccbf5d..729f79c97e2b 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -297,7 +297,7 @@ asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs) asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs) { if (user_mode(regs)) { - ulong syscall = regs->a7; + long syscall = regs->a7; regs->epc += 4; regs->orig_a0 = regs->a0; @@ -306,9 +306,9 @@ asmlinkage __visible __trap_section void do_trap_ecall_u(struct pt_regs *regs) syscall = syscall_enter_from_user_mode(regs, syscall); - if (syscall < NR_syscalls) + if (syscall >= 0 && syscall < NR_syscalls) syscall_handler(regs, syscall); - else + else if (syscall != -1) regs->a0 = -ENOSYS; syscall_exit_to_user_mode(regs); From 79bc3f85c51fc352f8e684ba6b626f677a3aa230 Mon Sep 17 00:00:00 2001 From: Nam Cao Date: Mon, 31 Jul 2023 20:39:25 +0200 Subject: [PATCH 2/6] riscv: correct riscv_insn_is_c_jr() and riscv_insn_is_c_jalr() The instructions c.jr and c.jalr must have rs1 != 0, but riscv_insn_is_c_jr() and riscv_insn_is_c_jalr() do not check for this. So, riscv_insn_is_c_jr() can match a reserved encoding, while riscv_insn_is_c_jalr() can match the c.ebreak instruction. Rewrite them with check for rs1 != 0. Signed-off-by: Nam Cao Reviewed-by: Charlie Jenkins Fixes: ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header") Link: https://lore.kernel.org/r/20230731183925.152145-1-namcaov@gmail.com Signed-off-by: Palmer Dabbelt --- arch/riscv/include/asm/insn.h | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h index 4e1505cef8aa..fce00400c9bc 100644 --- a/arch/riscv/include/asm/insn.h +++ b/arch/riscv/include/asm/insn.h @@ -110,6 +110,7 @@ #define RVC_INSN_FUNCT4_OPOFF 12 #define RVC_INSN_FUNCT3_MASK GENMASK(15, 13) #define RVC_INSN_FUNCT3_OPOFF 13 +#define RVC_INSN_J_RS1_MASK GENMASK(11, 7) #define RVC_INSN_J_RS2_MASK GENMASK(6, 2) #define RVC_INSN_OPCODE_MASK GENMASK(1, 0) #define RVC_ENCODE_FUNCT3(f_) (RVC_FUNCT3_##f_ << RVC_INSN_FUNCT3_OPOFF) @@ -245,8 +246,6 @@ __RISCV_INSN_FUNCS(c_jal, RVC_MASK_C_JAL, RVC_MATCH_C_JAL) __RISCV_INSN_FUNCS(auipc, RVG_MASK_AUIPC, RVG_MATCH_AUIPC) __RISCV_INSN_FUNCS(jalr, RVG_MASK_JALR, RVG_MATCH_JALR) __RISCV_INSN_FUNCS(jal, RVG_MASK_JAL, RVG_MATCH_JAL) -__RISCV_INSN_FUNCS(c_jr, RVC_MASK_C_JR, RVC_MATCH_C_JR) -__RISCV_INSN_FUNCS(c_jalr, RVC_MASK_C_JALR, RVC_MATCH_C_JALR) __RISCV_INSN_FUNCS(c_j, RVC_MASK_C_J, RVC_MATCH_C_J) __RISCV_INSN_FUNCS(beq, RVG_MASK_BEQ, RVG_MATCH_BEQ) __RISCV_INSN_FUNCS(bne, RVG_MASK_BNE, RVG_MATCH_BNE) @@ -273,6 +272,18 @@ static __always_inline bool riscv_insn_is_branch(u32 code) return (code & RV_INSN_OPCODE_MASK) == RVG_OPCODE_BRANCH; } +static __always_inline bool riscv_insn_is_c_jr(u32 code) +{ + return (code & RVC_MASK_C_JR) == RVC_MATCH_C_JR && + (code & RVC_INSN_J_RS1_MASK) != 0; +} + +static __always_inline bool riscv_insn_is_c_jalr(u32 code) +{ + return (code & RVC_MASK_C_JALR) == RVC_MATCH_C_JALR && + (code & RVC_INSN_J_RS1_MASK) != 0; +} + #define RV_IMM_SIGN(x) (-(((x) >> 31) & 1)) #define RVC_IMM_SIGN(x) (-(((x) >> 12) & 1)) #define RV_X(X, s, mask) (((X) >> (s)) & (mask)) From 8d0be64154cf24660a947b84340e8d5bb1af855a Mon Sep 17 00:00:00 2001 From: Guo Ren Date: Sat, 15 Jul 2023 20:15:05 -0400 Subject: [PATCH 3/6] riscv: stack: Fixup independent irq stack for CONFIG_FRAME_POINTER=n The independent irq stack uses s0 to save & restore sp, but s0 would be corrupted when CONFIG_FRAME_POINTER=n. So add s0 in the clobber list to fix the problem. Fixes: 163e76cc6ef4 ("riscv: stack: Support HAVE_IRQ_EXIT_ON_IRQ_STACK") Cc: stable@vger.kernel.org Reported-by: Zhangjin Wu Signed-off-by: Guo Ren Signed-off-by: Guo Ren Tested-by: Drew Fustini Link: https://lore.kernel.org/r/20230716001506.3506041-2-guoren@kernel.org Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/traps.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 729f79c97e2b..f798c853bede 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -372,6 +372,9 @@ asmlinkage void noinstr do_irq(struct pt_regs *regs) : [sp] "r" (sp), [regs] "r" (regs) : "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7", "t0", "t1", "t2", "t3", "t4", "t5", "t6", +#ifndef CONFIG_FRAME_POINTER + "s0", +#endif "memory"); } else #endif From ebc9cb03b21e263db607a7604134a195aa314ebb Mon Sep 17 00:00:00 2001 From: Guo Ren Date: Sat, 15 Jul 2023 20:15:06 -0400 Subject: [PATCH 4/6] riscv: stack: Fixup independent softirq stack for CONFIG_FRAME_POINTER=n The independent softirq stack uses s0 to save & restore sp, but s0 would be corrupted when CONFIG_FRAME_POINTER=n. So add s0 in the clobber list to fix the problem. Fixes: dd69d07a5a6c ("riscv: stack: Support HAVE_SOFTIRQ_ON_OWN_STACK") Cc: stable@vger.kernel.org Reported-by: Zhangjin Wu Signed-off-by: Guo Ren Signed-off-by: Guo Ren Tested-by: Drew Fustini Link: https://lore.kernel.org/r/20230716001506.3506041-3-guoren@kernel.org Signed-off-by: Palmer Dabbelt --- arch/riscv/kernel/irq.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c index d0577cc6a081..a8efa053c4a5 100644 --- a/arch/riscv/kernel/irq.c +++ b/arch/riscv/kernel/irq.c @@ -84,6 +84,9 @@ void do_softirq_own_stack(void) : [sp] "r" (sp) : "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7", "t0", "t1", "t2", "t3", "t4", "t5", "t6", +#ifndef CONFIG_FRAME_POINTER + "s0", +#endif "memory"); } else #endif From 4b05b993900dd3eba0fc83ef5c5ddc7d65d786c6 Mon Sep 17 00:00:00 2001 From: Alexandre Ghiti Date: Fri, 11 Aug 2023 17:06:04 +0200 Subject: [PATCH 5/6] riscv: uaccess: Return the number of bytes effectively not copied MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was reported that the riscv kernel hangs while executing the test in [1]. Indeed, the test hangs when trying to write a buffer to a file. The problem is that the riscv implementation of raw_copy_from_user() does not return the correct number of bytes not written when an exception happens and is fixed up, instead it always returns the initial size to copy, even if some bytes were actually copied. generic_perform_write() pre-faults the user pages and bails out if nothing can be written, otherwise it will access the userspace buffer: here the riscv implementation keeps returning it was not able to copy any byte though the pre-faulting indicates otherwise. So generic_perform_write() keeps retrying to access the user memory and ends up in an infinite loop. Note that before the commit mentioned in [1] that introduced this regression, it worked because generic_perform_write() would bail out if only one byte could not be written. So fix this by returning the number of bytes effectively not written in __asm_copy_[to|from]_user() and __clear_user(), as it is expected. Link: https://lore.kernel.org/linux-riscv/20230309151841.bomov6hq3ybyp42a@debian/ [1] Fixes: ebcbd75e3962 ("riscv: Fix the bug in memory access fixup code") Reported-by: Bo YU Closes: https://lore.kernel.org/linux-riscv/20230309151841.bomov6hq3ybyp42a@debian/#t Reported-by: Aurelien Jarno Closes: https://lore.kernel.org/linux-riscv/ZNOnCakhwIeue3yr@aurel32.net/ Signed-off-by: Alexandre Ghiti Reviewed-by: Björn Töpel Tested-by: Aurelien Jarno Reviewed-by: Aurelien Jarno Link: https://lore.kernel.org/r/20230811150604.1621784-1-alexghiti@rivosinc.com Signed-off-by: Palmer Dabbelt --- arch/riscv/lib/uaccess.S | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S index ec486e5369d9..09b47ebacf2e 100644 --- a/arch/riscv/lib/uaccess.S +++ b/arch/riscv/lib/uaccess.S @@ -17,8 +17,11 @@ ENTRY(__asm_copy_from_user) li t6, SR_SUM csrs CSR_STATUS, t6 - /* Save for return value */ - mv t5, a2 + /* + * Save the terminal address which will be used to compute the number + * of bytes copied in case of a fixup exception. + */ + add t5, a0, a2 /* * Register allocation for code below: @@ -176,7 +179,7 @@ ENTRY(__asm_copy_from_user) 10: /* Disable access to user memory */ csrc CSR_STATUS, t6 - mv a0, t5 + sub a0, t5, a0 ret ENDPROC(__asm_copy_to_user) ENDPROC(__asm_copy_from_user) @@ -228,7 +231,7 @@ ENTRY(__clear_user) 11: /* Disable access to user memory */ csrc CSR_STATUS, t6 - mv a0, a1 + sub a0, a3, a0 ret ENDPROC(__clear_user) EXPORT_SYMBOL(__clear_user) From ca09f772cccaeec4cd05a21528c37a260aa2dd2c Mon Sep 17 00:00:00 2001 From: Mingzheng Xing Date: Thu, 10 Aug 2023 00:56:48 +0800 Subject: [PATCH 6/6] riscv: Handle zicsr/zifencei issue between gcc and binutils Binutils-2.38 and GCC-12.1.0 bumped[0][1] the default ISA spec to the newer 20191213 version which moves some instructions from the I extension to the Zicsr and Zifencei extensions. So if one of the binutils and GCC exceeds that version, we should explicitly specifying Zicsr and Zifencei via -march to cope with the new changes. but this only occurs when binutils >= 2.36 and GCC >= 11.1.0. It's a different story when binutils < 2.36. binutils-2.36 supports the Zifencei extension[2] and splits Zifencei and Zicsr from I[3]. GCC-11.1.0 is particular[4] because it add support Zicsr and Zifencei extension for -march. binutils-2.35 does not support the Zifencei extension, and does not need to specify Zicsr and Zifencei when working with GCC >= 12.1.0. To make our lives easier, let's relax the check to binutils >= 2.36 in CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. For the other two cases, where clang < 17 or GCC < 11.1.0, we will deal with them in CONFIG_TOOLCHAIN_NEEDS_OLD_ISA_SPEC. For more information, please refer to: commit 6df2a016c0c8 ("riscv: fix build with binutils 2.38") commit e89c2e815e76 ("riscv: Handle zicsr/zifencei issues between clang and binutils") Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc [0] Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=98416dbb0a62579d4a7a4a76bab51b5b52fec2cd [1] Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=5a1b31e1e1cee6e9f1c92abff59cdcfff0dddf30 [2] Link: https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=729a53530e86972d1143553a415db34e6e01d5d2 [3] Link: https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b03be74bad08c382da47e048007a78fa3fb4ef49 [4] Link: https://lore.kernel.org/all/20230308220842.1231003-1-conor@kernel.org Link: https://lore.kernel.org/all/20230223220546.52879-1-conor@kernel.org Reviewed-by: Conor Dooley Acked-by: Guo Ren Cc: Signed-off-by: Mingzheng Xing Link: https://lore.kernel.org/r/20230809165648.21071-1-xingmingzheng@iscas.ac.cn Signed-off-by: Palmer Dabbelt --- arch/riscv/Kconfig | 28 ++++++++++++++++---------- arch/riscv/kernel/compat_vdso/Makefile | 8 +++++++- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 4c07b9189c86..10e7a7ad175a 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -570,24 +570,30 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE config TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI def_bool y # https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=aed44286efa8ae8717a77d94b51ac3614e2ca6dc - depends on AS_IS_GNU && AS_VERSION >= 23800 + # https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=98416dbb0a62579d4a7a4a76bab51b5b52fec2cd + depends on AS_IS_GNU && AS_VERSION >= 23600 help - Newer binutils versions default to ISA spec version 20191213 which - moves some instructions from the I extension to the Zicsr and Zifencei - extensions. + Binutils-2.38 and GCC-12.1.0 bumped the default ISA spec to the newer + 20191213 version, which moves some instructions from the I extension to + the Zicsr and Zifencei extensions. This requires explicitly specifying + Zicsr and Zifencei when binutils >= 2.38 or GCC >= 12.1.0. Zicsr + and Zifencei are supported in binutils from version 2.36 onwards. + To make life easier, and avoid forcing toolchains that default to a + newer ISA spec to version 2.2, relax the check to binutils >= 2.36. + For clang < 17 or GCC < 11.1.0, for which this is not possible, this is + dealt with in CONFIG_TOOLCHAIN_NEEDS_OLD_ISA_SPEC. config TOOLCHAIN_NEEDS_OLD_ISA_SPEC def_bool y depends on TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI # https://github.com/llvm/llvm-project/commit/22e199e6afb1263c943c0c0d4498694e15bf8a16 - depends on CC_IS_CLANG && CLANG_VERSION < 170000 + # https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=b03be74bad08c382da47e048007a78fa3fb4ef49 + depends on (CC_IS_CLANG && CLANG_VERSION < 170000) || (CC_IS_GCC && GCC_VERSION < 110100) help - Certain versions of clang do not support zicsr and zifencei via -march - but newer versions of binutils require it for the reasons noted in the - help text of CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI. This - option causes an older ISA spec compatible with these older versions - of clang to be passed to GAS, which has the same result as passing zicsr - and zifencei to -march. + Certain versions of clang and GCC do not support zicsr and zifencei via + -march. This option causes an older ISA spec compatible with these older + versions of clang and GCC to be passed to GAS, which has the same result + as passing zicsr and zifencei to -march. config FPU bool "FPU support" diff --git a/arch/riscv/kernel/compat_vdso/Makefile b/arch/riscv/kernel/compat_vdso/Makefile index 189345773e7e..b86e5e2c3aea 100644 --- a/arch/riscv/kernel/compat_vdso/Makefile +++ b/arch/riscv/kernel/compat_vdso/Makefile @@ -11,7 +11,13 @@ compat_vdso-syms += flush_icache COMPAT_CC := $(CC) COMPAT_LD := $(LD) -COMPAT_CC_FLAGS := -march=rv32g -mabi=ilp32 +# binutils 2.35 does not support the zifencei extension, but in the ISA +# spec 20191213, G stands for IMAFD_ZICSR_ZIFENCEI. +ifdef CONFIG_TOOLCHAIN_NEEDS_EXPLICIT_ZICSR_ZIFENCEI + COMPAT_CC_FLAGS := -march=rv32g -mabi=ilp32 +else + COMPAT_CC_FLAGS := -march=rv32imafd -mabi=ilp32 +endif COMPAT_LD_FLAGS := -melf32lriscv # Disable attributes, as they're useless and break the build.