A few quick mmap questions

[Corinna Vinschen]

On Dec 20 10:12, Ken Brown wrote:
> On 12/20/2024 8:13 AM, Corinna Vinschen wrote:
> > 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.

No worries. The expression

  size_t commit_len = u_len - (start_addr - u_addr);

may be right, but it's not generally intelligible.  If you'd like
to take a stab of changing the code to be more clear, I'm happy.


Corinna