Skip to content

Commit

Permalink
exec: fix tracking of matchBinary children
Browse files Browse the repository at this point in the history
If we executed a binary that directly executed in another binary, the
->mb_bitset field was zeroed out and the information was lost, which
meant that the matching would not work as expected.

Fix this by i) not zeroing out the ->mb_bitset field and, also,
or it with the parent.

Add a test.

Fixes: e73718c ("exec: track matchbinary children")

Signed-off-by: Kornilios Kourtis <kornilios@isovalent.com>
  • Loading branch information
kkourt committed Dec 9, 2024
1 parent f8a58fd commit 41c7a74
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 6 deletions.
13 changes: 13 additions & 0 deletions bpf/lib/process.h
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,22 @@ struct binary {
// STRING_POSTFIX_MAX_LENGTH reversed last bytes of the path
char end_r[STRING_POSTFIX_MAX_LENGTH];
// matchBinary bitset for binary
// NB: everything after and including ->mb_bitset will not be zeroed on a new exec. See
// binary_reset().
mbset_t mb_bitset;
}; // All fields aligned so no 'packed' attribute

FUNC_INLINE void
binary_reset(struct binary *b)
{
// buffer can be written at clone stage with parent's info, if previous path is longer than
// current, we can have leftovers at the end, so zero out bin structure.
//
// Do not zero the ->mb_bitset however, so that it can be inherited if exec() is called.
// This depends on ->mb_bitset being the last part of the struct.
memset(b, 0, offsetof(struct binary, mb_bitset));
}

// The execve_map_value is tracked by the TGID of the thread group
// the msg_execve_key.pid. The thread IDs are recorded on the
// fly and sent with every corresponding event.
Expand Down
7 changes: 3 additions & 4 deletions bpf/process/bpf_execve_event.c
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ void update_mb_bitset(struct binary *bin)
/* ->mb_bitset is used to track matchBinary matches to children (followChildren), so
* here we propagate the parent value to the child.
*/
bin->mb_bitset = parent->bin.mb_bitset;
bin->mb_bitset |= parent->bin.mb_bitset;
}

/* check the map and see if the binary path matches a binary
Expand Down Expand Up @@ -385,9 +385,8 @@ execve_send(void *ctx __arg_ctx)
curr->caps.inheritable = event->creds.caps.inheritable;
}
#endif
// buffer can be written at clone stage with parent's info, if previous
// path is longer than current, we can have leftovers at the end.
memset(&curr->bin, 0, sizeof(curr->bin));
/* zero out previous paths in ->bin */
binary_reset(&curr->bin);
#ifdef __LARGE_BPF_PROG
// read from proc exe stored at execve time
if (event->exe.len <= BINARY_PATH_MAX_LEN) {
Expand Down
13 changes: 11 additions & 2 deletions pkg/sensors/tracing/matchbinaries_follow_children_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,16 @@ func TestMatchBinariesFollowChildren(t *testing.T) {
t.Fatalf("failed to run command %s: %v", cmd, err)
}
}

perfring.RunTest(t, ctx, ops, eventFn)
require.Equal(t, 1, getcpuCnt)
require.Equal(t, 1, getcpuCnt, "single exec")

getcpuCnt = 0
ops2 := func() {
cmd := exec.Command(tmpShPath, "-c", fmt.Sprintf("exec sh -c %s", getcpuBin))
if err := cmd.Run(); err != nil {
t.Fatalf("failed to run command %s: %v", cmd, err)
}
}
perfring.RunTest(t, ctx, ops2, eventFn)
require.Equal(t, 1, getcpuCnt, "double exec")
}

0 comments on commit 41c7a74

Please sign in to comment.