ioi / isolate

Sandbox for securely executing untrusted programs
Other
1.1k stars 161 forks source link

cg2: `cf_cg_root` points to junk after `clone()` on Fedora #142

Closed AlexVasiluta closed 9 months ago

AlexVasiluta commented 9 months ago

Hello! It seems that the cg2 branch does not work on Fedora. I have tried running multiple kernel versions (6.2, 6.5, 6.6) and multiple Fedora versions (38, 39), and they all seem to corrupt the cf_cg_root variable after the clone() syscall. Ubuntu 23.10 does not seem to have that problem.

I tried doing some small printf debugging and it seems that in the proxy process, cf_cg_root points to the same memory, but it's partially overwritten. Of note is that cf_cg_root is correct in the main isolate process until clone, but also getting overwritten with junk after the clone call (the pointer stays the same, so it's not a stack space issue). That means that the heap is somehow corrupted.

I also have a virtual machine running ubuntu 23.10 (kernel 6.5, but also works after using the mainline 6.6.7 kernel) and isolate behaves properly in that environment, so something is probably wrong with shared memory on Fedora.

For what it's worth, I have the isolate-cg-keeper running and /var/run/isolate/cgroup points to the correct directory. I will update this issue after testing on other distributions, to get the common denominator and hopefully document a fix. UPDATE: Arch Linux works. Rocky Linux 9 works (though is not a good isolate cg2 host since it uses kernel 5.15).

Steps to reproduce:

  1. Set up any Fedora VM (if using kvm, the downloads page has a qcow2 image for easier install);
  2. checkout to cg2 branch, set it up;
  3. isolate --cg --init;
  4. isolate --cg --run /usr/bin/time (or any program);
  5. You will see in stdout: Cannot write <non-ascii chars>/box-0/cgroup.procs: No such file or directory.
AlexVasiluta commented 9 months ago

I made #143 which addresses this issue. It would be ideal if it can be pulled in the main repository soon, since this could also break others' flow.

gollux commented 9 months ago

Fixed by merging PR #143