Re: [RFC PATCH] Implement per task virtual memory limit

[Diego Nieto Cid]

On Mon, Dec 23, 2024 at 10:11:10AM +0300, Sergey Bugaev wrote:
> > +
> > +               vm_map_lock(parent_task->map);
> > +               vm_map_copy_limits(parent_task->map, new_task->map);
> > +               vm_map_unlock(parent_task->map);
> 
> Should this instead be done as a part of vm_map_fork? For the tasks
> that don't inherit memory, do we still want to copy the limit? If we
> do, could you make it a separate code path, so we don't lock the map
> twice in case we do inherit memory.
> 

Probably yes, There are three code paths in task_create_kernel:

        if (child_task == &kernel_task)  {
                new_task->map = kernel_map;                                 /* 
[1] */
        } else if (inherit_memory) {
                new_task->map = vm_map_fork(parent_task->map);              /* 
[2] */
        } else {
                pmap_t new_pmap = pmap_create((vm_size_t) 0);
                if (new_pmap == PMAP_NULL)
                        new_task->map = VM_MAP_NULL;
                else {
                        new_task->map = vm_map_create(new_pmap,
                                        round_page(VM_MIN_USER_ADDRESS),
                                        trunc_page(VM_MAX_USER_ADDRESS));
                        if (new_task->map == VM_MAP_NULL)                   /* 
[3] */
                                pmap_destroy(new_pmap);
                }
        }

At [1] we don't really care about the limit. The path labeled [2] could copy the
limit parameters in vm_map_fork as the map is locked there.

There only remains the case of non-inherited memory that may copy the limit 
from the
parent task in the else branch of the conditional at [3].

In that way the parent task map is only locked once.
 
> > +       target_task->map->map_size_cur_limit = current_limit;
> > +       target_task->map->map_size_max_limit = max_limit;
> > +       vm_map_unlock(target_task->map);
> > +
> > +       return KERN_SUCCESS;
> > +}
> 
> What should happen if the new limit is immediately violated by the
> current size of the map?

Good question.

> Should we ignore this fact and let all
> subsequent operations fail, until someone realizes they need to
> deallocate a bunch of memory for anything to work?

This is how setrlimit works on Linux. The call succeds, nothing is
done about the already allocated memory and any further allocations
fail.

It's probably useful to limit child processes as limits are preserved
across exec and fork. But not limit the parent initially.

So a parent process could allocate a big chunk of memory, then lower
the limit and fork then exec.