Atomic mmap replacement

[Ken Brown]

On 12/16/2024 2:58 PM, Ken Brown via Cygwin wrote:
> On 12/16/2024 8:32 AM, Corinna Vinschen via Cygwin wrote:
>> Right now, mmaping with PROT_NONE and then re-mmaping with PROT_WRITE
>> doesn't work.  Cygwin implements PROT_NONE not as MAP_RESERVE, but as
>> MEM_COMMIT with PAGE_NOACCESS.  mmap() doesn't check if the requested
>> pages are already allocated with PAGE_NOACCESS and then succeeds while
>> in fact just changing the page protection.  This is basically what
>> you want.  Right now, you'd have to call mprotect() instead.
>>
>> With anonymous mappings only, this is all just adding a bit of code to
>> mmap() to do what mprotect() does in this case.
> 
> [...]
> 
>> So only anonymous mappings would be possible, assuming we tweak mmap()
>> to check if the old mapping was anonymous either and then allow to
>> just change the page protection, as if mprotect has been called.
>>
>> And, funny enough, something pretty similar already exists in mmap().
>> See mmap.cc, line 1051 and the mmap_list::try_map() method.  Right
>> now it only checks if an anonymous mapping has been partially unmapped
>> and can be recycled.  But it could be improved by allowing to recycle
>> the anonymous mapping either way, as long as the new mapping is also
>> anonymous and the SHARED/PRIVATE flags match.
> Thanks!  This looks doable.  I hope to get back to you with a patch in 
> the not-too-distant future.
While looking at this, I came across a bug in the use of 
mmap_list::try_map(): On success, the recycled pages are given the same 
protection they had as part of the original mmap_record instead of the 
protection requested in the mmap call.  This is done in 
mmap_record::map_pages.  Here's a test case:

$ cat test.c
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>

const size_t page_size = 64 * 1024;

#define BUF_SIZE 10

int
main ()
{
    void *mem = mmap (NULL, 2 * page_size, PROT_NONE,
                      MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
    if (mem == MAP_FAILED)
      {
        perror ("mmap");
        exit (1);
      }

    if (munmap (mem, BUF_SIZE) == -1)
      {
        perror ("munmap");
        exit (2);
      }

    void *res = mmap (mem, BUF_SIZE, PROT_READ | PROT_WRITE,
                      MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
    if (res == MAP_FAILED)
      {
        perror ("mmap");
        exit (3);
      }

    if (printf ("Current string = %s\n", (char *) res) < 0)
      {
        perror ("printf");
        exit (4);
      }
    strncpy (res, "Hello", BUF_SIZE - 1);
    if (printf ("New string = %s\n", (char *) res) < 0)
      {
        perror ("printf");
        exit (5);
      }
}

$ gcc -o test test.c

$ ./test

$ echo $?
0

So it seems that reading and writing to the recycled pages fails 
silently.  On Linux, we get the expected output:

Current string =
New string = Hello

Would you like a separate patch to fix this, or should I just fix it as 
part of the bigger project?

Ken