Patch attached.
I'm not sure I handled the noreserve case in the best possible way, but
at least I didn't make it worse. The behavior in that case after my
patch is the same as before.
Ken
P.S. If no one has any comments in the next week or so, I might just go
ahead and push it (to main only), so it can get some testing. Corinna
has already said that's OK in
https://cygwin.com/pipermail/cygwin-developers/2024-December/012723.htmlFrom 22f124455d16d2af59ee156db558550686fa8770 Mon Sep 17 00:00:00 2001
From: Ken Brown <kbr...@cornell.edu>
Date: Fri, 27 Dec 2024 16:09:40 -0500
Subject: [PATCH] Cygwin: mmap: allow remapping part of an existing anonymous
mapping
Previously mmap would fail with EINVAL on an attempt to map an address
range contained in the chunk of an existing mapping. With this
commit, mmap will succeed, provided the mappings are anonymous, the
MAP_SHARED/MAP_PRIVATE flags agree, and MAP_NORESERVE is not set for
either mapping.
Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256901.html
Signed-off-by: Ken Brown <kbr...@cornell.edu>
---
winsup/cygwin/mm/mmap.cc | 45 ++++++++++++++++++++++---------------
winsup/cygwin/release/3.6.0 | 5 +++++
2 files changed, 32 insertions(+), 18 deletions(-)
diff --git a/winsup/cygwin/mm/mmap.cc b/winsup/cygwin/mm/mmap.cc
index fc126a87072a..0224779458ef 100644
--- a/winsup/cygwin/mm/mmap.cc
+++ b/winsup/cygwin/mm/mmap.cc
@@ -494,18 +494,24 @@ mmap_record::map_pages (caddr_t addr, SIZE_T len, int
new_prot)
off_t off = addr - get_address ();
off /= wincap.page_size ();
len = PAGE_CNT (len);
- /* First check if the area is unused right now. */
- for (SIZE_T l = 0; l < len; ++l)
- if (MAP_ISSET (off + l))
- {
- set_errno (EINVAL);
- return false;
- }
- if (!noreserve ()
- && !VirtualProtect (get_address () + off * wincap.page_size (),
- len * wincap.page_size (),
- ::gen_protect (new_prot, get_flags ()),
- &old_prot))
+ /* VirtualProtect can only be called on committed pages, so it's not
+ clear how to change protection in the noreserve case. In this
+ case we will therefore require that the pages are unmapped, in
+ order to keep the behavior the same as it was before new_prot was
+ introduced. FIXME: Is there a better way to handle this? */
+ if (noreserve ())
+ {
+ for (SIZE_T l = 0; l < len; ++l)
+ if (MAP_ISSET (off + l))
+ {
+ set_errno (EINVAL);
+ return false;
+ }
+ }
+ else if (!VirtualProtect (get_address () + off * wincap.page_size (),
+ len * wincap.page_size (),
+ ::gen_protect (new_prot, get_flags ()),
+ &old_prot))
{
__seterrno ();
return false;
@@ -625,8 +631,9 @@ mmap_list::try_map (void *addr, size_t len, int new_prot,
int flags, off_t off)
if (off == 0 && !fixed (flags))
{
- /* If MAP_FIXED isn't given, check if this mapping matches into the
- chunk of another already performed mapping. */
+ /* If MAP_FIXED isn't given, try to satisfy this mapping request
+ by recycling unmapped pages in the chunk of an existing
+ mapping. */
SIZE_T plen = PAGE_CNT (len);
LIST_FOREACH (rec, &recs, mr_next)
if (rec->find_unused_pages (plen) != (SIZE_T) -1)
@@ -640,9 +647,10 @@ mmap_list::try_map (void *addr, size_t len, int new_prot,
int flags, off_t off)
}
else if (fixed (flags))
{
- /* If MAP_FIXED is given, test if the requested area is in an
- unmapped part of an still active mapping. This can happen
- if a memory region is unmapped and remapped with MAP_FIXED. */
+ /* If MAP_FIXED is given, test if the requested area is
+ contained in the chunk of an existing mapping. If so, and if
+ the flags of that mapping are compatible with those in the
+ request, try to reset the protection on the requested area. */
caddr_t u_addr;
SIZE_T u_len;
@@ -1061,7 +1069,8 @@ go_ahead:
LIST_WRITE_LOCK ();
map_list = mmapped_areas.get_list_by_fd (fd, &st);
- /* Test if an existing anonymous mapping can be recycled. */
+ /* Try to satisfy the request by resetting the protection on part of
+ an existing anonymous mapping. */
if (map_list && anonymous (flags))
{
caddr_t tried = map_list->try_map (addr, len, prot, flags, off);
diff --git a/winsup/cygwin/release/3.6.0 b/winsup/cygwin/release/3.6.0
index 4b7604907902..92237207032e 100644
--- a/winsup/cygwin/release/3.6.0
+++ b/winsup/cygwin/release/3.6.0
@@ -78,3 +78,8 @@ What changed:
- Raise maximum pid from 65536 to 4194304 to account for scenarios
with lots of CPUs and lots of tasks.
Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256927.html
+
+- Allow mmap to succeed on an address range contained in the chunk of
+ an existing anonymous mapping, provided the MAP_SHARED/MAP_PRIVATE
+ flags agree and MAP_NORESERVE is not set for either mapping.
+ Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256901.html
--
2.45.1
