fix(driver/modern_bpf): simplify apply_dynamic_snaplen func

Signed-off-by: Roberto Scolaro <[email protected]>
falcosecurity · Apr 12, 2024 · 03835ed · 03835ed
1 parent 90cbbfc
commit 03835ed
Show file tree

Hide file tree

Showing 19 changed files with 68 additions and 129 deletions.
diff --git a/driver/modern_bpf/helpers/extract/extract_from_kernel.h b/driver/modern_bpf/helpers/extract/extract_from_kernel.h
@@ -29,13 +29,6 @@ enum capability_type
 	CAP_EFFECTIVE = 2,
 };
 
-enum syscall_nr
-{
-	SCN_UNSET = 0,
-	SCN_SENDTO  = 1,
-	SCN_SENDMSG = 2,
-};
-
 /* All the functions that are called in bpf to extract parameters
  * start with the `extract` prefix.
  */

diff --git a/driver/modern_bpf/helpers/store/auxmap_store_params.h b/driver/modern_bpf/helpers/store/auxmap_store_params.h
@@ -1298,7 +1298,7 @@ static __always_inline void auxmap__store_fdlist_param(struct auxiliary_map *aux
 	push__param_len(auxmap->data, &auxmap->lengths_pos, sizeof(uint16_t) + (num_pairs * (sizeof(int64_t) + sizeof(int16_t))));
 }
 
