A few quick mmap questions

[Ken Brown]

On 12/20/2024 8:13 AM, Corinna Vinschen wrote:
> On Dec 19 18:36, Ken Brown wrote:
>> I think I'm seeing a similar confusion in
>> mmap_is_attached_or_noreserve().  I'm tired now and am having trouble
>> sorting out exactly what that function is doing.
> 
> noreserve() pages are MAP_RESERVE pages.  Trying to read or write on them
> raises a STATUS_ACCESS_VIOLATION.  The idea is basically to commit the
> requested read/write region (in case of the signal: just 1 byte, which in
> turn requires to commit a full page.
> 
>> But the definition of
>> commit_len looks suspicious to me.  We know from above that start_addr <=
>> u_addr.
> 
> We do?  match() returns the intersection, independently of addr being
> lower or higher than get_address().  Same goes for the length.
> 
> In case of a signal:
> 
>    start_addr is addr rounded down to the page address and len is rounded
>    up to the pagesize.  So we have a single page.  rec->match() either
>    returns false, or if a noreserve() record matches, it will return the
>    single page in u_addr, u_len.
> 
>      commit_len = 4096 - (0x100000000 - 0x100000000) = 4096
> 
> In case we're called via fhandler_base::raw_read():
> 
>    Assuming the region to read is enclosing the noreserve() region...
> 
>    - addr = 0x100000000, len = 256K
>    - rec->get_address() = 0x100010000, len = 64K
> 
>    --> start_addr = addr = 0x100000000
>        len = 256K
> 
>        rec->match() returns
> 
>        u_addr = 0x100010000, u_len = 64K
> 
>        commit_len = 64K - (0x100000000 - 0x100010000)
> 		 = 64K - 0x0xFFFFFFFFFFFF0000
> 		 = 128K
> 
>        Note: commit_len is size_t, so it's unsigned!
> 
>        if (commit_len (128K) > len (256K))?  Nope!
> 
>    --> VirtualAlloc (0x100000000, 128K, MEM_COMMIT, rec->gen_protect ());
> 
>    If this VirtualAlloc fails, the process is screwed and gets a
>    well-deserved SIGBUS.
> 
> I'm not saying that there's a chance I'm missing something, but
> as of now, it looks ok to me.
> 
>> The only thing I can come up with immediately is that we should sort the list
>> of mmap_records in order of their starting addresses.  Then if start_addr ==
>> u_addr, we commit u_len bytes, otherwise we fail.
> 
> I don't understand this one.  The code loops over all records,
> so where's the problem?
I think I just missed several crucial things, such as commit_len being 
unsigned.  I also didn't pay attention to the conditions under which the 
function is called.  I have to study the code further, but I'm sure 
you're right.  Thanks for the explanation, and sorry for the noise.

Ken