Re: [RFC] Implementing RLIMIT_AS

[Diego Nieto Cid]

Hi,

On Sun, Dec 22, 2024 at 02:35:08AM +0100, Samuel Thibault wrote:
> 
> What do you refer to by hard/soft?
>

I just didn't understand the hard/soft limits. It's better described
by the structure members and not the comments:

    struct rlimit {
        rlim_t rlim_cur;  /* Soft limit */
        rlim_t rlim_max;  /* Hard limit (ceiling for rlim_cur) */
    };

So `rlim_cur` is the limit that must be enforced and `rlim_max` is
the maximum value an unprivileged process can set its `rlim_cur` to.

> 
> > So, for now, it's a plain rejection with ENOMEM.
> 
> Yes, that's what we want in the end.
>

Ok

> 
> >   2. At vm_map_setup, initialize the `hard_limit` field with the
> >      appropriate value which should be RLIM_INFINITY.
> 
> As mentioned on the contributing page, we can use as rule of thumb the
> default of linux: half the physical memory + size of swap.

Ok

> 
> >   3. Finally, enforce the limit in `vm_allocate`, `vm_map` and
> >      `vm_allocate_contiguous` by checking that current map size
> >      (`size` field) plus the requested size (`size` param) is less
> >      than the current map's `hard_limit` field.
> 
> As mentioned in the thread, the check should *really* rather be made
> inside vm_map/pmap functions, so they are shared by whatever happens to
> allocate adressing space. As mentioned, looking for what updates the
> size field would probably be a good fit.
> 

Yes, I started applying the check to the lower-level API, and I'm trying
to get a grasp of how the vm module works.

Currently, I have issues understanding how vm_map_copy_t and how they
affect the total memory of the process.

> > I thought of adding an RPC call that sets the `hard_limit` field
> > which, I guess, should be located among the other task related RPCs.
> 
> Yes, with the host port being an optional parameter for the case when
> the limit is getting requested to be increased.
> 

Great.


> 
> > One big point to address is how to enforce the ability to change this limit,
> > e.g. an unprivileged task shouldn't be able to increase its own memory
> > limit. You could reuse the host privileged port, but maybe it could make
> > sense to have a dedicated one for resource limits?
> 
> I'd say using the host port will be fine for now.
>

Ok

> Diego Nieto Cid, le jeu. 19 déc. 2024 22:54:23 -0300, a ecrit:
> > Also, I cannot make it to fail with the attached test program.
> 
> Note that malloc() uses mmap() for big allocations, thus escaping
> RLIMIT_DATA, as it should. You'd need a lot of small mallocs() to
> actually make the heap grow and reach RLIMIT_DATA.
>

Noted,

> Diego Nieto Cid, le jeu. 19 déc. 2024 19:54:31 -0300, a ecrit:
> > > > 
> > > >       I tried a lower value, like 2GB, but some process is mapping
> > > >       4GB at once during boot and it just hangs when the allocation
> > > >       fails.
> > > 
> > > Which process is that?
> > 
> > Its task name is `exec` and it's using vm_map. The log in question is:
> > 
> >     [vm_map] [task exec] map size: 0, requested size: 4294967296, hard 
> > limit: 2147483648
> 
> It'd be useful to get a backtrace. You can make your grub use
> /hurd/exec.static instead of /hurd/exec, and use kdb's trace/u command
> to get the userland backtrace easily. You could also add mach_print()s
> in exec.c.

Ah, the /u prefix. I was wondering why I couldn't go back to userland through
the gnumach debugger.

> 
> Luca, le ven. 20 déc. 2024 10:25:02 +0100, a ecrit:
> > are you working on x86_64? if yes, that could be the redzone configured
> > here:
> > 
> > https://git.savannah.gnu.org/cgit/hurd/hurd.git/tree/exec/exec.c#n1247
> 
> That's indeed a very good candidate.
> 
> One thing is: it's a VM_PROT_NONE/VM_PROT_NONE area. We wouldn't really
> want to make such area account for RLIMIT_AS, as they are not meant to
> store anything.
>

This complicates a bit the accounting. I can keep a count of memory allocated
whit that protection. But I supose I need to check for calls to `vm_protect` or
its underlying implementation.

> > > One additional point would be at least in task_create(), I guess the new
> > > task would have the same restriction as the one creating it.
> > 
> > Yes, indeed. A quick look at kern/task.c shows I should check vm_map_fork:
> > 
> >     } else if (inherit_memory) {
> >         new_task->map = vm_map_fork(parent_task->map);
> 
> Not really: exec does not set inherit_memory to 1, it always re-creates
> a completely new task. What you want is to make task_create always
> inherit from the parent_task, if any.
>

Ok

 
> Even that patch shouldn't be needed nowadays: the support was commited
> upstream, and it's only very old-built netdde/rumpdisk that would need the
> debian patch.
>

A small nuisance I'm getting is that the gnumach.gz I build from source
does not have a proper verison and dpkg firends complain when regenerating
GRUB configuration.

Thanks

--
Diego