cg2 version fails if memory.swap.max does not exist

ferk6a commented 11 months ago

Hello! I've successfully deployed isolate using Docker (systemd on the host running the service, /sys/fs/cgroup/isolate.slice/isolate.service/ binded as a volume), and just ran into a minor issue:

On my deployment environment, memory.swap.max is not available at all in cgroups (and of course, there is no swap), so I was forced to comment out the following line:

https://github.com/ioi/isolate/blob/4defd1d60427aba719941df285e42a5383f73712/cg.c#L238

After doing this, everything worked smoothly. Could we check that it exists before trying to set it?

bajcmartinez commented 10 months ago

hi @fer22f , were you able to fix this? can you provide more information on how you did the whole set up? I'm running into some issues trying to get it to work with docker.

Thanks

ferk6a commented 10 months ago

hi @fer22f , were you able to fix this?

Yes, I fixed it removing that line of code (but I think the code should just check to see if the file exists before trying to set it). This issue is not related to Docker, but rather, my machine doesn't have swap.

can you provide more information on how you did the whole set up? I'm running into some issues trying to get it to work with docker.

My Docker setup is as follows:

Installed isolate into the host and enabled the systemd service. That means that the file /sys/fs/cgroup/isolate.slice is available at the host.
In my docker-compose of my service I do:
```
runner:
  volumes:
    - type: bind
      source: /sys/fs/cgroup/isolate.slice
      target: /sys/fs/cgroup/isolate.slice
  privileged: true
```
That means that isolate running inside this container can use the slice provided by the daemon (I really don't want to run systemd inside Docker).
All that was left was to configure isolate to use the slice, so I just configured with isolate.cf as follows:
```
cg_root = /sys/fs/cgroup/isolate.slice/isolate.service
```
This configuration file is copied to /usr/local/etc/isolate of the container.

bajcmartinez commented 10 months ago

Thanks, that's fantastic. I was also trying to avoid running systemd in the container at all costs, didn't think about binding the service like that. And it should still be safe, I don't think there's much harm that can be done, I need to investigate this further though, I'm using isolate to run untrusted user code, so security is kinda critical, the host machine will be running everyone's code.

Thanks

gollux commented 10 months ago

Binding the subtree from outside the container is possible, but it's really not the recommended way and I don't promise it won't break in future versions of Isolate.

ferk6a commented 10 months ago

Binding the subtree from outside the container is possible, but it's really not the recommended way and I don't promise it won't break in future versions of Isolate.

What is the recommended way?

AlexVasiluta commented 10 months ago

https://github.com/ioi/isolate/blob/4defd1d60427aba719941df285e42a5383f73712/cg.c#L238

After doing this, everything worked smoothly. Could we check that it exists before trying to set it?

I think a adding a question mark at the beginning, like this: "?memory.swap.max", would suffice for both cases, right? You can create a pull request if you want, I haven't been able to reproduce memory.swap.max not existing.

ferk6a commented 10 months ago

https://github.com/ioi/isolate/blob/4defd1d60427aba719941df285e42a5383f73712/cg.c#L238

After doing this, everything worked smoothly. Could we check that it exists before trying to set it?

I think a adding a question mark at the beginning, like this: "?memory.swap.max", would suffice for both cases, right? You can create a pull request if you want, I haven't been able to reproduce memory.swap.max not existing.

That does work! I will submit a PR shortly, then.

For the record, here is the tree that is available to me:

root@machine:~# ls -1 /sys/fs/cgroup/isolate.slice/isolate.service
box-0
cgroup.controllers
cgroup.events
cgroup.freeze
cgroup.kill
cgroup.max.depth
cgroup.max.descendants
cgroup.procs
cgroup.stat
cgroup.subtree_control
cgroup.threads
cgroup.type
cpu.idle
cpu.max
cpu.max.burst
cpuset.cpus
cpuset.cpus.effective
cpuset.cpus.partition
cpuset.mems
cpuset.mems.effective
cpu.stat
cpu.weight
cpu.weight.nice
daemon
io.max
io.stat
memory.current
memory.events
memory.events.local
memory.high
memory.low
memory.max
memory.min
memory.numa_stat
memory.oom.group
memory.peak
memory.reclaim
memory.stat
pids.current
pids.events
pids.max
pids.peak

ioi / isolate

cg2 version fails if memory.swap.max does not exist #137