On 1/20/2025 6:49 AM, Corinna Vinschen wrote:
Nice idea, but this may not do what is expected if the mapping is an
anonymous mapping, leaving the protection or mapping of trailing pages
in a wrong state, isn't it?
Can we easily make sure the type of mapping (file vs anon) is known
at the time of rounding, so the rounding is performed differently?
I hadn't thought of that. Actually, I *think* the record length is
already a multiple of 64K for an anonymous mapping. But to play it safe
we could condition the rounding on whether or not fd == -1, like this:
--- a/winsup/cygwin/mm/mmap.cc
+++ b/winsup/cygwin/mm/mmap.cc
@@ -409,16 +409,32 @@ 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 for a file mapping or
+ to a Windows allocation granularity boundary for an anonymous
+ mapping. 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 for file
+ mappings 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 pagesize = get_fd () == -1 ? wincap.allocation_granularity ()
+ : wincap.page_size ();
+ SIZE_T rec_len = roundup2 (get_len (), pagesize);
caddr_t low = MAX (addr, get_address ());
caddr_t high = MIN (addr + len, get_address () + rec_len);
if (low < high)
Ken
