From: Celeste Liu <coelacanthushex@gmail.com> To: "Palmer Dabbelt" <palmer@rivosinc.com>, "Paul Walmsley" <paul.walmsley@sifive.com>, "Albert Ou" <aou@eecs.berkeley.edu>, "Guo Ren" <guoren@kernel.org>, "Björn Töpel" <bjorn@rivosinc.com>, "Conor Dooley" <conor.dooley@microchip.com>, linux-riscv@lists.infradead.org Cc: linux-kernel@vger.kernel.org, Andreas Schwab <schwab@suse.de>, David Laight <David.Laight@ACULAB.COM>, Celeste Liu <CoelacanthusHex@gmail.com>, Felix Yan <felixonmars@archlinux.org>, Ruizhe Pan <c141028@gmail.com>, Shiqi Zhang <shiqi@isrc.iscas.ac.cn>, Emil Renner Berthing <emil.renner.berthing@canonical.com> Subject: [PATCH v5] riscv: entry: set a0 = -ENOSYS only when syscall != -1 Date: Tue, 1 Aug 2023 22:15:16 +0800 [thread overview] Message-ID: <20230801141607.435192-1-CoelacanthusHex@gmail.com> (raw) 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 <felixonmars@archlinux.org> Co-developed-by: Ruizhe Pan <c141028@gmail.com> Signed-off-by: Ruizhe Pan <c141028@gmail.com> Co-developed-by: Shiqi Zhang <shiqi@isrc.iscas.ac.cn> Signed-off-by: Shiqi Zhang <shiqi@isrc.iscas.ac.cn> Signed-off-by: Celeste Liu <CoelacanthusHex@gmail.com> Tested-by: Felix Yan <felixonmars@archlinux.org> Tested-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> Reviewed-by: Björn Töpel <bjorn@rivosinc.com> Reviewed-by: Guo Ren <guoren@kernel.org> --- v4 -> v5: add Tested-by Emil Renner Berthing <emil.renner.berthing@canonical.com> v3 -> v4: use long instead of ulong to reduce type cast and avoid implementation-defined behavior, and make the judgment of syscall invalid more explicit v2 -> v3: use if-statement instead of set default value, clarify the type of syscall v1 -> v2: added explanation on why always got ENOSYS 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 f910dfccbf5d2..729f79c97e2bf 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); -- 2.41.0
WARNING: multiple messages have this Message-ID (diff)
From: Celeste Liu <coelacanthushex@gmail.com> To: "Palmer Dabbelt" <palmer@rivosinc.com>, "Paul Walmsley" <paul.walmsley@sifive.com>, "Albert Ou" <aou@eecs.berkeley.edu>, "Guo Ren" <guoren@kernel.org>, "Björn Töpel" <bjorn@rivosinc.com>, "Conor Dooley" <conor.dooley@microchip.com>, linux-riscv@lists.infradead.org Cc: linux-kernel@vger.kernel.org, Andreas Schwab <schwab@suse.de>, David Laight <David.Laight@ACULAB.COM>, Celeste Liu <CoelacanthusHex@gmail.com>, Felix Yan <felixonmars@archlinux.org>, Ruizhe Pan <c141028@gmail.com>, Shiqi Zhang <shiqi@isrc.iscas.ac.cn>, Emil Renner Berthing <emil.renner.berthing@canonical.com> Subject: [PATCH v5] riscv: entry: set a0 = -ENOSYS only when syscall != -1 Date: Tue, 1 Aug 2023 22:15:16 +0800 [thread overview] Message-ID: <20230801141607.435192-1-CoelacanthusHex@gmail.com> (raw) 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 <felixonmars@archlinux.org> Co-developed-by: Ruizhe Pan <c141028@gmail.com> Signed-off-by: Ruizhe Pan <c141028@gmail.com> Co-developed-by: Shiqi Zhang <shiqi@isrc.iscas.ac.cn> Signed-off-by: Shiqi Zhang <shiqi@isrc.iscas.ac.cn> Signed-off-by: Celeste Liu <CoelacanthusHex@gmail.com> Tested-by: Felix Yan <felixonmars@archlinux.org> Tested-by: Emil Renner Berthing <emil.renner.berthing@canonical.com> Reviewed-by: Björn Töpel <bjorn@rivosinc.com> Reviewed-by: Guo Ren <guoren@kernel.org> --- v4 -> v5: add Tested-by Emil Renner Berthing <emil.renner.berthing@canonical.com> v3 -> v4: use long instead of ulong to reduce type cast and avoid implementation-defined behavior, and make the judgment of syscall invalid more explicit v2 -> v3: use if-statement instead of set default value, clarify the type of syscall v1 -> v2: added explanation on why always got ENOSYS 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 f910dfccbf5d2..729f79c97e2bf 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); -- 2.41.0 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next reply other threads:[~2023-08-01 14:16 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-08-01 14:15 Celeste Liu [this message] 2023-08-01 14:15 ` [PATCH v5] riscv: entry: set a0 = -ENOSYS only when syscall != -1 Celeste Liu 2023-08-17 15:20 ` patchwork-bot+linux-riscv 2023-08-17 15:20 ` patchwork-bot+linux-riscv
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20230801141607.435192-1-CoelacanthusHex@gmail.com \ --to=coelacanthushex@gmail.com \ --cc=David.Laight@ACULAB.COM \ --cc=aou@eecs.berkeley.edu \ --cc=bjorn@rivosinc.com \ --cc=c141028@gmail.com \ --cc=conor.dooley@microchip.com \ --cc=emil.renner.berthing@canonical.com \ --cc=felixonmars@archlinux.org \ --cc=guoren@kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=palmer@rivosinc.com \ --cc=paul.walmsley@sifive.com \ --cc=schwab@suse.de \ --cc=shiqi@isrc.iscas.ac.cn \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.