Skip to content

Commit

Permalink
fix(ebpf): use ts as fd_arg_path_map key (#3674)
Browse files Browse the repository at this point in the history
A race condition has been reported when sequential open/close calls
cause the same FD (part of the map key) to be reused, therefore causing
value corruption.

The fd_arg_path_map continues as BPF_LRU_HASH just to avoid the need to
delete entries in userland even though the map is not used as a LRU
anymore - just as "oldest inserted", since there are no subsequent
accesses to update the LRU status of any entry.

P.S.: Another solution would be to use the inode number as part of the
key, but this value would not be available in the event at decode stage.
  • Loading branch information
geyslan authored Nov 14, 2023
1 parent 5a87dd1 commit ca5d7eb
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 31 deletions.
2 changes: 1 addition & 1 deletion pkg/ebpf/c/maps.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ BPF_PROG_ARRAY(sys_exit_submit_tail, MAX_EVENT_ID); // store prog
BPF_PROG_ARRAY(sys_enter_init_tail, MAX_EVENT_ID); // store program for performing syscall tracking logic in sys_enter
BPF_PROG_ARRAY(sys_exit_init_tail, MAX_EVENT_ID); // store program for performing syscall tracking logic in sys_exits
BPF_STACK_TRACE(stack_addresses, MAX_STACK_ADDRESSES); // store stack traces
BPF_LRU_HASH(fd_arg_path_map, fd_arg_task_t, fd_arg_path_t, 1024); // store fds paths by task
BPF_LRU_HASH(fd_arg_path_map, u64, fd_arg_path_t, 1024); // store fds paths by timestamp
BPF_LRU_HASH(bpf_attach_map, u32, bpf_used_helpers_t, 1024); // holds bpf prog info
BPF_LRU_HASH(bpf_attach_tmp_map, u32, bpf_used_helpers_t, 1024); // temporarily hold bpf_used_helpers_t
BPF_LRU_HASH(bpf_prog_load_map, u32, void *, 1024); // store bpf prog aux pointer between bpf_check and security_bpf_prog
Expand Down
16 changes: 6 additions & 10 deletions pkg/ebpf/c/tracee.bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -152,22 +152,18 @@ int sys_enter_submit(struct bpf_raw_tracepoint_args *ctx)
if (p.config->options & OPT_TRANSLATE_FD_FILEPATH && has_syscall_fd_arg(sys->id)) {
// Process filepath related to fd argument
uint fd_num = get_syscall_fd_num_from_arg(sys->id, &sys->args);
struct file *file = get_struct_file_from_fd(fd_num);

if (file) {
fd_arg_task_t fd_arg_task = {
.pid = p.event->context.task.pid,
.tid = p.event->context.task.tid,
.fd = fd_num,
};
struct file *f = get_struct_file_from_fd(fd_num);

if (f) {
u64 ts = sys->ts;
fd_arg_path_t fd_arg_path = {};
void *file_path = get_path_str(__builtin_preserve_access_index(&file->f_path));
void *file_path = get_path_str(__builtin_preserve_access_index(&f->f_path));

bpf_probe_read_str(&fd_arg_path.path, sizeof(fd_arg_path.path), file_path);
bpf_map_update_elem(&fd_arg_path_map, &fd_arg_task, &fd_arg_path, BPF_ANY);
bpf_map_update_elem(&fd_arg_path_map, &ts, &fd_arg_path, BPF_ANY);
}
}

if (sys->id != SYSCALL_RT_SIGRETURN && !p.task_info->syscall_traced) {
save_to_submit_buf(&p.event->args_buf, (void *) &(sys->args.args[0]), sizeof(int), 0);
events_perf_submit(&p, sys->id, 0);
Expand Down
6 changes: 0 additions & 6 deletions pkg/ebpf/c/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -183,12 +183,6 @@ typedef struct syscall_data {
unsigned long ret; // Syscall ret val. May be used by syscall exit tail calls.
} syscall_data_t;

typedef struct fd_arg_task {
u32 pid;
u32 tid;
int fd;
} fd_arg_task_t;

#define MAX_CACHED_PATH_SIZE 64

typedef struct fd_arg_path {
Expand Down
3 changes: 2 additions & 1 deletion pkg/ebpf/events_pipeline.go
Original file line number Diff line number Diff line change
Expand Up @@ -694,8 +694,9 @@ func (t *Tracee) parseArguments(e *trace.Event) error {
if err != nil {
return errfmt.WrapError(err)
}

if t.config.Output.ParseArgumentsFDs {
return events.ParseArgsFDs(e, t.FDArgPathMap)
return events.ParseArgsFDs(e, uint64(t.getOrigEvtTimestamp(e)), t.FDArgPathMap)
}
}

Expand Down
12 changes: 12 additions & 0 deletions pkg/ebpf/processor_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,18 @@ func (t *Tracee) normalizeEventCtxTimes(event *trace.Event) error {
return nil
}

// getOrigEvtTimestamp returns the original timestamp of the event.
// To be used only when the event timestamp was normalized via normalizeEventCtxTimes.
func (t *Tracee) getOrigEvtTimestamp(event *trace.Event) int {
if t.config.Output.RelativeTime {
// if the time was normalized relative to tracee start time, add the start time back
return event.Timestamp + int(t.startTime)
}

// if the time was normalized to "wall" time, subtract the boot time
return event.Timestamp - int(t.bootTime)
}

// processSchedProcessFork processes a sched_process_fork event by normalizing the start time.
func (t *Tracee) processSchedProcessFork(event *trace.Event) error {
return t.normalizeEventArgTime(event, "start_time")
Expand Down
16 changes: 3 additions & 13 deletions pkg/events/parse_args.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,6 @@ import (
"github.com/aquasecurity/tracee/types/trace"
)

type fdArgTask struct {
PID uint32
TID uint32
FD int32
}

func ParseArgs(event *trace.Event) error {
for i := range event.Args {
if ptr, isUintptr := event.Args[i].Value.(uintptr); isUintptr {
Expand Down Expand Up @@ -281,15 +275,11 @@ func ParseArgs(event *trace.Event) error {
return nil
}

func ParseArgsFDs(event *trace.Event, fdArgPathMap *bpf.BPFMap) error {
func ParseArgsFDs(event *trace.Event, origTimestamp uint64, fdArgPathMap *bpf.BPFMap) error {
if fdArg := GetArg(event, "fd"); fdArg != nil {
if fd, isInt32 := fdArg.Value.(int32); isInt32 {
fdArgTask := &fdArgTask{
PID: uint32(event.ProcessID),
TID: uint32(event.ThreadID),
FD: fd,
}
bs, err := fdArgPathMap.GetValue(unsafe.Pointer(fdArgTask))
ts := origTimestamp
bs, err := fdArgPathMap.GetValue(unsafe.Pointer(&ts))
if err != nil {
return errfmt.WrapError(err)
}
Expand Down

0 comments on commit ca5d7eb

Please sign in to comment.