Re: [PATCH v2] Set up control groups for most services

[Alyssa Ross <hi@alyssa.is>]

[-- Attachment #1: Type: text/plain, Size: 7740 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> On 6/24/26 22:03, Demi Marie Obenour wrote:
>> On 6/24/26 08:13, Alyssa Ross wrote:
>>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>>
>>>> The cgroups are handled by a Rust tool.  The name of the cgroup is
>>>> autogenerated from the service name.
>>>>
>>>> Using cgroups for process control is not implemented yet.
>>>>
>>>
>>> Perhaps you could include some more details of how the cgroup hierarchy
>>> is supposed to be set up and why, either in the patch body or in
>>> documentation?
>> 
>> Will fix in v3.
>> 
>>> As I understood it, the point of using cgroups was that we could bundle
>>> all services for a VM, including the VMM, into a single cgroup, but I
>>> don't see that here, just a pure translation of the service hierarchy
>>> (which the VMM might not even be part of).  Are per-VM cgroups coming
>>> later?
>> 
>> I think it would be significantly simpler to have the VMM be
>> just another VM service.  This would naturally put it under the
>> vm-services cgroup, solving this problem.  Since this would mostly
>> involve changes to execline scripting, I think it would be better if
>> you wrote this code.
>> 
>>>> diff --git a/host/rootfs/image/etc/fstab b/host/rootfs/image/etc/fstab
>>>> index 18bb5e45abafeaf43871306ce8233239e5c07f76..d92364d83f4d26f766fcd5b494614c06a630d2fe 100644
>>>> --- a/host/rootfs/image/etc/fstab
>>>> +++ b/host/rootfs/image/etc/fstab
>>>> @@ -6,3 +6,4 @@ tmpfs	/dev/shm	tmpfs	nosuid,nodev					0	0
>>>>  tmpfs	/media		tmpfs	nosuid,nodev,noexec,nosymfollow,mode=755	0	0
>>>>  sysfs	/sys		sysfs	nosuid,nodev,noexec				0	0
>>>>  tmpfs	/tmp		tmpfs	nosuid,nodev					0	0
>>>> +cgroup2 /sys/fs/cgroup  cgroup2 nosuid,nodev,noexec,nosymfollow			0	0
>>>
>>> Alignment is off here.  The rest of the file uses tabs at 8 characters.
>>> (I know, I know, but it's traditional.)
>>>
>>>> diff --git a/host/rootfs/image/etc/s6-linux-init/run-image/service/serial-getty-generator/run b/host/rootfs/image/etc/s6-linux-init/run-image/service/serial-getty-generator/run
>>>> index bf66ab0878fc6d8e77ca71a4a89e50c139a6bb11..5c9988c994ed9b942329055673b332d0aa918957 100755
>>>> --- a/host/rootfs/image/etc/s6-linux-init/run-image/service/serial-getty-generator/run
>>>> +++ b/host/rootfs/image/etc/s6-linux-init/run-image/service/serial-getty-generator/run
>>>> @@ -1,7 +1,8 @@
>>>> -#!/bin/execlineb -WP
>>>> +#!/bin/execlineb -WS1
>>>>  # SPDX-License-Identifier: EUPL-1.2+
>>>>  # SPDX-FileCopyrightText: 2024-2025 Alyssa Ross <hi@alyssa.is>
>>>>  
>>>> +cgroup-setup -- $1
>>>>  piperw 3 4
>>>>  background {
>>>>    fdclose 3
>>>
>>> What benefit does this cgroup provide?
>> 
>> None, I'll remove it.
>> 
>> The only requirement of the current code is that if a service is under
>> a cgroup, its parent should *also* be under a cgroup.  Otherwise you
>> can get conflicting cgroup paths.
>> 
>>>> diff --git a/host/rootfs/image/etc/s6-linux-init/run-image/service/serial-getty/run b/host/rootfs/image/etc/s6-linux-init/run-image/service/serial-getty/run
>>>> index 78f794202bf174f3c036f3e20755ac087a988277..ed0808b9e45b077023f237a0f9c0e5c830015ac2 100755
>>>> --- a/host/rootfs/image/etc/s6-linux-init/run-image/service/serial-getty/run
>>>> +++ b/host/rootfs/image/etc/s6-linux-init/run-image/service/serial-getty/run
>>>> @@ -1,5 +1,6 @@
>>>> -#!/bin/execlineb -WP
>>>> +#!/bin/execlineb -WS1
>>>>  # SPDX-License-Identifier: EUPL-1.2+
>>>>  # SPDX-FileCopyrightText: 2023 Alyssa Ross <hi@alyssa.is>
>>>>  
>>>> +cgroup-setup -- $1
>>>>  s6-svscan -d3 instance
>>>
>>> We're putting supervisors of instances services in a cgroup?  Can you
>>> explain to me why that's useful?
>> 
>> This allows configuring limits that apply to the whole instance, and
>> means that the whole instance can be reliably killed.  Services spawned
>> by the instance will be in their own sub-cgroups, so one can apply
>> separate limits to them so long as those limits do not exceed those
>> of the containing cgroup.
>> 
>> The hierarchy works like this:
>> 
>> /sys/fs/cgroup
>>   service1.service
>>     @inner.service # processes go here
>>     service2.service
>>       @inner.service # and here
>>       service3.service
>>         @inner.service # and here
>> 
>> You can apply resource limits at any level of the hierarchy.  Limits in
>> nested cgroups can be smaller but not larger.  Terminating processes
>> in a cgroup with cgroup.kill also terminates all processes in child
>> cgroups.  After terminating the processes in a cgroup, cgroup-setup
>> will delete everything in the hierarchy recursively.
>
> More generally:

(I already replied to the previous email before seeing this, but I'll
try to respond in depth here too.)

> The design I implemented is based on a model where cgroups and services
> are very tightly coupled: most services have an associated cgroup and
> every cgroup is attached to a service.  This is like systemd service
> units, except that it can be nested many layers deep.
>
> Since a VM may have many processes that serve it, this model requires
> that all of these processes be under a single service.  In this case,
> that service would be the vm-services service.
>
> That said, I'm not sure which of these service, if any, should be
> automatically restarted if they crash.  Automatic restart of services
> is actually very useful for attackers, as it lets them retry an exploit
> over and over until they get the memory layout they need for success.

This is exactly why this model doesn't fit.  It does not make sense to
use an s6 service for something that doesn't get restarted.  Oneshot
services are not a concept that exist in s6.  For a clean, repeatable
environment etc. one can use s6-sudod, but even if we had an s6-sudo
service for starting VMMs, they still wouldn't be under a vm-services
instance.  The service↔cgroup equivalence from systemd does not make
sense here.

> If you restart the VMM, you might as well restart every other
> service as well.  Unless Cloud Hypervisor automatically reconnects to
> vhost-user devices (and I don't think it does), there's no point in
> restarting virtiofsd or crosvm without either re-adding the device
> or restarting the VMM.  Even if you did re-add the device, I don't
> know if the guest would be able to recover.

Guests are able to recover from e.g. dbus crashing trivially, and I've
seen virtio-fs working fine after a virtiofsd restart too.  That's why
we run them as services.  It's set up the way it is for a reason!

> Without automatic restart, there's no real reason to use s6 to manage
> the per-VM services.  s6 assumes you do want to restart services
> that crash.

Which we do, generally.

> An alternative approach is to have cgroups be freestanding objects
> that exist independently of whether there are processes in them.
> This corresponds to systemd scope units.  It's not hard to implement
> this: just disable automatic purging and pass the full path to the
> cgroup.  But it requires each VM service to have identical resource
> limits, whereas one typically wants per-process limits.  For instance,
> none of the services should be able to create child processes, and
> per-service cgroups make this easy: just set the maximum amount of
> processes in the cgroup to 1.

Why would you be restricted to identical resource limits.

> Incidentally, this works very well with PID namespaces.  One can have
> the "main" process of the service run in a PID namespace, and have
> all the others enter that namespace.  Then if the main process dies,
> so do all of the others.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]