Patch attached. This version is much simpler and more efficient than
the previous one because it doesn't require keeping an mmap_list in a
specific order.
With this patch installed, the two test cases in
https://cygwin.com/pipermail/cygwin-developers/2024-December/012725.html
both succeed.
I have a question about the "Fixes" line. Since the commit in question
was in the old CVS style, it doesn't have a good one-line summary. I
tried to choose the next-best thing, but I'm not sure about it.
Ken
From 6501824000b4e21bec4d932a6d848bd89dd87ca3 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbr...@cornell.edu>
Date: Fri, 27 Dec 2024 10:10:06 -0500
Subject: [PATCH v2] Cygwin: mmap: fix mmap_is_attached_or_noreserve
This commit fixes two problems. The first is that
mmap_is_attached_or_noreserve would sometimes call VirtualAlloc with
MEM_COMMIT on address ranges that were not known to have MEM_RESERVE
status. These calls could fail, causing SIGBUS to be raised
incorrectly. See
https://cygwin.com/pipermail/cygwin-developers/2024-December/012725.html
for details. Fix this by calling VirtualAlloc only on the part of the
address range that's contained in the current mmap_record.
The second problem is that the code would sometimes break out of the
main loop without knowing whether attached mappings still occur later
in the mmap_list. This could cause SIGBUS to not be raised when it
should be. Fix this by using "continue" rather than "break". For
efficiency, introduce a boolean variable "nocover" that's set to true
if we discover that the address range cannot be covered by noreserve
mmap regions.
Addresses:
https://cygwin.com/pipermail/cygwin-developers/2024-December/012725.html
Fixes: 70e476d27be8 ("* include/cygwin/version.h: Bump DLL version to
1.7.0")
Signed-off-by: Ken Brown <kbr...@cornell.edu>
---
winsup/cygwin/mm/mmap.cc | 52 +++++++++++++++++++++++-----------------
1 file changed, 30 insertions(+), 22 deletions(-)
diff --git a/winsup/cygwin/mm/mmap.cc b/winsup/cygwin/mm/mmap.cc
index 13418d782baf..fc126a87072a 100644
--- a/winsup/cygwin/mm/mmap.cc
+++ b/winsup/cygwin/mm/mmap.cc
@@ -738,18 +738,18 @@ is_mmapped_region (caddr_t start_addr, caddr_t
end_address)
/* This function is called from exception_handler when a segmentation
violation has occurred. It should also be called from all Cygwin
- functions that want to support passing noreserve mmap page addresses
- to Windows system calls. In that case, it should be called only after
- a system call indicates that the application buffer passed had an
- invalid virtual address to avoid any performance impact in non-noreserve
- cases.
+ functions that want to support passing noreserve (anonymous) mmap
+ page addresses to Windows system calls. In that case, it should be
+ called only after a system call indicates that the application
+ buffer passed had an invalid virtual address to avoid any
+ performance impact in non-noreserve cases.
Check if the address range is all within noreserve mmap regions. If so,
call VirtualAlloc to commit the pages and return MMAP_NORESERVE_COMMITED
- on success. If the page has __PROT_ATTACH (SUSv3 memory protection
- extension), or if VirtualAlloc fails, return MMAP_RAISE_SIGBUS.
- Otherwise, return MMAP_NONE if the address range is not covered by an
- attached or noreserve map.
+ on success. If some page in the address range has __PROT_ATTACH
+ (SUSv3 memory protection extension), or if VirtualAlloc fails,
+ return MMAP_RAISE_SIGBUS. Otherwise, return MMAP_NONE if the
+ address range is not covered by noreserve maps.
On MAP_NORESERVE_COMMITED, the exeception handler should return 0 to
allow the application to retry the memory access, or the calling Cygwin
@@ -768,12 +768,16 @@ mmap_is_attached_or_noreserve (void *addr, size_t len)
len += ((caddr_t) addr - start_addr);
len = roundup2 (len, pagesize);
- if (map_list == NULL)
- goto out;
-
mmap_record *rec;
caddr_t u_addr;
SIZE_T u_len;
+ /* nocover is set to true if we discover that our address range
+ cannot be covered by noreserve mmap regions. */
+ bool nocover = false;
+ size_t remaining = len;
+
+ if (map_list == NULL)
+ goto out;
LIST_FOREACH (rec, &map_list->recs, mr_next)
{
@@ -784,23 +788,27 @@ mmap_is_attached_or_noreserve (void *addr, size_t len)
ret = MMAP_RAISE_SIGBUS;
break;
}
- if (!rec->noreserve ())
- break;
+ if (nocover)
+ /* We need to continue in case we encounter an attached mmap
+ later in the list. */
+ continue;
- size_t commit_len = u_len - (start_addr - u_addr);
- if (commit_len > len)
- commit_len = len;
+ if (!rec->noreserve ())
+ {
+ nocover = true;
+ continue;
+ }
- if (!VirtualAlloc (start_addr, commit_len, MEM_COMMIT,
- rec->gen_protect ()))
+ /* The interval determined by u_addr and u_len is the part of
+ our address range contained in the mmap region of rec. */
+ if (!VirtualAlloc (u_addr, u_len, MEM_COMMIT, rec->gen_protect ()))
{
ret = MMAP_RAISE_SIGBUS;
break;
}
- start_addr += commit_len;
- len -= commit_len;
- if (!len)
+ remaining -= u_len;
+ if (!remaining)
{
ret = MMAP_NORESERVE_COMMITED;
break;
--
2.45.1
