check if procfs is the actual host procfs #1417
Conversation
kkourt
added
the
release-note/misc
kkourt
force-pushed
the
pr/kkourt/kind-warning
branch
4 times, most recently
from
415d85e to
a91bc91
Compare
|
Maybe compare /proc/1/ns/pid with https://elixir.bootlin.com/linux/latest/source/include/linux/proc_ns.h#L46 IIRC it is an idr allocation that has been constant for a very long time... and init pid is statically declared anyway |
bpf/process/bpf_gethostpid.c
Outdated
|
|
||
| ev.pid = get_current_pid_tgid() >> 32; | ||
| perf_event_output(ctx, &tg_hpid_event_map, BPF_F_CURRENT_CPU, &ev, sizeof(ev)); | ||
| return 0; |
There was a problem hiding this comment.
seems like too much (using perf rb) for retrieving just one pid value.. could we just update map? or perhaps something like in pkg/sensors/map_update.go maybe the ebpf program could return the pid as return value, but I might be too optimistic ;-)
There was a problem hiding this comment.
I used a perf rb to handle the case where we get >1 than one events. it's very unlikely to happen but it does not seem like a lot of trouble to handle it.
There was a problem hiding this comment.
maybe the ebpf program could return the pid as return value, but I might be too optimistic ;-)
That's what I tried first using BPF_PROG_RUN, but I couldn't make it to work. Specifically, I could not find a program type that would allow me to get the pid. So, instead, I used a system call. Is there a way to use get_current_pid_tgid() from a bpf program that is executed with BPF_PROG_RUN?
kkourt
force-pushed
the
pr/kkourt/kind-warning
branch
from
a91bc91 to
fc09bad
Compare
✅ Deploy Preview for tetragon ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
pkg/bpf/hostpid/hostpid.go
Outdated
| off: 4343, | ||
| whence: 5151, |
There was a problem hiding this comment.
If you were to make these two values completely random, then you'd have a 1 in 2^64 chance of colliding with another lseek call that also set the FD to -1 (highly unlikely). In fact, you could set the FD to a random negative value too (maximum number of open files appears to be much less than max(int) so overflow into negative numbers appears unlikely) and increase this to 1 in 2^95 which is reaching cryptographic collision avoidance territory.
Doing that would mean you wouldn't need the ring buffer and a simple map would suffice.
There was a problem hiding this comment.
I tend to agree with not using the ring buffer and instead using a map to communicate the value to user space. Choosing random values for the magic numbers in the lseek call would avoid collisions with other lseek calls. Suggest this would simplify the code somewhat.
There was a problem hiding this comment.
I believe that what Djalal said in a previous comment makes sense. i.e. to compare /<option.Config.ProcFS>/1/ns/pid with https://elixir.bootlin.com/linux/latest/source/include/linux/proc_ns.h#L46.
$ kind create cluster
[...]
$ docker ps
CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES
799c437d3f63 kindest/node:v1.27.3 "/usr/local/bin/entr…" 43 seconds ago Up 36 seconds 127.0.0.1:41377->6443/tcp kind-control-plane
$ docker exec -it kind-control-plane bash
root@kind-control-plane:/# ls -la /proc/1/ns/pid
lrwxrwxrwx 1 root root 0 Oct 3 09:10 /proc/1/ns/pid -> 'pid:[4026532359]'
root@kind-control-plane:/#
exit
$ sudo ls -la /proc/1/ns/pid
lrwxrwxrwx 1 root root 0 Oct 2 10:17 /proc/1/ns/pid -> 'pid:[4026531836]'
If we run on the host pid namespace the value of /proc/1/ns/pid should always be 4026531836 (0xEFFFFFFC) (or I miss something).
Signed-off-by: Kornilios Kourtis <[email protected]>
Tetragon requires access to host /proc. In certain environments (e.g., kind) this is not achieved by default. This patch adds a warning by comparing the inode number of the pid namespace in the provided /proc with the expected hard-coded value. Signed-off-by: Kornilios Kourtis <[email protected]>
This patch modifies the localdev scripts to mount the actual host /proc in the tetragon container when using kind. This eliminates the warning introduced in the previous commits. To this end, this patch introduces a new helm variable to specify the path of the proc filesystem in the runtime. Signed-off-by: Kornilios Kourtis <[email protected]>
kkourt
force-pushed
the
pr/kkourt/kind-warning
branch
from
fc09bad to
6d7f078
Compare
|
Pushed a new version using @tixxdz's suggestion. PTAL, thanks! |
See patches.