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.

Sign up for GitHub

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-modern-bpf): fix execve USER vs MEMORY reads + cleanup + enforce upper bounds in push__bytebuf #633

Merged
merged 5 commits into from
Oct 13, 2022
Merged

fix(driver-modern-bpf): fix execve USER vs MEMORY reads + cleanup + enforce upper bounds in push__bytebuf #633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9d796ac
cleanup(driver-modern-bpf): minor refactor of push__bytebuf to reflec…
incertum Sep 26, 2022
df17037
cleanup(driver-modern-bpf): enforce upper bounds for push__bytebuf
incertum Sep 26, 2022
a94d950
fix(driver-modern-bpf): fix USER vs MEMORY attribution in execve filler
incertum Sep 26, 2022
7bf2082
cleanup(driver-modern-bpf): skip push__bytebuf reads when no valid le…
incertum Sep 26, 2022
1a388da
cleanup(driver-modern-bpf): start moving bound checks to auxmap__store*
incertum Oct 4, 2022
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
22 changes: 10 additions & 12 deletions driver/modern_bpf/helpers/base/push_data.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,25 +253,23 @@ static __always_inline u16 push__charbuf(u8 *data, u64 *payload_pos, unsigned lo
*/
static __always_inline u16 push__bytebuf(u8 *data, u64 *payload_pos, unsigned long bytebuf_pointer, u16 len_to_read, enum read_memory mem)
{

int written_bytes = 0;

if(mem == KERNEL)
{
written_bytes = bpf_probe_read_kernel(&data[SAFE_ACCESS(*payload_pos)],
if(bpf_probe_read_kernel(&data[SAFE_ACCESS(*payload_pos)],
len_to_read,
(char *)bytebuf_pointer);
(void *)bytebuf_pointer) != 0)
{
return 0;
}
}
else
{
written_bytes = bpf_probe_read_user(&data[SAFE_ACCESS(*payload_pos)],
if(bpf_probe_read_user(&data[SAFE_ACCESS(*payload_pos)],
len_to_read,
(char *)bytebuf_pointer);
}

if(written_bytes != 0)
{
return 0;
(void *)bytebuf_pointer) != 0)
{
return 0;
}
}

*payload_pos += len_to_read;
Expand Down
6 changes: 5 additions & 1 deletion driver/modern_bpf/helpers/store/auxmap_store_params.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,11 @@ static __always_inline u16 auxmap__store_charbuf_param(struct auxiliary_map *aux
*/
static __always_inline u16 auxmap__store_bytebuf_param(struct auxiliary_map *auxmap, unsigned long bytebuf_pointer, unsigned long len_to_read, enum read_memory mem)
{
u16 bytebuf_len = push__bytebuf(auxmap->data, &auxmap->payload_pos, bytebuf_pointer, len_to_read, mem);
u16 bytebuf_len = 0;
if (len_to_read > 0)
{
bytebuf_len = push__bytebuf(auxmap->data, &auxmap->payload_pos, bytebuf_pointer, len_to_read, mem);
}
/* If we are not able to push anything with `push__bytebuf`
* `bytebuf_len` will be equal to `0` so we will send an
* empty param to userspace.
Expand Down
6 changes: 3 additions & 3 deletions driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/execve.bpf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,15 +79,15 @@ int BPF_PROG(execve_x,
/* We need to extract the len of `exe` arg so we can undestand
* the overall length of the remaining args.
*/
u16 exe_arg_len = auxmap__store_charbuf_param(auxmap, arg_start_pointer, KERNEL);
u16 exe_arg_len = auxmap__store_charbuf_param(auxmap, arg_start_pointer, USER);

/* Parameter 3: args (type: PT_CHARBUFARRAY) */
/* Here we read the whole array starting from the pointer to the first
* element. We could also read the array element per element but
* since we know the total len we read it as a `bytebuf`.
* The `\0` after every argument are preserved.
*/
auxmap__store_bytebuf_param(auxmap, arg_start_pointer + exe_arg_len, total_args_len - exe_arg_len, KERNEL);
auxmap__store_bytebuf_param(auxmap, arg_start_pointer + exe_arg_len, total_args_len - exe_arg_len, USER);
}
else
{
Expand Down Expand Up @@ -203,7 +203,7 @@ int BPF_PROG(t1_execve_x,
* since we know the total len we read it as a `bytebuf`.
* The `\0` after every argument are preserved.
*/
auxmap__store_bytebuf_param(auxmap, env_start_pointer, total_env_len, KERNEL);
auxmap__store_bytebuf_param(auxmap, env_start_pointer, total_env_len, USER);
}
else
{
Expand Down