The risc-v kernel copyin() routine loops forever if the user passs a pointer with the high bit set. I"m using qemu 5.2.0 to run this image: https://download.freebsd.org/ftp/snapshots/VM-IMAGES/14.0-CURRENT/riscv64/Latest/FreeBSD-14.0-CURRENT-riscv-riscv64.raw.xz I compile and run this program: #include <fcntl.h> main() { fcntl(1, F_GETLK, 0x800000c000000000); } The kernel fcntl calls copyin(), which never returns. I'm guessing that copyin's bgt in copyinout.S should be a bgtu: ENTRY(copyin) beqz a2, copyin_end /* If len == 0 then skip loop */ add a3, a0, a2 li a4, VM_MAXUSER_ADDRESS bgt a3, a4, copyio_fault_nopcb ...
Thanks for the report, indeed the use of bgt seems inappropriate here. I will look at this in detail shortly.
Also, creat(0x0000004000000000UL,0) causes copyinstr() to loop forever. Perhaps bgeu rather that bgt.
(In reply to Robert Morris from comment #2) I have posted a review for the fix, which uses the bgeu instruction: https://reviews.freebsd.org/D31209 I was wondering why these cases would loop continuously, rather than panic the system with a fatal page fault. I was able to track this down too: https://reviews.freebsd.org/D31208 So, thanks again for the detailed report. Out of curiosity, did you find the issue by inspection, or did it manifest in some real program or test?
(In reply to Mitchell Horne from comment #3) I found the bug using a symbolic execution system for risc-v that I've been working on.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=8babb5582eed2250309084d76898798409a2aae0 commit 8babb5582eed2250309084d76898798409a2aae0 Author: Mitchell Horne <mhorne@FreeBSD.org> AuthorDate: 2021-10-07 21:12:30 +0000 Commit: Mitchell Horne <mhorne@FreeBSD.org> CommitDate: 2021-10-07 21:12:30 +0000 riscv: fix VM_MAXUSER_ADDRESS checks in asm routines There are two issues with the checks against VM_MAXUSER_ADDRESS. First, the comparison should consider the values as unsigned, otherwise addresses with the high bit set will fail to branch. Second, the value of VM_MAXUSER_ADDRESS is, by convention, one larger than the maximum mappable user address and invalid itself. Thus, use the bgeu instruction for these comparisons. Add a regression test case for copyin(9). PR: 257193 Reported by: Robert Morris <rtm@lcs.mit.edu> Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D31209 sys/riscv/riscv/copyinout.S | 6 +++--- sys/riscv/riscv/support.S | 20 ++++++++++---------- tests/sys/kern/kern_copyin.c | 24 ++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 13 deletions(-)
A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=d925085dcdbd9dd3953dc1a1c726df8d359705b2 commit d925085dcdbd9dd3953dc1a1c726df8d359705b2 Author: Mitchell Horne <mhorne@FreeBSD.org> AuthorDate: 2021-10-07 21:12:30 +0000 Commit: Mitchell Horne <mhorne@FreeBSD.org> CommitDate: 2021-10-15 15:22:13 +0000 riscv: fix VM_MAXUSER_ADDRESS checks in asm routines There are two issues with the checks against VM_MAXUSER_ADDRESS. First, the comparison should consider the values as unsigned, otherwise addresses with the high bit set will fail to branch. Second, the value of VM_MAXUSER_ADDRESS is, by convention, one larger than the maximum mappable user address and invalid itself. Thus, use the bgeu instruction for these comparisons. Add a regression test case for copyin(9). PR: 257193 Reported by: Robert Morris <rtm@lcs.mit.edu> Reviewed by: markj Differential Revision: https://reviews.freebsd.org/D31209 (cherry picked from commit 8babb5582eed2250309084d76898798409a2aae0) sys/riscv/riscv/copyinout.S | 6 +++--- sys/riscv/riscv/support.S | 20 ++++++++++---------- tests/sys/kern/kern_copyin.c | 24 ++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 13 deletions(-)