pkg/uprobetracer: use kfilefields to identify inodes

[i-Pear]

pkg/uprobetracer: use kfilefields to identify inodes

This patch introduced private_inode field to pkg kfilefields,
and it's then used in uprobetracer to get an unique inode-ID for each fd.

Testing

Run many containers sharing the same image, and see if the records are duplicate.
Run many containers not sharing the same image, and see if any record is missing.
Run programs by docker exec, try multi-process applications, and see if any record is missing.

[alban]

Since getInodeUUID() can be called several times for a single gadget, it would make sense to optimize kfilefields.ReadFInodeFromFd to create the kfilefield ebpf program only once. Maybe with:

func ReadFInodesFromsFd(fd []int) ([]uint64, error)

(this could be done in a future PR)

[alban]

I use the following to test it:

package main

import (
	"fmt"
	"os"

	"github.com/inspektor-gadget/inspektor-gadget/pkg/kfilefields"
)

func main() {
	for _, filename := range os.Args[1:] {
		file, err := os.Open(filename)
		if err != nil {
			panic(err)
		}
		pd, err := kfilefields.ReadPrivateDataFromFd(int(file.Fd()))
		if err != nil {
			panic(err)
		}
		pi, err := kfilefields.ReadPrivateInodeFromFd(int(file.Fd()))
		if err != nil {
			panic(err)
		}
		fmt.Printf("%v\n", filename)
		fmt.Printf("PrivateData: %v\nPrivateInode: %v\n", pd, pi)
		runtime.KeepAlive(file)
	}
}

Build it:

go build -o kfilefield ./main.go

Prepare 2 busybox containers:

docker run -ti --rm -v /:/host --name busybox1 busybox
docker run -ti --rm -v /:/host --name busybox2 busybox

Get the pids of the 2 containers:

export BUSYBOX1=$(docker inspect busybox1|jq '.[0].State.Pid')
export BUSYBOX2=$(docker inspect busybox2|jq '.[0].State.Pid')

A couple of tests on /bin/sh:

$ sudo ./kfilefield /proc/$BUSYBOX1/root/bin/sh /proc/$BUSYBOX2/root/bin/sh
/proc/2517303/root/bin/sh
PrivateData: 18446612684266798848
PrivateInode: 18446612699345928680
/proc/2517475/root/bin/sh
PrivateData: 18446612684261101312
PrivateInode: 18446612699345928680
$ sudo ./kfilefield /proc/$BUSYBOX1/root/bin/sh /proc/$BUSYBOX2/root/bin/sh
/proc/2517303/root/bin/sh
PrivateData: 18446612682768175872
PrivateInode: 12884901889
/proc/2517475/root/bin/sh
PrivateData: 18446612683637488896
PrivateInode: 18446612699345928680

The results are not consistent. So I don't think it works fine.

[alban]

(struct file *)->private_data is casted to a struct file * but this is only valid if the filesystem is overlay.

We can check if it is the case with (struct file *)->i_sb->s_type->name == "overlay".

[alban]

Better check without string comparisons:

--- a/pkg/kfilefields/bpf/filefields.bpf.c
+++ b/pkg/kfilefields/bpf/filefields.bpf.c
@@ -4,6 +4,11 @@
 #include <bpf/bpf_core_read.h>
 #include <bpf/bpf_tracing.h>
 
+// include/uapi/linux/magic.h
+#ifndef OVERLAYFS_SUPER_MAGIC
+#define OVERLAYFS_SUPER_MAGIC   0x794c7630
+#endif
+
 // configured by userspace
 const volatile u64 socket_ino = 0;
 
@@ -52,6 +57,7 @@ int BPF_KRETPROBE(ig_fget_x, struct file *ret)
        u64 current_pid_tgid;
        struct file_fields *ff;
        struct file *private_file;
+       struct file *real_file;
        int zero = 0;
 
        if (tracer_pid_tgid == 0)
