Re: [RFC] Implementing RLIMIT_AS

[Sergey Bugaev]

On Thu, Dec 19, 2024 at 6:56 PM Diego Nieto Cid <dnietoc@gmail.com> wrote:
> Hello,

Hi,

a few pointers from me:

>      I started with hard limit beacause I still need to research how
>      soft limits work. So, for now, it's a plain rejection with ENOMEM.
>
>   2. At vm_map_setup, initialize the `hard_limit` field with the
>      appropriate value which should be RLIM_INFINITY. In this first
>      attempt, I hardcoded 8GB for testing.
>
>      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.
>
>   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.
>
> 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.

Indeed, this could be something like

routine vm_set_size_limit(target_task : vm_task_t;
                           size_limit : vm_size_t);

but as Luca says, you need to consider who's allowed to increase and
decrease the limit. Yes, having access to host priv port is basically
equivalent to being Unix root (although it takes some steps to gain
actual Hurd UID of 0 once you get access to the host priv port in
gnumach exploits).

> Also, I wanted to ask whether I covered all the allocation points
> or there is somewhere else where limits shall be enforced. For instance,
> something I still have to look at is the out-of-line data sent through
> a mach_msg call.
>
> Regards
>
> ----
>
> Index: gnumach-1.8+git20240714/vm/vm_map.c
> ===================================================================
> --- gnumach-1.8+git20240714.orig/vm/vm_map.c
> +++ gnumach-1.8+git20240714/vm/vm_map.c
> @@ -198,6 +198,9 @@ void vm_map_setup(
>         map->first_free = vm_map_to_entry(map);
>         map->hint = vm_map_to_entry(map);
>         map->name = NULL;
> +       /* TODO hardcoded limit for testing purposes, rather use 
> RLIM_INFINITY */
> +       /* TODO add RPC to update this limit */
> +       map->hard_limit = 8l * 1024l * 1024l * 1024l;

I suppose the default state should be 'unlimited', so either
(vm_size_t) -1, or VM_MAX_USER_ADDRESS - VM_MIN_USER_ADDRESS. Also
make sure to avoid limiting the kernel's own maps.

>         vm_map_lock_init(map);
>         simple_lock_init(&map->ref_lock);
>         simple_lock_init(&map->hint_lock);
> Index: gnumach-1.8+git20240714/vm/vm_map.h
> ===================================================================
> --- gnumach-1.8+git20240714.orig/vm/vm_map.h
> +++ gnumach-1.8+git20240714/vm/vm_map.h
> @@ -198,6 +198,10 @@ struct vm_map {
>         unsigned int            timestamp;      /* Version number */
>
>         const char              *name;          /* Associated name */
> +
> +       /* TODO only hard limits are enforced */
> +       /* TODO does getting rlim_t here make sense? */
> +       vm_size_t               hard_limit;     /* hard limit as set by 
> RLIMIT_AS */

I'd name this max_size, or size_limit perhaps. And maybe don't
reference Unix concepts (rlim_t, RLIM_INFINITY, RLIMIT_AS) this
blatantly :)

>  };
>
>  #define vm_map_to_entry(map)   ((struct vm_map_entry *) &(map)->hdr.links)
> Index: gnumach-1.8+git20240714/vm/vm_user.c
> ===================================================================
> --- gnumach-1.8+git20240714.orig/vm/vm_user.c
> +++ gnumach-1.8+git20240714/vm/vm_user.c
> @@ -81,6 +81,12 @@ kern_return_t vm_allocate(
>                 *addr = trunc_page(*addr);
>         size = round_page(size);
>
> +       if (map->size + size > map->hard_limit)
> +         {
> +               printf("map size: %lu, requested size: %lu, hard limit: 
> %lu\n", map->size, size, map->hard_limit);
> +               return(KERN_NO_SPACE);
> +         }

The checking should happen at vm_map level, rather than vm_user
(vm_user is wrappers around vm_map API  that are exported from Mach
for usage from userland), since there are ways to get VM entries
allocated in a map without calling vm_allocate or vm_map, such as
receiving out-of-line memory in a message. You could look for all the
places where map->size is increased, and add checks for not exceeding
the limit. Of course, do the checks and bail out with KERN_NO_SPACE
before any changes to the map are made, i.e. well before map->size
gets increased. This would also be correct wrt locking the map.

Hope that helps. And now some overall design questions for the feature
(not to discourage you): why? do we actually want this limit? what's
it useful for? isn't address space cheap? is it a sort of advisory
limit, or is it meant to be robust against malicious tasks? "Because
Unix has it" should not be, by itself, considered enough of a reason
to bring something into Mach.

Isn't the limit trivial to work around by spawning a new task (forking
at Unix level)? Even if the new task inherits the parent's limit, you
now have twice as much address space available. Moreover, the Hurd's
exec server will happily give anyone a fresh new task derived from
itself (as opposed to the caller) if you pass oldtask =
MACH_PORT_NULL.

Sergey