runc delete -f: fix for cg v1 + paused container #3217
Conversation
runc delete -f is not working for a paused container, since in cgroup v1 SIGKILL does nothing if a process is frozen (unlike cgroup v2, in which you can kill a frozen process with a fatal signal). Theoretically, we only need this for v1, but doing it for v2 as well is OK. Signed-off-by: Kir Kolyshkin <[email protected]>
| if s, ok := s.(unix.Signal); ok && s == unix.SIGKILL { | ||
| _ = c.cgroupManager.Freeze(configs.Thawed) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
| return ErrNotRunning |
There was a problem hiding this comment.
Haven't tried, but what happens currently if -f is used and it's not running? (thinking: -f should ignore the case and just proceed?) I see we have a special case for all
There was a problem hiding this comment.
The logic for not allowing kill even if you force it is that on paper you could trick runc to kill an arbitrary process. I think an actual attack that uses this is a bit theoretical but shrug. You can't signal a process that doesn't exist.
| // For cgroup v1, killing a process in a frozen cgroup | ||
| // does nothing until it's thawed. Only thaw the cgroup | ||
| // for SIGKILL. | ||
| if s, ok := s.(unix.Signal); ok && s == unix.SIGKILL { |
There was a problem hiding this comment.
Would've been nice to check if the cgroup manager is v1 or v2 here, but still LGTM.
There was a problem hiding this comment.
We are going to use cgroup.kill for v2 and hybrid hierarchies, this code is just to fix the issue
There was a problem hiding this comment.
Right, I get that -- my point is more that it would've been nice if we didn't thaw the cgroup unless we really had to.
|
Fixes: #3134 |
runc delete -f is not working for a paused container, since in cgroup v1
SIGKILL does nothing if a process is frozen (unlike cgroup v2, in which
you can kill a frozen process with a fatal signal).
Theoretically, we only need this for v1, but doing it for v2 as well is
OK.
Proposed changelog entry
Fixes: #3134