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): avoid sending a NULL tuple in recvfrom syscall #1767

Merged
merged 2 commits into from
Mar 28, 2024
Merged

fix(driver): avoid sending a NULL tuple in recvfrom syscall #1767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f908ec1
fix(driver): avoid sending a null tuple in recvfrom
mrgian Mar 27, 2024
e3afeb6
fix(test): expect a non-null tuple in recvfrom syscall
mrgian Mar 27, 2024
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
18 changes: 16 additions & 2 deletions driver/bpf/fillers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4175,6 +4175,11 @@ FILLER(sys_recvfrom_x, true)
CHECK_RES(res);

if (retval >= 0) {
/*
* Get the fd
*/
fd = bpf_syscall_get_argument(data, 0);

/*
* Get the address
*/
Expand All @@ -4196,8 +4201,6 @@ FILLER(sys_recvfrom_x, true)
err = bpf_addr_to_kernel(usrsockaddr, addrlen,
(struct sockaddr *)data->tmp_scratch);
if (err >= 0) {
fd = bpf_syscall_get_argument(data, 0);

/*
* Convert the fd into socket endpoint information
*/
Expand All @@ -4209,6 +4212,17 @@ FILLER(sys_recvfrom_x, true)
true,
data->tmp_scratch + sizeof(struct sockaddr_storage));
}
} else {
/*
* Get socket endpoint information from fd if the user-provided *sockaddr is NULL
*/
size = bpf_fd_to_socktuple(data,
fd,
NULL,
0,
false,
true,
data->tmp_scratch + sizeof(struct sockaddr_storage));
}
}

Expand Down
7 changes: 4 additions & 3 deletions driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/recvfrom.bpf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,17 @@ int BPF_PROG(recvfrom_x,
}

/* Collect parameters at the beginning to manage socketcalls */
unsigned long args[2];
extract__network_args(args, 2, regs);
unsigned long args[5];
extract__network_args(args, 5, regs);

/* Parameter 2: data (type: PT_BYTEBUF) */
unsigned long received_data_pointer = args[1];
auxmap__store_bytebuf_param(auxmap, received_data_pointer, snaplen, USER);

