The two attached patches fix the bugs reported in https://cygwin.com/pipermail/cygwin/2024-December/256911.html
and https://cygwin.com/pipermail/cygwin/2024-December/256913.htmlThe second one is still under discussion on the cygwin list and may turn out to be wrong.
Ken
From a8ce46ae4353988827a2d646b63d14a6abb8bc90 Mon Sep 17 00:00:00 2001 From: Ken Brown <kbr...@cornell.edu> Date: Wed, 18 Dec 2024 11:39:31 -0500 Subject: [PATCH 1/2] Cygwin: mmap: fix protection when unused pages are recycled Previously, when unused pages from an mmap_record were recycled, they were given the protection of the mmap_record rather than the protection requested in the mmap call. Fix this by adding a "prot" parameter to mmap_list::try_map and mmap_record::map_pages to keep track of the requested protection. Then use prot in the call to VirtualProtect. Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256911.html Fixes: ?? Signed-off-by: Ken Brown <kbr...@cornell.edu> --- winsup/cygwin/mm/mmap.cc | 28 +++++++++++++++------------- winsup/cygwin/release/3.5.5 | 3 +++ 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/winsup/cygwin/mm/mmap.cc b/winsup/cygwin/mm/mmap.cc index 332c015a72d9..a2041ce63d45 100644 --- a/winsup/cygwin/mm/mmap.cc +++ b/winsup/cygwin/mm/mmap.cc @@ -339,8 +339,8 @@ class mmap_record SIZE_T find_unused_pages (SIZE_T pages) const; bool match (caddr_t addr, SIZE_T len, caddr_t &m_addr, SIZE_T &m_len); - off_t map_pages (SIZE_T len); - bool map_pages (caddr_t addr, SIZE_T len); + off_t map_pages (SIZE_T len, int prot); + bool map_pages (caddr_t addr, SIZE_T len, int prot); bool unmap_pages (caddr_t addr, SIZE_T len); int access (caddr_t address); @@ -373,7 +373,7 @@ class mmap_list void set (int nfd, struct stat *st); mmap_record *add_record (mmap_record &r); bool del_record (mmap_record *rec); - caddr_t try_map (void *addr, size_t len, int flags, off_t off); + caddr_t try_map (void *addr, size_t len, int prot, int flags, off_t off); }; class mmap_areas @@ -455,14 +455,14 @@ mmap_record::init_page_map (mmap_record &r) } off_t -mmap_record::map_pages (SIZE_T len) +mmap_record::map_pages (SIZE_T len, int prot) { /* Used ONLY if this mapping matches into the chunk of another already performed mapping in a special case of MAP_ANON|MAP_PRIVATE. Otherwise it's job is now done by init_page_map(). */ DWORD old_prot; - debug_printf ("map_pages (fd=%d, len=%lu)", get_fd (), len); + debug_printf ("map_pages (fd=%d, len=%lu, prot=%y)", get_fd (), len, prot); len = PAGE_CNT (len); off_t off = find_unused_pages (len); @@ -470,7 +470,8 @@ mmap_record::map_pages (SIZE_T len) return (off_t) 0; if (!noreserve () && !VirtualProtect (get_address () + off * wincap.page_size (), - len * wincap.page_size (), gen_protect (), + len * wincap.page_size (), + ::gen_protect (prot, get_flags ()), &old_prot)) { __seterrno (); @@ -483,9 +484,9 @@ mmap_record::map_pages (SIZE_T len) } bool -mmap_record::map_pages (caddr_t addr, SIZE_T len) +mmap_record::map_pages (caddr_t addr, SIZE_T len, int prot) { - debug_printf ("map_pages (addr=%p, len=%lu)", addr, len); + debug_printf ("map_pages (addr=%p, len=%lu, prot=%y)", addr, len, prot); DWORD old_prot; off_t off = addr - get_address (); off /= wincap.page_size (); @@ -499,7 +500,8 @@ mmap_record::map_pages (caddr_t addr, SIZE_T len) } if (!noreserve () && !VirtualProtect (get_address () + off * wincap.page_size (), - len * wincap.page_size (), gen_protect (), + len * wincap.page_size (), + ::gen_protect (prot, get_flags ()), &old_prot)) { __seterrno (); @@ -614,7 +616,7 @@ mmap_list::del_record (mmap_record *rec) } caddr_t -mmap_list::try_map (void *addr, size_t len, int flags, off_t off) +mmap_list::try_map (void *addr, size_t len, int prot, int flags, off_t off) { mmap_record *rec; @@ -628,7 +630,7 @@ mmap_list::try_map (void *addr, size_t len, int flags, off_t off) break; if (rec && rec->compatible_flags (flags)) { - if ((off = rec->map_pages (len)) == (off_t) -1) + if ((off = rec->map_pages (len, prot)) == (off_t) -1) return (caddr_t) MAP_FAILED; return (caddr_t) rec->get_address () + off; } @@ -655,7 +657,7 @@ mmap_list::try_map (void *addr, size_t len, int flags, off_t off) set_errno (EINVAL); return (caddr_t) MAP_FAILED; } - if (!rec->map_pages ((caddr_t) addr, len)) + if (!rec->map_pages ((caddr_t) addr, len, prot)) return (caddr_t) MAP_FAILED; return (caddr_t) addr; } @@ -1051,7 +1053,7 @@ go_ahead: /* Test if an existing anonymous mapping can be recycled. */ if (map_list && anonymous (flags)) { - caddr_t tried = map_list->try_map (addr, len, flags, off); + caddr_t tried = map_list->try_map (addr, len, prot, flags, off); /* try_map returns NULL if no map matched, otherwise it returns a valid address, or MAP_FAILED in case of a fatal error. */ if (tried) diff --git a/winsup/cygwin/release/3.5.5 b/winsup/cygwin/release/3.5.5 index e99739241c18..eb97c3d4aafe 100644 --- a/winsup/cygwin/release/3.5.5 +++ b/winsup/cygwin/release/3.5.5 @@ -61,3 +61,6 @@ Fixes: - Fix several problems triggered when a lot of SIGSTOP/SIGCONT signals are received rapidly. Addresses: https://cygwin.com/pipermail/cygwin/2024-November/256744.html + +- Fix the protection when mmap(2) recycles unused pages. + Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256911.html -- 2.45.1
From c09bc8669696aeb6fd75e2e715d7859f94af6ad2 Mon Sep 17 00:00:00 2001 From: Ken Brown <kbr...@cornell.edu> Date: Wed, 18 Dec 2024 11:43:09 -0500 Subject: [PATCH 2/2] Cygwin: mmap_list::try_map: fix a condition in a test of an mmap request In testing whether the requested area is contained in an existing mapped region, an incorrect condition was used due to a misinterpretation of the u_addr and u_len variables. Addresses: https://cygwin.com/pipermail/cygwin/2024-December/256913.html Fixes: ? Signed-off-by: Ken Brown <kbr...@cornell.edu> --- winsup/cygwin/mm/mmap.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/winsup/cygwin/mm/mmap.cc b/winsup/cygwin/mm/mmap.cc index a2041ce63d45..8dd93230e69c 100644 --- a/winsup/cygwin/mm/mmap.cc +++ b/winsup/cygwin/mm/mmap.cc @@ -648,7 +648,7 @@ mmap_list::try_map (void *addr, size_t len, int prot, int flags, off_t off) break; if (rec) { - if (u_addr > (caddr_t) addr || u_addr + len < (caddr_t) addr + len + if (u_addr > (caddr_t) addr || u_len < len || !rec->compatible_flags (flags)) { /* Partial match only, or access mode doesn't match. */ -- 2.45.1
