NetBSD expects this testcase to fail with SIGBUS, but instead it fails with SIGSEGV on FreeBSD. This test case is new to FreeBSD and only available on ^/projects/netbsd-tests-update-12 so far.. $ svn info /usr/src/svn/ | grep ^URL: URL: svn+ssh://ngie@repo.freebsd.org/base/projects/netbsd-tests-update-12 $ svnversion /usr/src/svn 304238M $ sudo kyua debug -k /usr/tests/lib/libc/sys/Kyuafile mmap_test:mmap_truncate_signal mmap_test:mmap_truncate_signal -> failed: child process got SIGSEGV instead of SIGBUS
A commit references this bug: Author: ngie Date: Wed Aug 17 06:41:06 UTC 2016 New revision: 304257 URL: https://svnweb.freebsd.org/changeset/base/304257 Log: Expect :mmap_truncate_signal to fail on FreeBSD Additional investigation is being done as part of bug 211924 PR: 211924 Sponsored by: EMC / Isilon Storage Division Changes: projects/netbsd-tests-update-12/contrib/netbsd-tests/lib/libc/sys/t_mmap.c
kib, the test here does a map = mmap(NULL, page, PROT_READ, MAP_FILE|MAP_PRIVATE, fd, 0); then fork() and tries to access the mapping in the child. The test expects SIGBUS but gets SIGSEGV. Perhaps the right change is to just expect SIGSEGV on FreeBSD but I wanted to check if you have an opinion on the choice of signal.
(In reply to Ed Maste from comment #2) We return SIGSEGV/SEGV_MAPERR, which is explained by POSIX as 'Address not mapped to object', which it is. FreeBSD used to deliver SIGBUS in this situation, but it was rightfully corrected to SIGSEGV by davidxu AFAIR, and then I introduced compat sysctls to select the specific signal to deliver. See the story in PR 118304.
Reopen for a change to the test case - we should remove the atf_tc_expect_fail("testcase fails with SIGSEGV on FreeBSD; bug # 211924"); and check for the desired signal on FreeBSD (or allow either SIGSEGV or SIGBUS).
I think FreeBSD is wrong here. Reading the mmap page of SUSv4tc2 XSH 3 System Interfaces: ] The system shall always zero-fill any partial page at the end of an ] object. Further, the system shall never write out any modified portions ] of the last page of an object which are beyond its end. References ] within the address range starting at pa and continuing for len bytes to ] whole pages following the end of an object shall result in delivery of a ] SIGBUS signal. As far as I understand, SEGV_MAPERR is for the case where no mapping exists for the address, and SEGV_ACCERR is for the case where the access conflicts with the prot parameter passed to mmap() (PROT_READ/PROT_WRITE/PROT_EXEC). The size of the mapping is equal to the len parameter passed to mmap(), whether the underlying object is that long or not. So this does not mean that the change mentioned in 118304 is completely wrong. There is only a problem for pages of the mapped region beyond the end of the object. By the way, the testcase is not POSIX-compliant either, since the aforementioned mmap page also says: ] If the size of the mapped file changes after the call to mmap() as a ] result of some other operation on the mapped file, the effect of ] references to portions of the mapped region that correspond to added or ] removed portions of the file is unspecified.
(In reply to Jilles Tjoelker from comment #5) I believe we are compliant with the behavior for the partial page. We always zero-fill invalid parts of the partially-valid pages that are mapped into userspace. Was there a history behind the SIGBUS requirement ? I believe that the change for SIGBUS->SIGSEGV came with r151316, and I cannot remember such explicit language at that time. Anyway, I am certain that we do not want to change the signal definitions back, because there are programs that depend on it and which already did the switch to accommodate to the change.
Here is a real-world example of code expecting SIGBUS: https://gitlab.freedesktop.org/xorg/xserver/blob/master/os/busfault.c (Note that this is the same "busfault" I referred to in the file sealing review https://reviews.freebsd.org/D21391 . I think preventing truncation via sealing is generally a better option than handling signals, but it may not always be possible.) Like the NetBSD testcase, this assumes that truncating after the mapping has been created behaves normally, as if the truncation has been done beforehand. Sending SIGSEGV where SIGBUS is expected will expose the X server to an easy DoS by a client truncating a file or posixshm mapped by the X server. This only applies to certain modern protocol extensions since older ones use SysV shared memory segments that have a fixed size. Accepting SIGSEGV in the X server code runs the risk of trying to "fix", for example, an incorrect write to a read-only mapping or a jump to a non-executable mapping by treating it as if the client unexpectedly truncated the object. Checking si_code does not help since FreeBSD currently sends SEGV_ACCERR in both situations. The signal definitions should indeed not be changed back to what they were. Instead, the number of possible signal/code combinations from page faults should be increased from two to three. In sys/amd64/amd64/trap.c, this corresponds to the return value from trap_pfault(). This will also need changes to sys/vm/vm_fault.c, since vm_fault() currently returns KERN_PROTECTION_FAILURE for both accesses violating the protection such as writes to a read-only mapping (which should result in SIGSEGV with code SEGV_ACCERR) and accesses inside a valid mapping but outside the underlying object (which should result in SIGBUS with code BUS_OBJERR).
https://reviews.freebsd.org/D21566
A commit references this bug: Author: jilles Date: Wed Sep 18 21:00:32 UTC 2019 New revision: 352495 URL: https://svnweb.freebsd.org/changeset/base/352495 Log: Add some tests for page fault signals and codes It is useful to have some tests for page fault signals. More tests would be useful but creating the conditions (such as various kinds of running out of memory and I/O errors) is more complicated. The tests page_fault_signal__bus_objerr_1 and page_fault_signal__bus_objerr_2 depend on https://reviews.freebsd.org/D21566 before they can pass. PR: 211924 Reviewed by: kib Differential Revision: https://reviews.freebsd.org/D21624 Changes: head/tests/sys/vm/Makefile head/tests/sys/vm/page_fault_signal.c
A commit references this bug: Author: kib Date: Fri Sep 27 18:43:39 UTC 2019 New revision: 352807 URL: https://svnweb.freebsd.org/changeset/base/352807 Log: Improve MD page fault handlers. Centralize calculation of signal and ucode delivered on unhandled page fault in new function vm_fault_trap(). MD trap_pfault() now almost always uses the signal numbers and error codes calculated in consistent MI way. This introduces the protection fault compatibility sysctls to all non-x86 architectures which did not have that bug, but apparently they were already much more wrong in selecting delivered signals on protection violations. Change the delivered signal for accesses to mapped area after the backing object was truncated. According to POSIX description for mmap(2): The system shall always zero-fill any partial page at the end of an object. Further, the system shall never write out any modified portions of the last page of an object which are beyond its end. References within the address range starting at pa and continuing for len bytes to whole pages following the end of an object shall result in delivery of a SIGBUS signal. An implementation may generate SIGBUS signals when a reference would cause an error in the mapped object, such as out-of-space condition. Adjust according to the description, keeping the existing compatibility code for SIGSEGV/SIGBUS on protection failures. For situations where kernel cannot handle page fault due to resource limit enforcement, SIGBUS with a new error code BUS_OBJERR is delivered. Also, provide a new error code SEGV_PKUERR for SIGSEGV on amd64 due to protection key access violation. vm_fault_hold() is renamed to vm_fault(). Fixed some nits in trap_pfault()s like mis-interpreting Mach errors as errnos. Removed unneeded truncations of the fault addresses reported by hardware. PR: 211924 Reviewed by: alc Discussed with: jilles, markj Sponsored by: The FreeBSD Foundation MFC after: 1 week Differential revision: https://reviews.freebsd.org/D21566 Changes: head/sys/amd64/amd64/trap.c head/sys/amd64/vmm/vmm.c head/sys/arm/arm/trap-v4.c head/sys/arm/arm/trap-v6.c head/sys/arm64/arm64/trap.c head/sys/i386/i386/trap.c head/sys/kern/sys_process.c head/sys/mips/mips/trap.c head/sys/powerpc/powerpc/trap.c head/sys/riscv/riscv/trap.c head/sys/sparc64/sparc64/trap.c head/sys/sys/signal.h head/sys/vm/vm_extern.h head/sys/vm/vm_fault.c head/sys/vm/vm_map.c head/sys/vm/vm_param.h
A commit references this bug: Author: jilles Date: Sun Sep 29 15:17:59 UTC 2019 New revision: 352869 URL: https://svnweb.freebsd.org/changeset/base/352869 Log: Adjust tests after page fault changes in r352807 Commit r352807 fixed various signal numbers and codes from page faults; adjust the tests so they expect the fixes to be present. PR: 211924 Changes: head/contrib/netbsd-tests/lib/libc/sys/t_mmap.c head/tests/sys/posixshm/posixshm_test.c head/tests/sys/vm/page_fault_signal.c
A commit references this bug: Author: kib Date: Fri Oct 4 12:18:06 UTC 2019 New revision: 353102 URL: https://svnweb.freebsd.org/changeset/base/353102 Log: MFC r352807: Improve MD page fault handlers. PR: 211924 Changes: _U stable/12/ stable/12/sys/amd64/amd64/trap.c stable/12/sys/amd64/vmm/vmm.c stable/12/sys/arm/arm/trap-v4.c stable/12/sys/arm/arm/trap-v6.c stable/12/sys/arm64/arm64/trap.c stable/12/sys/i386/i386/trap.c stable/12/sys/kern/sys_process.c stable/12/sys/mips/mips/trap.c stable/12/sys/powerpc/powerpc/trap.c stable/12/sys/riscv/riscv/trap.c stable/12/sys/sparc64/sparc64/trap.c stable/12/sys/sys/signal.h stable/12/sys/vm/vm_extern.h stable/12/sys/vm/vm_fault.c stable/12/sys/vm/vm_map.c stable/12/sys/vm/vm_param.h
A commit references this bug: Author: jilles Date: Sun Oct 6 20:36:26 UTC 2019 New revision: 353148 URL: https://svnweb.freebsd.org/changeset/base/353148 Log: MFC r352495,r352869: Adjust tests for page fault changes in r353102 PR: 211924 Changes: _U stable/12/ stable/12/contrib/netbsd-tests/lib/libc/sys/t_mmap.c stable/12/tests/sys/posixshm/posixshm_test.c stable/12/tests/sys/vm/Makefile stable/12/tests/sys/vm/page_fault_signal.c
(In reply to commit-hook from comment #11) Does this require updates to /usr/include/sys/signal.h or updates to the siginfo(3) man page for the values of si_code? This change does fix the undocumented return of T_PAGEFLT (=12) as an si_code in siginfo when signal is SIGBUS
(In reply to Nick Briggs from comment #14) I do not see why anything would need to be changed. I do not want to document the ABI compat sysctl. The only description that is missing is for BUS_OOMERR. https://reviews.freebsd.org/D24761