Skip to content

Commit

Permalink
chore(ebpf): replace sizeof with bpf type helpers
Browse files Browse the repository at this point in the history
When using sizeof on kernel structs, false results may happen due to the
sizeof builtin not being designed to take into account btf relocations.

Use the libbpf helpers bpf_core_type_size and bpf_core_field_size where
relevant.

NOTE: iovec size checks were left unchanged, since they are currently
using copies to stack allocated variables.
  • Loading branch information
NDStrahilevitz committed Jun 9, 2024
1 parent 68602ff commit 078a567
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 19 deletions.
16 changes: 8 additions & 8 deletions pkg/ebpf/c/common/buffer.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ statfunc buf_t *get_buf(int idx)
}

// biggest elem to be saved with 'save_to_submit_buf' should be defined here:
#define MAX_ELEMENT_SIZE sizeof(struct sockaddr_un)
#define MAX_ELEMENT_SIZE bpf_core_type_size(struct sockaddr_un)

statfunc int save_to_submit_buf(args_buffer_t *buf, void *ptr, u32 size, u8 index)
{
Expand Down Expand Up @@ -320,19 +320,19 @@ statfunc int save_sockaddr_to_buf(args_buffer_t *buf, struct socket *sock, u8 in
get_network_details_from_sock_v4(sk, &net_details, 0);
get_local_sockaddr_in_from_network_details(&local, &net_details, family);

save_to_submit_buf(buf, (void *) &local, sizeof(struct sockaddr_in), index);
save_to_submit_buf(buf, (void *) &local, bpf_core_type_size(struct sockaddr_in), index);
} else if (family == AF_INET6) {
net_conn_v6_t net_details = {};
struct sockaddr_in6 local;

get_network_details_from_sock_v6(sk, &net_details, 0);
get_local_sockaddr_in6_from_network_details(&local, &net_details, family);

save_to_submit_buf(buf, (void *) &local, sizeof(struct sockaddr_in6), index);
save_to_submit_buf(buf, (void *) &local, bpf_core_type_size(struct sockaddr_in6), index);
} else if (family == AF_UNIX) {
struct unix_sock *unix_sk = (struct unix_sock *) sk;
struct sockaddr_un sockaddr = get_unix_sock_addr(unix_sk);
save_to_submit_buf(buf, (void *) &sockaddr, sizeof(struct sockaddr_un), index);
save_to_submit_buf(buf, (void *) &sockaddr, bpf_core_type_size(struct sockaddr_un), index);
}
return 0;
}
Expand Down Expand Up @@ -398,13 +398,13 @@ statfunc int save_args_to_submit_buf(event_data_t *event, args_t *args)
bpf_probe_read(&family, sizeof(short), (void *) args->args[i]);
switch (family) {
case AF_UNIX:
size = sizeof(struct sockaddr_un);
size = bpf_core_type_size(struct sockaddr_un);
break;
case AF_INET:
size = sizeof(struct sockaddr_in);
size = bpf_core_type_size(struct sockaddr_in);
break;
case AF_INET6:
size = sizeof(struct sockaddr_in6);
size = bpf_core_type_size(struct sockaddr_in6);
break;
default:
size = sizeof(short);
Expand All @@ -420,7 +420,7 @@ statfunc int save_args_to_submit_buf(event_data_t *event, args_t *args)
rc = save_to_submit_buf(&(event->args_buf), (void *) (args->args[i]), size, index);
break;
case TIMESPEC_T:
size = sizeof(struct __kernel_timespec);
size = bpf_core_type_size(struct __kernel_timespec);
rc = save_to_submit_buf(&(event->args_buf), (void *) (args->args[i]), size, index);
break;
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/ebpf/c/common/filesystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ statfunc size_t get_path_str_buf(struct path *path, buf_t *out_buf)
}

struct path f_path;
bpf_probe_read_kernel(&f_path, sizeof(struct path), path);
bpf_probe_read_kernel(&f_path, bpf_core_type_size(struct path), path);
char slash = '/';
int zero = 0;
struct dentry *dentry = f_path.dentry;
Expand Down
3 changes: 2 additions & 1 deletion pkg/ebpf/c/common/network.h
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ statfunc volatile unsigned char get_sock_state(struct sock *sock)
statfunc struct ipv6_pinfo *get_inet_pinet6(struct inet_sock *inet)
{
struct ipv6_pinfo *pinet6_own_impl;
bpf_core_read(&pinet6_own_impl, sizeof(pinet6_own_impl), &inet->pinet6);
bpf_core_read(&pinet6_own_impl, sizeof(struct ipv6_pinfo *), &inet->pinet6);
return pinet6_own_impl;
}