-static __always_inline void apply_dynamic_snaplen(struct pt_regs *regs, uint16_t *snaplen, bool only_port_range, enum syscall_nr syscall_number)
+static __always_inline void apply_dynamic_snaplen(struct pt_regs *regs, uint16_t *snaplen, bool only_port_range, struct sockaddr *sockaddr)
 {
 	if(!maps__get_do_dynamic_snaplen())
 	{
@@ -1339,121 +1339,66 @@ static __always_inline void apply_dynamic_snaplen(struct pt_regs *regs, uint16_t
 	 *  - recvmsg
 	 *  - sendmsg
 	 */
+	unsigned long args[3];
+	extract__network_args(args, 3, regs);
 
-	int32_t socket_fd = 0;
-	uint16_t port_local = 0;
-	uint16_t port_remote = 0;
-	uint16_t socket_family = 0;
-	unsigned long args[6];
-	bool extracted = true;
-	struct sockaddr *sockaddr;
-
-	switch(syscall_number)
+	/* All the syscalls involved in this logic have the `fd` as first syscall argument */
+	int32_t socket_fd = (int32_t)args[0];
+	if(socket_fd < 0)
 	{
-		case SCN_SENDTO:
-		{
-			extract__network_args(args, 6, regs);
-
-			socket_fd = (int32_t)args[0];
-			if(socket_fd < 0)
-			{
-				return;
-			}
-
-			if((void*)args[4] != NULL)
-			{
-				sockaddr = (struct sockaddr*)args[4];
-				BPF_CORE_READ_USER_INTO(&socket_family, (struct sockaddr*)sockaddr, sa_family);
-			}
-			break;
-		}
-		case SCN_SENDMSG:
-		{
-			extract__network_args(args, 3, regs);
-
-			socket_fd = (int32_t)args[0];
-			if(socket_fd < 0)
-			{
-				return;
-			}
-
-			if((void*)args[1] != NULL)
-			{
-				BPF_CORE_READ_USER_INTO(&sockaddr, (struct msghdr*)args[1], msg_name);
-				BPF_CORE_READ_USER_INTO(&socket_family, sockaddr, sa_family);
-			}
-			break;
-		}
-		default:
-		{
-			extracted = false;
-			break;
-		}
+		return;
 	}
 
-	// If socket_family is 0 we skip this part.
-	switch(socket_family)
+	struct file *file = extract__file_struct_from_fd(socket_fd);
+	struct socket *socket = BPF_CORE_READ(file, private_data);
+	struct sock *sk = BPF_CORE_READ(socket, sk);
+	if(!sk)
 	{
-		case AF_INET:
-		{
-			struct sockaddr_in sockaddr_in = {};
-			bpf_probe_read_user(&sockaddr_in, bpf_core_type_size(struct sockaddr_in), sockaddr);
-			port_remote = ntohs(sockaddr_in.sin_port);
-			break;
-		}
-		case AF_INET6:
-		{
-			struct sockaddr_in6 sockaddr_in6 = {};
-			bpf_probe_read_user(&sockaddr_in6, bpf_core_type_size(struct sockaddr_in6), sockaddr);
-			port_remote = ntohs(sockaddr_in6.sin6_port);
-			break;
-		}
-		default:
-			break;
+		return;
 	}
 
-	if(port_local == 0 || port_remote == 0)
+	uint16_t port_local = 0;
+	uint16_t port_remote = 0;
+
+	/* We perform some checks regarding ports only for these 2 families */
+	uint16_t socket_family = BPF_CORE_READ(sk, __sk_common.skc_family);
+	/* We return if `fd` is not a socket */
+	if(socket_family == 0)
 	{
-		// Extract args only if they are not already been extracted.
-		if(!extracted)
-		{
-			extract__network_args(args, 3, regs);
-			/* All the syscalls involved in this logic have the `fd` as first syscall argument */
-			socket_fd = (int32_t)args[0];
-			if(socket_fd < 0)
-			{
-				return;
-			}
-		}
+		return;
+	}
 
-		struct file *file = extract__file_struct_from_fd(socket_fd);
-		struct socket *socket = BPF_CORE_READ(file, private_data);
-		struct sock *sk = BPF_CORE_READ(socket, sk);
-		if(!sk)
-		{
-			return;
-		}
+	if(socket_family == AF_INET || socket_family == AF_INET6)
+	{
+		struct inet_sock *inet = (struct inet_sock *)sk;
+		BPF_CORE_READ_INTO(&port_local, inet, inet_sport);
+		BPF_CORE_READ_INTO(&port_remote, sk, __sk_common.skc_dport);
+		port_local = ntohs(port_local);
+		port_remote = ntohs(port_remote);
 
-		/* We perform some checks regarding ports only for these 2 families */
-		socket_family = BPF_CORE_READ(sk, __sk_common.skc_family);
-		/* We return if `fd` is not a socket */
-		if(socket_family == 0)
+		if(port_remote == 0 && sockaddr != NULL)
 		{
-			return;
-		}
+			BPF_CORE_READ_USER_INTO(&socket_family, (struct sockaddr*)sockaddr, sa_family);
 
-		if(socket_family == AF_INET || socket_family == AF_INET6)
-		{
-			struct inet_sock *inet = (struct inet_sock *)sk;
-			if(port_local == 0)
-			{
-				BPF_CORE_READ_INTO(&port_local, inet, inet_sport);
-				port_local = ntohs(port_local);
-			}
-			if(port_remote == 0)
+			// If socket_family is 0 we skip this part.
+			switch(socket_family)
 			{
-				BPF_CORE_READ_INTO(&port_remote, sk, __sk_common.skc_dport);
-				port_remote = ntohs(port_remote);
+				case AF_INET:
+				{
+					struct sockaddr_in sockaddr_in = {};
+					bpf_probe_read_user(&sockaddr_in, bpf_core_type_size(struct sockaddr_in), sockaddr);
+					port_remote = ntohs(sockaddr_in.sin_port);
+					break;
+				}
+				case AF_INET6:
+				{
+					struct sockaddr_in6 sockaddr_in6 = {};
+					bpf_probe_read_user(&sockaddr_in6, bpf_core_type_size(struct sockaddr_in6), sockaddr);
+					port_remote = ntohs(sockaddr_in6.sin6_port);
+					break;
+				}
+				default:
+					break;
 			}
 		}
 	}

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/pread64.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/pread64.bpf.c
@@ -73,7 +73,7 @@ int BPF_PROG(pread64_x,
 		 * have in the buffer.
 		 */
 		uint16_t snaplen = maps__get_snaplen();
-		apply_dynamic_snaplen(regs, &snaplen, false, SCN_UNSET);
+		apply_dynamic_snaplen(regs, &snaplen, false, NULL);
 		if(snaplen > ret)
 		{
 			snaplen = ret;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/preadv.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/preadv.bpf.c
@@ -72,7 +72,7 @@ int BPF_PROG(preadv_x,
 		 * have in the buffer.
 		 */
 		uint16_t snaplen = maps__get_snaplen();
-		apply_dynamic_snaplen(regs, &snaplen, true, SCN_UNSET);
+		apply_dynamic_snaplen(regs, &snaplen, true, NULL);
 		if(snaplen > ret)
 		{
 			snaplen = ret;

diff --git a/...r/modern_bpf/programs/tail_called/events/syscall_dispatched_events/process_vm_readv.bpf.c b/...r/modern_bpf/programs/tail_called/events/syscall_dispatched_events/process_vm_readv.bpf.c
@@ -68,7 +68,7 @@ int BPF_PROG(process_vm_readv_x,
 		 * have in the buffer.
 		 */
 		uint16_t snaplen = maps__get_snaplen();
-		apply_dynamic_snaplen(regs, &snaplen, true, SCN_UNSET);
+		apply_dynamic_snaplen(regs, &snaplen, true, NULL);
 		if(snaplen > ret)
 		{
 			snaplen = ret;

diff --git a/.../modern_bpf/programs/tail_called/events/syscall_dispatched_events/process_vm_writev.bpf.c b/.../modern_bpf/programs/tail_called/events/syscall_dispatched_events/process_vm_writev.bpf.c
@@ -68,7 +68,7 @@ int BPF_PROG(process_vm_writev_x,
 		 * have in the buffer.
 		 */
 		uint16_t snaplen = maps__get_snaplen();
-		apply_dynamic_snaplen(regs, &snaplen, true, SCN_UNSET);
+		apply_dynamic_snaplen(regs, &snaplen, true, NULL);
 		if(snaplen > ret)
 		{
 			snaplen = ret;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/pwrite64.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/pwrite64.bpf.c
@@ -72,7 +72,7 @@ int BPF_PROG(pwrite64_x,
 	 */
 	int64_t bytes_to_read = ret > 0 ? ret : extract__syscall_argument(regs, 2);
 	uint16_t snaplen = maps__get_snaplen();
-	apply_dynamic_snaplen(regs, &snaplen, false, SCN_UNSET);
+	apply_dynamic_snaplen(regs, &snaplen, false, NULL);
 	if((int64_t)snaplen > bytes_to_read)
 	{
 		snaplen = bytes_to_read;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/pwritev.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/pwritev.bpf.c
@@ -75,7 +75,7 @@ int BPF_PROG(pwritev_x,
 	 * the return value if the syscall is successful.
 	 */
 	uint16_t snaplen = maps__get_snaplen();
-	apply_dynamic_snaplen(regs, &snaplen, true, SCN_UNSET);
+	apply_dynamic_snaplen(regs, &snaplen, true, NULL);
 	if(ret > 0 && snaplen > ret)
 	{
 		snaplen = ret;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/read.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/read.bpf.c
@@ -69,7 +69,7 @@ int BPF_PROG(read_x,
 		 * have in the buffer.
 		 */
 		uint16_t snaplen = maps__get_snaplen();
-		apply_dynamic_snaplen(regs, &snaplen, false, SCN_UNSET);
+		apply_dynamic_snaplen(regs, &snaplen, false, NULL);
 		if(snaplen > ret)
 		{
 			snaplen = ret;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/readv.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/readv.bpf.c
@@ -68,7 +68,7 @@ int BPF_PROG(readv_x,
 		 * have in the buffer.
 		 */
 		uint16_t snaplen = maps__get_snaplen();
-		apply_dynamic_snaplen(regs, &snaplen, true, SCN_UNSET);
+		apply_dynamic_snaplen(regs, &snaplen, true, NULL);
 		if(snaplen > ret)
 		{
 			snaplen = ret;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/recv.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/recv.bpf.c
@@ -74,7 +74,7 @@ int BPF_PROG(recv_x,
 		extract__network_args(args, 2, regs);
 
 		uint16_t snaplen = maps__get_snaplen();
-		apply_dynamic_snaplen(regs, &snaplen, false, SCN_UNSET);
+		apply_dynamic_snaplen(regs, &snaplen, false, NULL);
 		if(snaplen > ret)
 		{
 			snaplen = ret;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/recvfrom.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/recvfrom.bpf.c
@@ -77,7 +77,7 @@ int BPF_PROG(recvfrom_x,
 		 * have in the buffer.
 		 */
 		uint16_t snaplen = maps__get_snaplen();
-		apply_dynamic_snaplen(regs, &snaplen, false, SCN_UNSET);
+		apply_dynamic_snaplen(regs, &snaplen, false, NULL);
 		if(snaplen > ret)
 		{
 			snaplen = ret;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/recvmsg.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/recvmsg.bpf.c
@@ -77,7 +77,7 @@ int BPF_PROG(recvmsg_x,
 		 * have in the buffer.
 		 */
 		uint16_t snaplen = maps__get_snaplen();
-		apply_dynamic_snaplen(regs, &snaplen, true, SCN_UNSET);
+		apply_dynamic_snaplen(regs, &snaplen, true, NULL);
 		if(snaplen > ret)
 		{
 			snaplen = ret;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/send.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/send.bpf.c
@@ -73,7 +73,7 @@ int BPF_PROG(send_x,
 
 	int64_t bytes_to_read = ret > 0 ? ret : args[2];
 	uint16_t snaplen = maps__get_snaplen();
-	apply_dynamic_snaplen(regs, &snaplen, false, SCN_UNSET);
+	apply_dynamic_snaplen(regs, &snaplen, false, NULL);
 	if((int64_t)snaplen > bytes_to_read)
 	{
 		snaplen = bytes_to_read;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/sendmsg.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/sendmsg.bpf.c
@@ -96,7 +96,9 @@ int BPF_PROG(sendmsg_x,
 	 * the return value if the syscall is successful.
 	 */
 	uint16_t snaplen = maps__get_snaplen();
-	apply_dynamic_snaplen(regs, &snaplen, true, SCN_SENDMSG);
+	struct sockaddr *sockaddr;
+	BPF_CORE_READ_USER_INTO(&sockaddr, (struct msghdr*)args[1], msg_name);
+	apply_dynamic_snaplen(regs, &snaplen, true, sockaddr);
 	if(ret > 0 && snaplen > ret)
 	{
 		snaplen = ret;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/sendto.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/sendto.bpf.c
@@ -85,15 +85,15 @@ int BPF_PROG(sendto_x,
 	auxmap__store_s64_param(auxmap, ret);
 
 	/* Collect parameters at the beginning to manage socketcalls */
-	unsigned long args[3];
-	extract__network_args(args, 3, regs);
+	unsigned long args[5];
+	extract__network_args(args, 5, regs);
 
 	/* If the syscall doesn't fail we use the return value as `size`
 	 * otherwise we need to rely on the syscall parameter provided by the user.
 	 */
 	int64_t bytes_to_read = ret > 0 ? ret : args[2];
 	uint16_t snaplen = maps__get_snaplen();
-	apply_dynamic_snaplen(regs, &snaplen, false, SCN_SENDTO);
+	apply_dynamic_snaplen(regs, &snaplen, false, (struct sockaddr*)args[4]);
 	if((int64_t)snaplen > bytes_to_read)
 	{
 		snaplen = bytes_to_read;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/write.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/write.bpf.c
@@ -68,7 +68,7 @@ int BPF_PROG(write_x,
 	 */
 	int64_t bytes_to_read = ret > 0 ? ret : extract__syscall_argument(regs, 2);
 	uint16_t snaplen = maps__get_snaplen();
-	apply_dynamic_snaplen(regs, &snaplen, false, SCN_UNSET);
+	apply_dynamic_snaplen(regs, &snaplen, false, NULL);
 	if((int64_t)snaplen > bytes_to_read)
 	{
 		snaplen = bytes_to_read;

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/writev.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/writev.bpf.c
@@ -71,7 +71,7 @@ int BPF_PROG(writev_x,
 	 * the return value if the syscall is successful.
 	 */
 	uint16_t snaplen = maps__get_snaplen();
-	apply_dynamic_snaplen(regs, &snaplen, true, SCN_UNSET);
+	apply_dynamic_snaplen(regs, &snaplen, true, NULL);
 	if(ret > 0 && snaplen > ret)
 	{
 		snaplen = ret;

diff --git a/test/libsinsp_e2e/udp_client_server.cpp b/test/libsinsp_e2e/udp_client_server.cpp
@@ -1035,7 +1035,6 @@ TEST_F(sys_call_test, statsd_client_snaplen)
 	captured_event_callback_t callback = [&](const callback_param& param)
 	{
 		sinsp_evt* e = param.m_evt;
-		std::cout << e->get_name() << std::endl;
 		EXPECT_EQ(payload, e->get_param_value_str("data"))
 		    << "Failure on " << e->get_name() << " n=" << n;
 		n++;