Bug 211924 - lib/libc/sys/mmap_test:mmap_truncate_signal fails with SIGSEGV instead of SIGBUS
Summary: lib/libc/sys/mmap_test:mmap_truncate_signal fails with SIGSEGV instead of SIGBUS
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: tests (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-testing mailing list
Depends on:
Reported: 2016-08-17 06:38 UTC by Enji Cooper
Modified: 2019-09-18 23:40 UTC (History)
3 users (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Enji Cooper freebsd_committer 2016-08-17 06:38:02 UTC
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
$ 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
Comment 1 commit-hook freebsd_committer 2016-08-17 06:41:13 UTC
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

  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

Comment 2 Ed Maste freebsd_committer 2019-07-31 13:23:16 UTC
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.
Comment 3 Konstantin Belousov freebsd_committer 2019-08-02 16:46:51 UTC
(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.
Comment 4 Ed Maste freebsd_committer 2019-08-02 17:36:05 UTC
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).
Comment 5 Jilles Tjoelker freebsd_committer 2019-09-04 21:36:15 UTC
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.
Comment 6 Konstantin Belousov freebsd_committer 2019-09-06 12:54:00 UTC
(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.
Comment 7 Jilles Tjoelker freebsd_committer 2019-09-08 12:33:36 UTC
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).
Comment 8 Konstantin Belousov freebsd_committer 2019-09-08 16:33:56 UTC
Comment 9 commit-hook freebsd_committer 2019-09-18 23:40:10 UTC
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

  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