[PATCH] Cygwin: mmap: use 64K pages for bookkeeping

[Ken Brown]

On 1/15/2025 12:31 PM, Corinna Vinschen wrote:
>> Ouch.  It looks like we can't go to 64K bookkeeping.  Windows files are
>> not length-aligned to 64K allocation granularity, but to 4K pagesize.
>> Thus, if we align the length to 64K in mprotect or
>> mmap_record::unmap_pages, it tries to access the unallocatd area from
>> the EOF page to the last page in the 64K area, which, obviously fails.
> 
> Alternatively it has to be faked in the affected functions, which then
> stealthily only access the pages up to EOF under the hood...
It's possible that the following simple patch (on top of the previous 
patch) solves the problem:

--- a/winsup/cygwin/mm/mmap.cc
+++ b/winsup/cygwin/mm/mmap.cc
@@ -409,16 +409,28 @@ mmap_record::find_unused_pages (SIZE_T pages) const

  /* Return true if the interval I from addr to addr + len intersects
     the interval J of this mmap_record.  The endpoint of the latter is
-   first rounded up to a page boundary.  If there is an intersection,
-   then it is the interval from m_addr to m_addr + m_len.  The
-   variable 'contains' is set to true if J contains I.
+   first rounded up to a Windows page boundary.  If there is an
+   intersection, then it is the interval from m_addr to
+   m_addr + m_len.  The variable 'contains' is set to true if J contains I.
+
+   It is necessary to use a 4K Windows page boundary above because
+   Windows files are length-aligned to 4K pages, not to the 64K
+   allocation granularity.  If we were to align the record length to
+   64K, then callers of this function might try to access the
+   unallocated memory from the EOF page to the last page in the 64K
+   area.  See
+
+     https://cygwin.com/pipermail/cygwin-patches/2025q1/013240.html
+
+   for an example in which mprotect and mmap_record::unmap_pages both
+   fail when we align the record length to 64K.
  */
  bool
  mmap_record::match (caddr_t addr, SIZE_T len, caddr_t &m_addr, SIZE_T 
&m_len,
                     bool &contains)
  {
    contains = false;
-  SIZE_T rec_len = PAGE_CNT (get_len ()) * 
wincap.allocation_granularity ();
+  SIZE_T rec_len = roundup2 (get_len (), wincap.page_size ());
    caddr_t low = MAX (addr, get_address ());
    caddr_t high = MIN (addr + len, get_address () + rec_len);
    if (low < high)

I've checked that gdb functions normally after this patch, but I can't 
claim to have thought through all possible situations where an 
mmap-related function might fail as a result of switching to 64K 
bookkeeping.

Ken