Author: kevans
Date: Tue Apr 14 13:32:03 2020
New Revision: 359918
URL: https://svnweb.freebsd.org/changeset/base/359918
Log:
posixshm: fix counting of writable mappings
Similar to mmap'ing vnodes, posixshm should count any mapping where maxprot
contains VM_PROT_WRITE (i.e. fd opened r/w with no write-seal applied) as
writable and thus blocking of any write-seal.
The memfd tests have been amended to reflect the fixes here, which notably
includes:
1. Fix for error return bug; EPERM is not a documented failure mode for mmap
2. Fix rejection of write-seal with active mappings that can be upgraded via
mprotect(2).
Reported by: markj
Discussed with: markj, kib
Modified:
head/sys/kern/uipc_shm.c
head/tests/sys/kern/memfd_test.c
Modified: head/sys/kern/uipc_shm.c
==============================================================================
--- head/sys/kern/uipc_shm.c Tue Apr 14 13:12:22 2020 (r359917)
+++ head/sys/kern/uipc_shm.c Tue Apr 14 13:32:03 2020 (r359918)
@@ -1147,14 +1147,16 @@ shm_mmap(struct file *fp, vm_map_t map, vm_offset_t *a
if ((fp->f_flag & FWRITE) != 0 &&
(shmfd->shm_seals & F_SEAL_WRITE) == 0)
maxprot |= VM_PROT_WRITE;
- writecnt = (prot & VM_PROT_WRITE) != 0;
- if (writecnt && (shmfd->shm_seals & F_SEAL_WRITE) != 0) {
- error = EPERM;
- goto out;
- }
- /* Don't permit shared writable mappings on read-only
descriptors. */
- if (writecnt && (maxprot & VM_PROT_WRITE) == 0) {
+ /*
+ * Any mappings from a writable descriptor may be upgraded to
+ * VM_PROT_WRITE with mprotect(2), unless a write-seal was
+ * applied between the open and subsequent mmap(2). We want to
+ * reject application of a write seal as long as any such
+ * mapping exists so that the seal cannot be trivially bypassed.
+ */
+ writecnt = (maxprot & VM_PROT_WRITE) != 0;
+ if (!writecnt && (prot & VM_PROT_WRITE) != 0) {
error = EACCES;
goto out;
}
Modified: head/tests/sys/kern/memfd_test.c
==============================================================================
--- head/tests/sys/kern/memfd_test.c Tue Apr 14 13:12:22 2020
(r359917)
+++ head/tests/sys/kern/memfd_test.c Tue Apr 14 13:32:03 2020
(r359918)
@@ -108,7 +108,7 @@ ATF_TC_BODY(write_seal, tc)
ATF_REQUIRE(mmap(0, BUF_SIZE, (PROT_READ | PROT_WRITE), MAP_SHARED,
fd, 0) == MAP_FAILED);
- ATF_REQUIRE(errno == EPERM);
+ ATF_REQUIRE(errno == EACCES);
close(fd);
}
@@ -136,13 +136,23 @@ ATF_TC_BODY(mmap_write_seal, tc)
ATF_REQUIRE(munmap(addr, BUF_SIZE) == 0);
+ /*
+ * This should fail, because raddr still exists and it was spawned from
+ * a r/w fd.
+ */
+ ATF_REQUIRE(fcntl(fd, F_ADD_SEALS, F_SEAL_WRITE) == -1);
+ ATF_REQUIRE(errno == EBUSY);
+
+ ATF_REQUIRE(munmap(raddr, BUF_SIZE) == 0);
+ /* This one should succeed; only the private mapping remains. */
ATF_REQUIRE(fcntl(fd, F_ADD_SEALS, F_SEAL_WRITE) == 0);
ATF_REQUIRE(munmap(paddr, BUF_SIZE) == 0);
- ATF_REQUIRE(munmap(raddr, BUF_SIZE) == 0);
ATF_REQUIRE(mmap(0, BUF_SIZE, (PROT_READ | PROT_WRITE), MAP_SHARED,
fd, 0) == MAP_FAILED);
- ATF_REQUIRE(errno == EPERM);
+ ATF_REQUIRE(errno == EACCES);
+
+ /* Make sure we can still map privately r/w or shared r/o. */
paddr = mmap(0, BUF_SIZE, (PROT_READ | PROT_WRITE), MAP_PRIVATE, fd, 0);
ATF_REQUIRE(paddr != MAP_FAILED);
raddr = mmap(0, BUF_SIZE, PROT_READ, MAP_SHARED, fd, 0);
@@ -227,7 +237,7 @@ ATF_TC_BODY(dup_seals, tc)
ATF_REQUIRE(mmap(0, BUF_SIZE, (PROT_READ | PROT_WRITE), MAP_SHARED,
fdx, 0) == MAP_FAILED);
- ATF_REQUIRE(errno == EPERM);
+ ATF_REQUIRE(errno == EACCES);
close(fd);
close(fdx);
@@ -259,7 +269,6 @@ ATF_TC_BODY(immutable_seals, tc)
"Added duplicate grow seal after restricting seals");
close(fd);
}
-
ATF_TP_ADD_TCS(tp)
{
_______________________________________________
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
