casaroli commented on issue #1329: URL: https://github.com/apache/nuttx/issues/1329#issuecomment-6278193936
A note on how a fault on a user address fits the current series. #20412 and #20418 check that a user pointer is in the user address window, and end the caller when it is a kernel address. A pointer in the window that has no page passes that check, for example `read(fd, (void *)0xf0000000, 1)` on qemu-armv8a:knsh. The kernel then faults at EL1 when it touches the pointer, and arm64 panics. #20822 is an arm64 change for that case, as a fallback: when the kernel takes a translation, access flag or permission fault at EL1 on a user address during a system call of a user task, the handler ends the caller with `_exit(SIGSEGV)`, the same result that `uaccess_fault()` gives for a kernel address. Faults on kernel addresses still panic. It touches `arch/arm64` (the fault handler and `dispatch_syscall()`), two labels in `libs/libc/machine/arm64/arch_memcpy.S`/`arch_memset.S`, and in two more commits `fs/vfs` (the stat family and `nx_vioctl()`), so it does not overlap the files of the series. Result on qemu-armv8a:knsh (HVF) with a test program that gives 11 system calls a bad user pointer, each in a forked child (read, write to a pipe, stat, ioctl, nanosleep, clock_gettime, sigprocmask, a read into program text, a read that runs from a mapped page into an unmapped page): - master: kernel panic on the first case (`common/arm64_fatal.c:696`). - with the change: each child ends with exit status 11 and NSH continues, 231 of 231 over 21 passes; ostest exits 0 on knsh and nsh. - kernel heap: no growth for 20 faulting children of any case (also `fstat()`, `statfs()` and `fstatfs()` with a bad buffer). A fault in the arm64 `memcpy()` or `memset()` returns from that function, so the system call finishes and releases its locks and references; the caller ends when the system call returns. The stat family now writes the user buffer with one `memcpy()` after it releases its references, and `ioctl(FIONBIO)` reads its `int` before it takes the file reference. FIONBIO is not one of the ioctls that #20418 copies in, so the two do not overlap. The limit: a fault in other code stops the system call in the middle, so a lock or a reference that it holds stays as it is, and the caller gets no `EFAULT`. A user-access layer with fault fixups (`copy_from_user()`-style helpers that return `-EFAULT`) can come later, path by path, starting with the #20418 wrappers. The kill path then stays as the fallback for code that touches user memory directly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
