Skip to content

v1.0.0-pre.0

v1.0.0-pre.0: bpf: remove cgrpid_tracker
@kkourt kkourt tagged this 13 Oct 17:08 
There is an issue with the cgroup tracker id (details can be found
below). This issue causes policyfilter to misbehave. Furthermore, the
original intention behind the cgroup tracker id was never realized
and so it currently serves no purpose. Its only user is the
policyfilter.

This PR removes the cgroup tracker id, and, instead, computes the cgroup
id at the time of the policyfilter check. For most common setups
(cgroupv2) the overhead of doing so is negligible. Computing the cgroup
id at the time of check is more reliable and also simplifies the code.

More details can be found below.

We have a namespaced policy installed that mathes connect() calls to 8.8.8.8, and execute the
following on a pod in the same namespace:

`kubectl exec -it nonqos-demo -- curl 8.8.8.8`

We observe two things:
 * there is no event from the policy (as it should be)
 * the cgroup tracker id of the curl process is the same as the parent runc process (which should
   not be the case)

The exec event we get from curl has:
 .process.pid: 244958
 .process.binary: /usr/bin/curl
 .parent.pid: 244948
 .parent.binary: /usr/bin/runc

Looking at the cgroup tracker id in the execve map for these two pids we see:
"cgrpid_tracker": 2371
"cgrpid_tracker": 2371

Which means that the cgroup id tracker is not correct, and the curl
process will not be matched by the policy.

The issue might arise from the fact that in the wake_up_new_task(),
computing the cgroup tracker id, uses get_current_cgroup_id(). We are,
however, still in the parent so the get_current_cgroup_id() helper will
return the cgroup of the parent, not of the child (which is what we
want).

To simplify the code complexity (both from the perspective of
understandabiliuty but also verification complexity), we instead remove
the tracker id and retrieve the current cgroup id at the time of the
check. This is much simpler, and not less efficient for the common case
(cgroupv2).

Following the same steps with a version build with the current patch,
produces an event as expected.

Signed-off-by: Kornilios Kourtis <[email protected]>
Assets 2
Loading