@@ -68,8 +74,12 @@ int BPF_KRETPROBE(ig_fget_x, struct file *ret)
 
        ff->private_data = (u64)BPF_CORE_READ(ret, private_data);
        ff->f_op = (u64)BPF_CORE_READ(ret, f_op);
-       private_file = (struct file *)ff->private_data;
-       ff->private_inode = (u64)BPF_CORE_READ(private_file, f_inode);
+
+       real_file = ret;
+       if (BPF_CORE_READ(ret, f_inode, i_sb, s_magic) == OVERLAYFS_SUPER_MAGIC) {
+               real_file = (struct file *) ff->private_data;
+       }
+       ff->private_inode = (u64)BPF_CORE_READ(real_file, f_inode);
 
        // Initialize private_data for only one execution of __scm_send
        tracer_pid_tgid = 0;

[i-Pear]

The results are not consistent. So I don't think it works fine.

Yes, I can reproduce. At most of the time it works, but sometimes the PrivateInode is zero, or to be a short value :(

[alban]

The results are not consistent. So I don't think it works fine.

Yes, I can reproduce. At most of the time it works, but sometimes the PrivateInode is zero, or to be a short value :(

The randomness was due to a bug in my test program main.go. I added runtime.KeepAlive(file) (I updated my previous comment) to ensure the fd is not closed and recycled to something else. That fixed the randomness.

[i-Pear]

OK, I've learned once again that if a Golang program encounters sporadic randomness issues, the first thing to check is the garbage collector.

[alban]

It needs make clang-format and make ebpf-objects.

[i-Pear]

It needs make clang-format and make ebpf-objects.

Done, thanks.

[alban]

I suggest to rename inodeUUID to realInodePtr everywhere. What do you think?

And the comment before inodeRefCount should briefly explain why the real inode ptr is used instead of just the inode number, and why we need to care about getting the underlying file under overlayfs.

[i-Pear]

Do you mean using realInodePtr as a new type, and replace all uint64?

[alban]

No, uint64 is fine for me. I meant in the comment above and elsewhere in the file.

[alban]

Could ReadRealInodeFromFd be tested in an unit test? We would not test whether the pointer has the correct value, but rather that the comparison between two realInodePtr is correct. The test would need to setup two temporary overlay filesystems with the same lower layer.

This could be in a separate PR, so this PR is not delayed too much.

[i-Pear]

This should be a unittest for kfilefields, if the testing framework could support this, I'd like to do so.

[alban]

Thanks! LGTM from code review. I haven't tested it yet. Let's see the results of the CI first.

[alban]

I prepared the following workload:

$ cat main.c 
#include <stdlib.h>
#include <unistd.h>

int main() {
  for (;;) {
    sleep(2);
    void *ptr = malloc(1);
    sleep(1);
    free(ptr);
  }
  return 0;
}
$ cat Dockerfile 
FROM ubuntu
COPY main /bin/main
$ cat Makefile 
all:
	gcc -Wall -o main ./main.c

docker:
	docker build -t malloc .

I started the workload on the host filesystem:

$ docker run -ti --rm -v /:/host --name c1 ubuntu
# /host/home/alban/test/malloc/main
$ docker run -ti --rm -v /:/host --name c2 ubuntu
# /host/home/alban/test/malloc/main

And traced with and without filtering options:

sudo -E ./ig run trace_malloc
sudo -E ./ig run trace_malloc -c c1
sudo -E ./ig run trace_malloc -c c2

The container filtering and enrichment worked fine and there are no duplicate events.

Then, I tried the workload again but on the overlay filesystem:

$ docker run -ti --rm -v /:/host --name c1 malloc /bin/main
$ docker run -ti --rm -v /:/host --name c2 malloc /bin/main
$ sudo -E ./ig run trace_malloc
$ sudo -E ./ig run trace_malloc -c c1
$ sudo -E ./ig run trace_malloc -c c2

It worked fined as well.