Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(driver,test): drop socketcalls called with wrong SYS_ argument in all 3 drivers #1501

Merged
merged 3 commits into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions driver/bpf/fillers.h
Original file line number Diff line number Diff line change
Expand Up @@ -3043,6 +3043,18 @@ FILLER(sys_generic, true)
const struct syscall_evt_pair *sc_evt;

native_id = bpf_syscall_get_nr(data->ctx);

// We are already in a tail-called filler.
// if we are in ia32 syscall sys_{enter,exit} already
// validated the converted 32bit->64bit syscall ID for us,
// otherwise the event would've been discarded.
#if defined(CONFIG_X86_64) && defined(CONFIG_IA32_EMULATION)
if (bpf_in_ia32_syscall())
{
native_id = convert_ia32_to_64(native_id);
}
#endif

sc_evt = get_syscall_info(native_id);
if (!sc_evt) {
bpf_printk("no routing for syscall %d\n", native_id);
Expand Down
14 changes: 12 additions & 2 deletions driver/bpf/probe.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,15 @@ BPF_PROBE("raw_syscalls/", sys_enter, sys_enter_args)
#ifdef BPF_SUPPORTS_RAW_TRACEPOINTS
bool is_syscall_return = false;
int return_code = convert_network_syscalls(ctx, &is_syscall_return);
if (return_code == -1)
Andreagit97 marked this conversation as resolved.
Show resolved Hide resolved
{
// Wrong SYS_ argument passed. Drop the syscall.
return 0;
}
if(!is_syscall_return)
{
evt_type = return_code;
drop_flags = return_code == PPME_GENERIC_E ? UF_ALWAYS_DROP : UF_USED;
drop_flags = UF_USED;
}
else
{
Expand Down Expand Up @@ -210,10 +215,15 @@ BPF_PROBE("raw_syscalls/", sys_exit, sys_exit_args)
#ifdef BPF_SUPPORTS_RAW_TRACEPOINTS
bool is_syscall_return = false;
int return_code = convert_network_syscalls(ctx, &is_syscall_return);
if (return_code == -1)
{
// Wrong SYS_ argument passed. Drop the syscall.
return 0;
}
if(!is_syscall_return)
{
evt_type = return_code + 1; // we are in sys_exit!
drop_flags = return_code == PPME_GENERIC_X ? UF_ALWAYS_DROP : UF_USED;
drop_flags = UF_USED;
}
else
{
Expand Down
17 changes: 7 additions & 10 deletions driver/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -1408,6 +1408,12 @@ static inline struct event_data_t *manage_socketcall(struct event_data_t *event_
{
bool is_syscall_return;
int return_code = convert_network_syscalls(event_data->event_info.syscall_data.regs, &is_syscall_return);
if (return_code == -1)
FedeDP marked this conversation as resolved.
Show resolved Hide resolved
{
// Wrong SYS_ argument passed. Drop the syscall.
return NULL;
}


/* If the return code is not the generic event we will need to extract parameters
* with the socket call mechanism.
Expand All @@ -1423,19 +1429,10 @@ static inline struct event_data_t *manage_socketcall(struct event_data_t *event_
// and`id=__NR_ia32_socketcall`...We resolved the correct event type but we cannot
// update the `id`.
event_data->deny_syscalls_filtering = true;

/* The user provided a wrong code, we will send a generic event,
* no need for socket call arguments extraction logic.
*/
if(return_code == PPME_GENERIC_E)
{
event_data->extract_socketcall_params = false;
}
/* we need to use `return_code + 1` because return_code
* is the enter event.
*/
record_event_all_consumers(return_code + is_exit,
return_code == PPME_GENERIC_E ? UF_ALWAYS_DROP : UF_USED,
record_event_all_consumers(return_code + is_exit, UF_USED,
event_data, is_exit ? KMOD_PROG_SYS_EXIT : KMOD_PROG_SYS_ENTER);
return NULL; // managed
}
Expand Down
14 changes: 4 additions & 10 deletions driver/modern_bpf/helpers/interfaces/syscalls_dispatcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,6 @@ static __always_inline bool syscalls_dispatcher__64bit_interesting_syscall(uint3
return maps__64bit_interesting_syscall(syscall_id);
}

static __always_inline uint32_t syscalls_dispatcher__convert_ia32_to_64(uint32_t syscall_id)
{
return maps__ia32_to_64(syscall_id);
}

static __always_inline long convert_network_syscalls(struct pt_regs *regs)
{
int socketcall_id = (int)extract__syscall_argument(regs, 0);
Expand Down Expand Up @@ -160,10 +155,9 @@ static __always_inline long convert_network_syscalls(struct pt_regs *regs)
* -> In this case we drop the event
*/

// Reset NR_socketcall to send a generic even with correct id
#ifdef __NR_socketcall
return __NR_socketcall;
#else
/* If we are not able to convert to a syscall we drop the event.
* This should happen in the cases listed above or when we receive
* a wrong SOCKETCALL code.
*/
return -1;
#endif
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ int BPF_PROG(sys_enter,
}
else
{
syscall_id = syscalls_dispatcher__convert_ia32_to_64(syscall_id);
syscall_id = maps__ia32_to_64(syscall_id);
// syscalls defined only on 32 bits are dropped here.
if(syscall_id == (uint32_t)-1)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ int BPF_PROG(sys_exit,
*/
if(syscall_id != X86_64_NR_EXECVE && syscall_id != X86_64_NR_EXECVEAT)
{
syscall_id = syscalls_dispatcher__convert_ia32_to_64(syscall_id);
syscall_id = maps__ia32_to_64(syscall_id);
if(syscall_id == (uint32_t)-1)
{
return 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ int BPF_PROG(generic_e,

/*=============================== COLLECT PARAMETERS ===========================*/

// We are already in a tail-called filler.
// if we are in ia32 syscall sys_{enter,exit} already
// validated the converted 32bit->64bit syscall ID for us,
// otherwise the event would've been discarded.
#if defined(__TARGET_ARCH_x86)
if(bpf_in_ia32_syscall())
{
id = maps__ia32_to_64(id);
}
#endif

/* Parameter 1: ID (type: PT_SYSCALLID) */
/* This is the PPM_SC code obtained from the syscall id. */
ringbuf__store_u16(&ringbuf, maps__get_ppm_sc(id));
Expand Down Expand Up @@ -58,9 +69,21 @@ int BPF_PROG(generic_x,

/*=============================== COLLECT PARAMETERS ===========================*/

uint32_t id = extract__syscall_id(regs);
// We are already in a tail-called filler.
// if we are in ia32 syscall sys_{enter,exit} already
// validated the converted 32bit->64bit syscall ID for us,
// otherwise the event would've been discarded.
#if defined(__TARGET_ARCH_x86)
if(bpf_in_ia32_syscall())
{
id = maps__ia32_to_64(id);
}
#endif

/* Parameter 1: ID (type: PT_SYSCALLID) */
/* This is the PPM_SC code obtained from the syscall id. */
ringbuf__store_u16(&ringbuf, maps__get_ppm_sc(extract__syscall_id(regs)));
ringbuf__store_u16(&ringbuf, maps__get_ppm_sc(id));

/*=============================== COLLECT PARAMETERS ===========================*/

Expand Down
2 changes: 1 addition & 1 deletion driver/socketcall_to_syscall.h
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ static inline int socketcall_code_to_syscall_code(int socketcall_code, bool* is_
break;
}

return PPME_GENERIC_E;
return -1;
}

#endif /* SOCKETCALL_TO_SYSCALL_H */
22 changes: 8 additions & 14 deletions test/drivers/test_suites/actions_suite/ia32.cpp.in
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,6 @@ TEST(Actions, ia32)
*/
evt_test->assert_event_absence(ret_pid, PPME_SOCKET_ACCEPT4_6_E);
evt_test->assert_event_absence(ret_pid, PPME_SOCKET_ACCEPT4_6_X);

/*
* Last event sent by the script is a socketcall with non existing SYS_ code.
* We don't expect any event generated.
*/
evt_test->assert_event_absence(ret_pid, PPME_GENERIC_E);
evt_test->assert_event_absence(ret_pid, PPME_GENERIC_X);
}
else
{
Expand All @@ -112,14 +105,15 @@ TEST(Actions, ia32)
*/
evt_test->assert_event_presence(ret_pid, PPME_SOCKET_ACCEPT_5_E);
evt_test->assert_event_presence(ret_pid, PPME_SOCKET_ACCEPT_5_X);

/*
* Last event sent by the script is a socketcall with non existing SYS_ code.
* We expect generic events to be pushed.
*/
evt_test->assert_event_presence(ret_pid, PPME_GENERIC_E);
evt_test->assert_event_presence(ret_pid, PPME_GENERIC_X);
}
/*
* Last event sent by the script is a socketcall with non existing SYS_ code.
* We don't expect any event generated.
*/
// We cannot assert PPME_GENERIC_E since 'exit_group' syscall is sent when program exits,
// and it is mapped to a generic enter event.
// evt_test->assert_event_absence(ret_pid, PPME_GENERIC_E);
evt_test->assert_event_absence(ret_pid, PPME_GENERIC_X);
}

#ifdef __NR_execve
Expand Down
51 changes: 5 additions & 46 deletions test/drivers/test_suites/syscall_exit_suite/socketcall_x.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3317,6 +3317,7 @@ TEST(SyscallExit, socketcall_getsocknameX)

TEST(SyscallExit, socketcall_wrong_code_socketcall_interesting)
{
// Even if the socketcall is marked as interesting we drop the event
auto evt_test = get_syscall_event_test(__NR_socketcall, EXIT_EVENT);

evt_test->enable_capture();
Expand All @@ -3335,33 +3336,12 @@ TEST(SyscallExit, socketcall_wrong_code_socketcall_interesting)

evt_test->disable_capture();

evt_test->assert_event_presence(CURRENT_PID, PPME_GENERIC_X);

if(HasFatalFailure())
{
return;
}

evt_test->parse_event();

evt_test->assert_header();

/*=============================== ASSERT PARAMETERS ===========================*/

/* Parameter 1: ID (type: PT_SYSCALLID) */
/* this is the PPM_SC code obtained from the syscall id. */
evt_test->assert_numeric_param(1, (uint16_t)PPM_SC_SOCKETCALL);

/*=============================== ASSERT PARAMETERS ===========================*/

evt_test->assert_num_params_pushed(1);
evt_test->assert_event_absence(CURRENT_PID, PPME_GENERIC_X);
}

TEST(SyscallExit, socketcall_wrong_code_socketcall_not_interesting)
{
/* if we don't set the socketcall as interesting we won't obtain a generic event.
* In this case we set `setsockopt` as interesting
*/
// Same as the previous test
auto evt_test = get_syscall_event_test(__NR_setsockopt, EXIT_EVENT);

evt_test->enable_capture();
Expand Down Expand Up @@ -3433,6 +3413,7 @@ TEST(SyscallExit, socketcall_null_pointer)

TEST(SyscallExit, socketcall_null_pointer_and_wrong_code_socketcall_interesting)
{
// We send a wrong code so the event will be dropped
auto evt_test = get_syscall_event_test(__NR_socketcall, EXIT_EVENT);

evt_test->enable_capture();
Expand All @@ -3446,29 +3427,7 @@ TEST(SyscallExit, socketcall_null_pointer_and_wrong_code_socketcall_interesting)

evt_test->disable_capture();

/* Even if we have a null pointer, we will send a generic event so we don't need to preload
* the socketcall params! This is the reason why we don't drop the event in this case.
*/
evt_test->assert_event_presence(CURRENT_PID, PPME_GENERIC_X);

if(HasFatalFailure())
{
return;
}

evt_test->parse_event();

evt_test->assert_header();

/*=============================== ASSERT PARAMETERS ===========================*/

/* Parameter 1: ID (type: PT_SYSCALLID) */
/* this is the PPM_SC code obtained from the syscall id. */
evt_test->assert_numeric_param(1, (uint16_t)PPM_SC_SOCKETCALL);

/*=============================== ASSERT PARAMETERS ===========================*/

evt_test->assert_num_params_pushed(1);
evt_test->assert_event_absence(CURRENT_PID, PPME_GENERIC_X);
}

#endif /* __NR_socketcall */