Expand All @@ -369,6 +369,7 @@ statfunc struct sockaddr_un get_unix_sock_addr(struct unix_sock *sock)
struct unix_address *addr = BPF_CORE_READ(sock, addr);
int len = BPF_CORE_READ(addr, len);
struct sockaddr_un sockaddr = {};
// NOTE(nadav.str): stack allocated, so runtime core size check is avoided
if (len <= sizeof(struct sockaddr_un)) {
bpf_probe_read(&sockaddr, len, addr->name);
}
Expand Down
26 changes: 17 additions & 9 deletions pkg/ebpf/c/tracee.bpf.c
Original file line number Diff line number Diff line change
Expand Up @@ -509,20 +509,23 @@ statfunc int send_socket_dup(program_data_t *p, u64 oldfd, u64 newfd)
get_network_details_from_sock_v4(sk, &net_details, 0);
get_remote_sockaddr_in_from_network_details(&remote, &net_details, family);

save_to_submit_buf(&(p->event->args_buf), &remote, sizeof(struct sockaddr_in), 2);
save_to_submit_buf(
&(p->event->args_buf), &remote, bpf_core_type_size(struct sockaddr_in), 2);
} else if (family == AF_INET6) {
net_conn_v6_t net_details = {};
struct sockaddr_in6 remote;

get_network_details_from_sock_v6(sk, &net_details, 0);
get_remote_sockaddr_in6_from_network_details(&remote, &net_details, family);

save_to_submit_buf(&(p->event->args_buf), &remote, sizeof(struct sockaddr_in6), 2);
save_to_submit_buf(
&(p->event->args_buf), &remote, bpf_core_type_size(struct sockaddr_in6), 2);
} else if (family == AF_UNIX) {
struct unix_sock *unix_sk = (struct unix_sock *) sk;
struct sockaddr_un sockaddr = get_unix_sock_addr(unix_sk);

save_to_submit_buf(&(p->event->args_buf), &sockaddr, sizeof(struct sockaddr_un), 2);
save_to_submit_buf(
&(p->event->args_buf), &sockaddr, bpf_core_type_size(struct sockaddr_un), 2);
}

return events_perf_submit(p, 0);
Expand Down Expand Up @@ -2636,13 +2639,13 @@ int BPF_KPROBE(trace_security_socket_connect)
size_t sockaddr_len = 0;
switch (sa_fam) {
case AF_INET:
sockaddr_len = sizeof(struct sockaddr_in);
sockaddr_len = bpf_core_type_size(struct sockaddr_in);
break;
case AF_INET6:
sockaddr_len = sizeof(struct sockaddr_in6);
sockaddr_len = bpf_core_type_size(struct sockaddr_in6);
break;
case AF_UNIX:
sockaddr_len = sizeof(struct sockaddr_un);
sockaddr_len = bpf_core_type_size(struct sockaddr_un);
if (addr_len < sockaddr_len)
need_workaround = true;

Expand All @@ -2654,6 +2657,7 @@ int BPF_KPROBE(trace_security_socket_connect)
// Workaround for sockaddr_un struct length (issue: #1129).
struct sockaddr_un sockaddr = {0};
bpf_probe_read(&sockaddr, (u32) addr_len, (void *) address);
// NOTE(nadav.str): stack allocated, so runtime core size check is avoided
stsb(args_buf, (void *) &sockaddr, sizeof(struct sockaddr_un), 2);
}
#endif
Expand Down Expand Up @@ -2757,7 +2761,8 @@ int BPF_KPROBE(trace_security_socket_bind)
connect_id.protocol = protocol;

if (sa_fam == AF_INET) {
save_to_submit_buf(&p.event->args_buf, (void *) address, sizeof(struct sockaddr_in), 1);
save_to_submit_buf(
&p.event->args_buf, (void *) address, bpf_core_type_size(struct sockaddr_in), 1);

struct sockaddr_in *addr = (struct sockaddr_in *) address;

Expand All @@ -2767,7 +2772,8 @@ int BPF_KPROBE(trace_security_socket_bind)
connect_id.port = BPF_CORE_READ(addr, sin_port);
}
} else if (sa_fam == AF_INET6) {
save_to_submit_buf(&p.event->args_buf, (void *) address, sizeof(struct sockaddr_in6), 1);
save_to_submit_buf(
&p.event->args_buf, (void *) address, bpf_core_type_size(struct sockaddr_in6), 1);

struct sockaddr_in6 *addr = (struct sockaddr_in6 *) address;

Expand All @@ -2779,12 +2785,14 @@ int BPF_KPROBE(trace_security_socket_bind)
#if defined(__TARGET_ARCH_x86) // TODO: this is broken in arm64 (issue: #1129)
if (addr_len <= sizeof(struct sockaddr_un)) {
struct sockaddr_un sockaddr = {};
// NOTE(nadav.str): stack allocated, so runtime core size check is avoided
bpf_probe_read(&sockaddr, addr_len, (void *) address);
save_to_submit_buf(
&p.event->args_buf, (void *) &sockaddr, sizeof(struct sockaddr_un), 1);
} else
#endif
save_to_submit_buf(&p.event->args_buf, (void *) address, sizeof(struct sockaddr_un), 1);
save_to_submit_buf(
&p.event->args_buf, (void *) address, bpf_core_type_size(struct sockaddr_un), 1);
}

return events_perf_submit(&p, 0);
Expand Down

0 comments on commit 078a567

Please sign in to comment.