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

feat(libsinsp_e2e): add unix_udp_client_server_read test #2231

Merged
merged 5 commits into from
Jan 14, 2025
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
21 changes: 14 additions & 7 deletions driver/bpf/filler_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -425,10 +425,7 @@ static __always_inline bool bpf_getsockname(struct socket *sock,

u = (struct unix_sock *)sk;
addr = _READ(u->addr);
if(!addr) {
sunaddr->sun_family = AF_UNIX;
sunaddr->sun_path[0] = 0;
} else {
if(u && addr) {
unsigned int len = _READ(addr->len);

if(len > sizeof(struct sockaddr_storage))
Expand All @@ -440,6 +437,13 @@ static __always_inline bool bpf_getsockname(struct socket *sock,
#else
bpf_probe_read_kernel(sunaddr, len, addr->name);
#endif
} else {
sunaddr->sun_family = AF_UNIX;
sunaddr->sun_path[0] = 0;
// The first byte to 0 can be confused with an `abstract socket address` for this reason
// we put also the second byte to 0 to comunicate to the caller that the address is not
// valid.
sunaddr->sun_path[1] = 0;
}

break;
Expand Down Expand Up @@ -880,6 +884,7 @@ static __always_inline long bpf_fd_to_socktuple(struct filler_data *data,
*/
struct unix_sock *us = (struct unix_sock *)sk;
struct sock *speer = _READ(us->peer);
struct sockaddr_un *usrsockaddr_un;
char *us_name = NULL;

data->buf[data->state->tail_ctx.curoff & SCRATCH_SIZE_HALF] = socket_family_to_scap(family);
Expand All @@ -895,15 +900,17 @@ static __always_inline long bpf_fd_to_socktuple(struct filler_data *data,
memcpy(&data->buf[(data->state->tail_ctx.curoff + 1 + 8) & SCRATCH_SIZE_HALF], &us, 8);
bpf_getsockname(sock, peer_address, 1);
us_name = ((struct sockaddr_un *)peer_address)->sun_path;
if(us_name[0] == '\0' && us_name[1] == '\0' && usrsockaddr != NULL) {
usrsockaddr_un = (struct sockaddr_un *)usrsockaddr;
us_name = usrsockaddr_un->sun_path;
}
}

size = 1 + 8 + 8;
int res = unix_socket_path(
&data->buf[(data->state->tail_ctx.curoff + 1 + 8 + 8) & SCRATCH_SIZE_HALF],
us_name,
UNIX_PATH_MAX);
size += res;

size = 1 + 8 + 8 + res;
break;
}
}
Expand Down
2 changes: 1 addition & 1 deletion driver/modern_bpf/helpers/base/shared_size.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
#define MAX_IOVCNT 32

/* Maximum number of supported sendmmsg/recvmmsg messages */
#define MAX_SENDMMSG_RECVMMSG_SIZE 16
#define MAX_SENDMMSG_RECVMMSG_SIZE 8

/* Maximum number of `pollfd` structures that we can analyze. */
#define MAX_POLLFD 16
Expand Down
26 changes: 15 additions & 11 deletions driver/modern_bpf/helpers/store/auxmap_store_params.h
Original file line number Diff line number Diff line change
Expand Up @@ -776,7 +776,8 @@ static __always_inline void auxmap__store_socktuple_param(struct auxiliary_map *

case AF_UNIX: {
struct unix_sock *socket_local = (struct unix_sock *)sk;
struct unix_sock *socket_remote = (struct unix_sock *)BPF_CORE_READ(socket_local, peer);
struct unix_sock *socket_peer = (struct unix_sock *)BPF_CORE_READ(socket_local, peer);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A rename.

struct sockaddr_un usrsockaddr_un = {};
char *path = NULL;

/* Pack the tuple info:
Expand All @@ -787,18 +788,23 @@ static __always_inline void auxmap__store_socktuple_param(struct auxiliary_map *
*/
push__u8(auxmap->data, &auxmap->payload_pos, socket_family_to_scap(socket_family));
if(direction == OUTBOUND) {
push__u64(auxmap->data, &auxmap->payload_pos, (uint64_t)socket_remote);
push__u64(auxmap->data, &auxmap->payload_pos, (uint64_t)socket_peer);
push__u64(auxmap->data, &auxmap->payload_pos, (uint64_t)socket_local);
path = BPF_CORE_READ(socket_remote, addr, name[0].sun_path);
if(socket_peer == NULL && usrsockaddr != NULL) {
bpf_probe_read_user(&usrsockaddr_un,
bpf_core_type_size(struct sockaddr_un),
(void *)usrsockaddr);
path = usrsockaddr_un.sun_path;
} else {
path = BPF_CORE_READ(socket_peer, addr, name[0].sun_path);
}
} else {
push__u64(auxmap->data, &auxmap->payload_pos, (uint64_t)socket_local);
push__u64(auxmap->data, &auxmap->payload_pos, (uint64_t)socket_remote);
push__u64(auxmap->data, &auxmap->payload_pos, (uint64_t)socket_peer);
path = BPF_CORE_READ(socket_local, addr, name[0].sun_path);
}

unsigned long start_reading_point;
char first_path_byte = *(char *)path;
if(first_path_byte == '\0') {
if(path[0] == '\0') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid convoluted logic with multiple variables involved.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i have the suspect it was used to avoid some verifier madness, but let's see 🤞

/* Please note exceptions in the `sun_path`:
* Taken from: https://man7.org/linux/man-pages/man7/unix.7.html
*
Expand All @@ -808,14 +814,12 @@ static __always_inline void auxmap__store_socktuple_param(struct auxiliary_map *
*
* So in this case, we need to skip the initial `\0`.
*/
start_reading_point = (unsigned long)path + 1;
} else {
start_reading_point = (unsigned long)path;
path++;
}

uint16_t written_bytes = push__charbuf(auxmap->data,
&auxmap->payload_pos,
start_reading_point,
(unsigned long)path,
MAX_UNIX_SOCKET_PATH,
KERNEL);
final_param_len = FAMILY_SIZE + KERNEL_POINTER + KERNEL_POINTER + written_bytes;
Expand Down
36 changes: 18 additions & 18 deletions driver/ppm_events.c
Original file line number Diff line number Diff line change
Expand Up @@ -233,30 +233,24 @@ inline int sock_getname(struct socket *sock, struct sockaddr *sock_address, int
case AF_UNIX: {
struct sockaddr_un *sunaddr = (struct sockaddr_un *)sock_address;
struct unix_sock *u;
struct unix_address *u_addr = NULL;

if(peer) {
if(peer)
sk = ((struct unix_sock *)sk)->peer;
if(!sk) {
return -ENOTCONN;
}
}

u = (struct unix_sock *)sk;
u_addr = u->addr;
if(!u_addr) {
if(u && u->addr) {
unsigned int len = u->addr->len;
if(unlikely(len > sizeof(struct sockaddr_storage))) {
len = sizeof(struct sockaddr_storage);
}
memcpy(sunaddr, u->addr->name, len);
} else {
sunaddr->sun_family = AF_UNIX;
sunaddr->sun_path[0] = 0;
// The first byte to 0 can be confused with an `abstract socket address` for this reason
// we put also the second byte to 0 to comunicate to the caller that the address is not
// valid.
sunaddr->sun_path[1] = 0;
} else {
unsigned int len = u_addr->len;
if(unlikely(len > sizeof(struct sockaddr_storage))) {
len = sizeof(struct sockaddr_storage);
}
memcpy(sunaddr, u_addr->name, len);
}
break;
}
Expand Down Expand Up @@ -861,6 +855,11 @@ static struct socket *ppm_sockfd_lookup_light(int fd, int *err, int *fput_needed
*/

static void unix_socket_path(char *dest, const char *path, size_t size) {
if(path == NULL) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid de-referencing a possibly null ptr.

dest[0] = '\0';
return;
}

if(path[0] == '\0') {
/* Please note exceptions in the `sun_path`:
* Taken from: https://man7.org/linux/man-pages/man7/unix.7.html
Expand Down Expand Up @@ -999,8 +998,8 @@ uint16_t fd_to_socktuple(int fd,
struct sockaddr_in *usrsockaddr_in;
struct sockaddr_in6 *usrsockaddr_in6;
uint16_t size;
struct sockaddr_storage sock_address;
struct sockaddr_storage peer_address;
struct sockaddr_storage sock_address = {};
struct sockaddr_storage peer_address = {};
struct socket *sock;
char *dest;
struct unix_sock *us;
Expand Down Expand Up @@ -1173,7 +1172,8 @@ uint16_t fd_to_socktuple(int fd,
} else {
*(uint64_t *)(targetbuf + 1) = (uint64_t)(unsigned long)speer;
*(uint64_t *)(targetbuf + 1 + 8) = (uint64_t)(unsigned long)us;
sock_getname(sock, (struct sockaddr *)&peer_address, 1);
err = sock_getname(sock, (struct sockaddr *)&peer_address, 1);
ASSERT(err == 0);
us_name = ((struct sockaddr_un *)&peer_address)->sun_path;
}

Expand All @@ -1182,7 +1182,7 @@ uint16_t fd_to_socktuple(int fd,
// Note that we check the second byte of `us_name`, see `sock_getname` for more details.
// Some times `usrsockaddr` is provided as a NULL pointer, checking `use_userdata` should be
// enough but just to be sure we check also `usrsockaddr != NULL`
if(us_name && us_name[1] == '\0' && use_userdata && usrsockaddr != NULL) {
if((!us_name || (us_name[0] == '\0' && us_name[1] == '\0')) && usrsockaddr != NULL) {
usrsockaddr_un = (struct sockaddr_un *)usrsockaddr;

/*
Expand Down
76 changes: 76 additions & 0 deletions test/drivers/test_suites/syscall_enter_suite/sendto_e.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,82 @@ TEST(SyscallEnter, sendtoE_ipv4_tcp_NULL_sockaddr) {

/*=============================== UDP ===========================*/

TEST(SyscallEnter, sendtoE_unix_udp) {
auto evt_test = get_syscall_event_test(__NR_sendto, ENTER_EVENT);

evt_test->enable_capture();

/*=============================== TRIGGER SYSCALL ===========================*/

int32_t server_socket_fd = syscall(__NR_socket, AF_UNIX, SOCK_DGRAM, 0);
assert_syscall_state(SYSCALL_SUCCESS, "socket", server_socket_fd, NOT_EQUAL, -1);
evt_test->server_reuse_address_port(server_socket_fd);
sockaddr_un server_addr = {};
evt_test->server_fill_sockaddr_un(&server_addr);
assert_syscall_state(SYSCALL_SUCCESS,
"bind (server)",
syscall(__NR_bind, server_socket_fd, &server_addr, sizeof(server_addr)),
NOT_EQUAL,
-1);

int32_t client_socket_fd = syscall(__NR_socket, AF_UNIX, SOCK_DGRAM, 0);
assert_syscall_state(SYSCALL_SUCCESS, "socket", client_socket_fd, NOT_EQUAL, -1);
evt_test->client_reuse_address_port(client_socket_fd);
sockaddr_un client_addr = {};
evt_test->client_fill_sockaddr_un(&client_addr);
assert_syscall_state(SYSCALL_SUCCESS,
"bind (client)",
syscall(__NR_bind, client_socket_fd, &client_addr, sizeof(client_addr)),
NOT_EQUAL,
-1);

const void* sent_data = (const void*)SHORT_MESSAGE;
size_t sent_data_len = SHORT_MESSAGE_LEN;
uint32_t sendto_flags = 0;

int64_t sent_bytes = syscall(__NR_sendto,
client_socket_fd,
sent_data,
sent_data_len,
sendto_flags,
&server_addr,
sizeof(server_addr));
assert_syscall_state(SYSCALL_SUCCESS, "sendto (client)", sent_bytes, NOT_EQUAL, -1);

syscall(__NR_shutdown, server_socket_fd, 2);
syscall(__NR_shutdown, client_socket_fd, 2);
close(client_socket_fd);
close(server_socket_fd);
syscall(__NR_unlinkat, 0, UNIX_CLIENT, 0);
syscall(__NR_unlinkat, 0, UNIX_SERVER, 0);

/*=============================== TRIGGER SYSCALL ===========================*/

evt_test->disable_capture();

evt_test->assert_event_presence();

if(HasFatalFailure()) {
return;
}

evt_test->parse_event();

evt_test->assert_header();

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

/* Parameter 2: size (type: PT_UINT32)*/
evt_test->assert_numeric_param(2, (uint32_t)SHORT_MESSAGE_LEN);

/* Parameter 3: tuple (type: PT_SOCKTUPLE)*/
evt_test->assert_tuple_unix_param(3, PPM_AF_UNIX, UNIX_SERVER);

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

evt_test->assert_num_params_pushed(3);
}

TEST(SyscallEnter, sendtoE_ipv4_udp) {
auto evt_test = get_syscall_event_test(__NR_sendto, ENTER_EVENT);

Expand Down
1 change: 1 addition & 0 deletions test/libsinsp_e2e/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ add_executable(
thread_state.cpp
udp_client_server.cpp
unix_client_server.cpp
unix_udp_client_server.cpp
)

if(BUILD_BPF)
Expand Down
8 changes: 3 additions & 5 deletions test/libsinsp_e2e/udp_client_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -194,11 +194,9 @@ class udp_server {
}

void wait_for_server_ready() {
{
std::unique_lock<std::mutex> lock(m_mutex);
m_condition_server_ready.wait(lock, [this]() { return m_server_ready; });
m_server_ready = false;
}
std::unique_lock<std::mutex> lock(m_mutex);
m_condition_server_ready.wait(lock, [this]() { return m_server_ready; });
m_server_ready = false;
}

int64_t get_tid() { return m_tid; }
Expand Down
5 changes: 0 additions & 5 deletions test/libsinsp_e2e/unix_client_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,12 @@ limitations under the License.

#include <algorithm>
#include <cassert>
#include <list>
#include <tuple>

#include <libsinsp/sinsp_int.h>

#define NAME "/tmp/python_unix_sockets_example"
#define PAYLOAD "0123456789QWERTYUIOPASDFGHJKLZXCVBNM"

#define PAYLOAD "0123456789QWERTYUIOPASDFGHJKLZXCVBNM"
#define BUFFER_LENGTH (sizeof(PAYLOAD) - 1)
#define FALSE 0

inline void parse_tuple(const std::string& tuple,
std::string& srcstr,
Expand Down
Loading
Loading