trace/dns + trace/exec + trace/open: fix unsafe typecasting

[flyth]

Before this change, we cast incoming events from the RawSample byte slice directly to expected types (like in other gadgets). However, in these gadgets, we optimized the data we send in eBPF to reduce load by not sending empty fields. Casting those abbreviated events to the full struct though makes them not backed by memory anymore - and thus unsafe to use. This change adds a new type for each gadget containing only the guaranteed parts that are sent and fills the (dynamic) remainder directly from the RawSample data. In case of changes to the original event layout, these new structs have to be updated as well.

This also fixes an issue with the trace/dns example.

Fixes #2337.

[mauriciovasquezbernal]

I was trying another approach to avoid having to manually declare these structs: to copy the sample to a slice that is big enough, and then do the casting:

--- pkg/gadgets/trace/exec/tracer/tracer.go
+++ pkg/gadgets/trace/exec/tracer/tracer.go
@@ -148,9 +148,10 @@ func (t *Tracer) run() {
                        continue
                }
 
-               // this works regardless the kind of event because cwd is defined at the end of the
-               // structure. (Just before args that are handled in a different way below)
-               bpfEvent := (*execsnoopEvent)(unsafe.Pointer(&record.RawSample[0]))
+               var bpfEvent *execsnoopEvent
+               tmp := make([]byte, unsafe.Sizeof(*bpfEvent))
+               copy(tmp, record.RawSample)
+               bpfEvent = (*execsnoopEvent)(unsafe.Pointer(&tmp[0]))
 
                event := types.Event{
                        Event: eventtypes.Event{
@@ -174,7 +175,7 @@ func (t *Tracer) run() {
                args := &bpfEvent.Args
 
                if t.config.GetCwd {
-                       bpfEventWithCwd := (*execsnoopWithCwdEvent)(unsafe.Pointer(&record.RawSample[0]))
+                       bpfEventWithCwd := (*execsnoopWithCwdEvent)(unsafe.Pointer(&tmp[0]))
                        event.Cwd = gadgets.FromCString(bpfEventWithCwd.Cwd[:])
                        args = &bpfEventWithCwd.Args
                }

It'll create this extra copy, but perhaps it's better than having to duplicate the structures? What do you think? (I'm fine with either change).

btw, trace open also has this issue:

inspektor-gadget/pkg/gadgets/trace/open/tracer/bpf/opensnoop.bpf.c

Lines 225 to 227 in e63af49

	bpf_perf_event_output(ctx, &events, BPF_F_CURRENT_CPU, event,
	sizeof(struct event) -
	(PATH_MAX - full_fname_len));

[flyth]

It'll create this extra copy, but perhaps it's better than having to duplicate the structures? What do you think? (I'm fine with either change).

I'm also fine with both solutions, however, as this is the hot path of the gadget, I lean a bit towards the faster solution with slightly higher maintenance cost (and since it's "legacy" anyway... hehe).

btw, trace open also has this issue

Ah, thanks. I'll add it once we've decided which solution to use.

[mauriciovasquezbernal]

I'm also fine with both solutions, however, as this is the hot path of the gadget, I lean a bit towards the faster solution with slightly higher maintenance cost (and since it's "legacy" anyway... hehe).

I agree, let's go for your solution. Only thing, would you mind adding a comment on the structure definition on the .bpf.c files indicating it has to be keep in sync as well?

[mauriciovasquezbernal]

LGTM. Thanks for the fix!

[flyth]

Thanks for the reviews!