/* Parameter 3: tuple (type: PT_SOCKTUPLE) */
uint32_t socket_fd = (uint32_t)args[0];
auxmap__store_socktuple_param(auxmap, socket_fd, INBOUND, NULL);
struct sockaddr *usrsockaddr = (struct sockaddr *)args[4];
auxmap__store_socktuple_param(auxmap, socket_fd, INBOUND, usrsockaddr);
}
else
{
Expand Down
8 changes: 0 additions & 8 deletions driver/ppm_events.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,10 +204,6 @@ inline int sock_getname(struct socket* sock, struct sockaddr* sock_address, int

sin->sin_family = AF_INET;
if (peer) {
if (!inet->inet_dport ||
((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_SYN_SENT))) {
return -ENOTCONN;
}
sin->sin_port = inet->inet_dport;
sin->sin_addr.s_addr = inet->inet_daddr;
} else {
Expand All @@ -228,10 +224,6 @@ inline int sock_getname(struct socket* sock, struct sockaddr* sock_address, int

sin->sin6_family = AF_INET6;
if (peer) {
if ((!inet->inet_dport) ||
((1 << sk->sk_state) & (TCPF_CLOSE | TCPF_SYN_SENT))) {
return -ENOTCONN;
}
sin->sin6_port = inet->inet_dport;
sin->sin6_addr = sk->sk_v6_daddr;
} else {
Expand Down
11 changes: 11 additions & 0 deletions driver/ppm_fillers.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2544,6 +2544,17 @@ int f_sys_recvfrom_x(struct event_filler_arguments *args)
targetbuf,
STR_STORAGE_SIZE);
}
} else {
/*
* Get socket endpoint information from fd if the user-provided *sockaddr is NULL
*/
size = fd_to_socktuple(fd,
NULL,
0,
false,
true,
targetbuf,
STR_STORAGE_SIZE);
}
}

Expand Down
48 changes: 10 additions & 38 deletions test/drivers/test_suites/syscall_exit_suite/recvfrom_x.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -215,17 +215,8 @@ TEST(SyscallExit, recvfromX_tcp_connection_NULL_sockaddr)
evt_test->assert_bytebuf_param(2, FULL_MESSAGE, DEFAULT_SNAPLEN);

/* Parameter 3: tuple (type: PT_SOCKTUPLE) */
if(evt_test->is_modern_bpf_engine())
{
/* The server performs a 'recvmsg` so the server is the final destination of the packet while the client is the src. */
evt_test->assert_tuple_inet_param(3, PPM_AF_INET, IPV4_CLIENT, IPV4_SERVER, IPV4_PORT_CLIENT_STRING, IPV4_PORT_SERVER_STRING);
}
else
{
evt_test->assert_empty_param(3);
evt_test->assert_num_params_pushed(3);
GTEST_SKIP() << "[RECVFROM_X]: we send an empty tuple, but we can fix this case" << std::endl;
}
/* The server performs a 'recvmsg` so the server is the final destination of the packet while the client is the src. */
evt_test->assert_tuple_inet_param(3, PPM_AF_INET, IPV4_CLIENT, IPV4_SERVER, IPV4_PORT_CLIENT_STRING, IPV4_PORT_SERVER_STRING);

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

Expand Down Expand Up @@ -293,20 +284,9 @@ TEST(SyscallExit, recvfromX_udp_connection_snaplen)
evt_test->assert_bytebuf_param(2, FULL_MESSAGE, DEFAULT_SNAPLEN);

/* Parameter 3: tuple (type: PT_SOCKTUPLE) */
if(!evt_test->is_modern_bpf_engine())
{
/* The server performs a 'recvmsg` so the server is the final destination of the packet while the client is the src. */
evt_test->assert_tuple_inet_param(3, PPM_AF_INET, IPV4_CLIENT, IPV4_SERVER, IPV4_PORT_CLIENT_STRING, IPV4_PORT_SERVER_STRING);
}
else
{
/* In UDP connections we cannot extract the tuple from kernel structs we always need to use the userspace struct
* Right now the modern probe doesn't support this behavior we need to fix it
*/
evt_test->assert_tuple_inet_param(3, PPM_AF_INET, IPV4_EMPTY, IPV4_SERVER, IPV4_PORT_EMPTY_STRING, IPV4_PORT_SERVER_STRING);
evt_test->assert_num_params_pushed(3);
GTEST_SKIP() << "[RECVFROM_X]: we send a tuple without the source, but we can fix this case" << std::endl;
}
/* The server performs a 'recvmsg` so the server is the final destination of the packet while the client is the src. */
evt_test->assert_tuple_inet_param(3, PPM_AF_INET, IPV4_CLIENT, IPV4_SERVER, IPV4_PORT_CLIENT_STRING, IPV4_PORT_SERVER_STRING);


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

Expand Down Expand Up @@ -372,19 +352,11 @@ TEST(SyscallExit, recvfromX_udp_connection_NULL_sockaddr)
evt_test->assert_bytebuf_param(2, FULL_MESSAGE, DEFAULT_SNAPLEN);

/* Parameter 3: tuple (type: PT_SOCKTUPLE) */
if(!evt_test->is_modern_bpf_engine())
{
evt_test->assert_empty_param(3);
GTEST_SKIP() << "[RECVFROM_X]: we send an empty tuple, but we can at least send the dest ip and source" << std::endl;
}
else
{
/* This is the correct behavior because if the userspace struct is empty
* we cannot extract the source ip and port, unless we directly read the packet
* headers!
*/
evt_test->assert_tuple_inet_param(3, PPM_AF_INET, IPV4_EMPTY, IPV4_SERVER, IPV4_PORT_EMPTY_STRING, IPV4_PORT_SERVER_STRING);
}
/* If the userspace struct is empty
* we cannot extract the source ip and port, unless we directly read the packet
* headers!
*/
evt_test->assert_tuple_inet_param(3, PPM_AF_INET, IPV4_EMPTY, IPV4_SERVER, IPV4_PORT_EMPTY_STRING, IPV4_PORT_SERVER_STRING);

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

Expand Down
Loading