fix(drivers): align and improve connect kernel drivers fillers

The current bpf and kmod's `connect` fillers implementations extract
socket tuple information only if the socket file descriptor is greater
than or equal to zero: this does not ensure that the file descriptor
is valid and that the kernel has the correct amount of information
needed to build the socket tuple.

On the other side, the modern probe implementation correctly checks
the system call return value on the exit program, but doesn't try
hard enough to build the socket tuple, as it does not leverage the
sockaddr provided by the user.

Align all three implementations, checking the system call return value
and leveraging the user-provided sockaddr information to build the
socket tuple.

Signed-off-by: Leonardo Di Giovanna <[email protected]>

Author: ekoops

driver/bpf/fillers.h

@@ -1404,101 +1404,89 @@ FILLER(sys_setrlimit_x, true) {
 }
 
 FILLER(sys_connect_e, true) {
-	struct sockaddr *usrsockaddr;
-	unsigned long val;
-	long size = 0;
-	long retval;
-	int err;
-	int res;
-	int fd;
-
-	fd = bpf_syscall_get_argument(data, 0);
-	res = bpf_push_s64_to_ring(data, fd);
+	/* Parameter 1: fd (type: PT_FD) */
+	int64_t fd = (int64_t)(int32_t)bpf_syscall_get_argument(data, 0);
+	int res = bpf_push_s64_to_ring(data, fd);
 	CHECK_RES(res);
 
-	if(fd >= 0) {
-		usrsockaddr = (struct sockaddr *)bpf_syscall_get_argument(data, 1);
-		val = bpf_syscall_get_argument(data, 2);
+	/* Get the sockaddr pointer and its length. */
+	struct sockaddr __user *usrsockaddr =
+	        (struct sockaddr __user *)bpf_syscall_get_argument(data, 1);
+	unsigned long usrsockaddr_len = bpf_syscall_get_argument(data, 2);
 
-		if(usrsockaddr && val != 0) {
-			/*
-			 * Copy the address
-			 */
-			err = bpf_addr_to_kernel(usrsockaddr, val, (struct sockaddr *)data->tmp_scratch);
-			if(err >= 0) {
-				/*
-				 * Convert the fd into socket endpoint information
-				 */
-				size = bpf_pack_addr(data, (struct sockaddr *)data->tmp_scratch, val);
-			}
+	long addr_size = 0;
+	if(usrsockaddr != NULL && usrsockaddr_len != 0) {
+		struct sockaddr *ksockaddr = (struct sockaddr *)data->tmp_scratch;
+		/* Copy the address into kernel memory. */
+		res = bpf_addr_to_kernel(usrsockaddr, usrsockaddr_len, ksockaddr);
+		if(likely(res >= 0)) {
+			/* Convert the fd into socket endpoint information. */
+			addr_size = bpf_pack_addr(data, ksockaddr, usrsockaddr_len);
 		}
 	}
 
-	/*
-	 * Copy the endpoint info into the ring
-	 */
+	/* Parameter 2: addr (type: PT_SOCKADDR) */
 	data->curarg_already_on_frame = true;
-	res = bpf_val_to_ring_len(data, 0, size);
-
-	return res;
+	return bpf_val_to_ring_len(data, 0, addr_size);
 }
 
 FILLER(sys_connect_x, true) {
-	struct sockaddr *usrsockaddr;
-	unsigned long val;
-	long size = 0;
-	long retval;
-	int err;
-	int res;
-	int fd;
-
-	/*
-	 * Push the result
-	 */
-	retval = bpf_syscall_get_retval(data->ctx);
-	res = bpf_push_s64_to_ring(data, retval);
+	/* Parameter 1: res (type: PT_ERRNO) */
+	long retval = bpf_syscall_get_retval(data->ctx);
+	int res = bpf_push_s64_to_ring(data, retval);
 	CHECK_RES(res);
 
-	/*
-	 * Retrieve the fd and push it to the ring.
-	 * Note that, even if we are in the exit callback, the arguments are still
-	 * in the stack, and therefore we can consume them.
-	 */
-	fd = bpf_syscall_get_argument(data, 0);
-	if(fd >= 0) {
-		usrsockaddr = (struct sockaddr *)bpf_syscall_get_argument(data, 1);
-		val = bpf_syscall_get_argument(data, 2);
+	int64_t fd = (int64_t)(int32_t)bpf_syscall_get_argument(data, 0);
 
-		if(usrsockaddr && val != 0) {
-			/*
-			 * Copy the address
-			 */
-			err = bpf_addr_to_kernel(usrsockaddr, val, (struct sockaddr *)data->tmp_scratch);
-			if(err >= 0) {
-				/*
-				 * Convert the fd into socket endpoint information
-				 */
-				size = bpf_fd_to_socktuple(data,
-				                           fd,
-				                           (struct sockaddr *)data->tmp_scratch,
-				                           val,
-				                           true,
-				                           false,
-				                           data->tmp_scratch + sizeof(struct sockaddr_storage));
-			}
+	if(retval != 0 && retval != -EINPROGRESS) {
+		/* Parameter 2: tuple (type: PT_SOCKTUPLE) */
+		res = bpf_push_empty_param(data);
+		CHECK_RES(res);
+
+		/* Parameter 3: fd (type: PT_FD) */
+		return bpf_push_s64_to_ring(data, fd);
+	}
+
+	/* Get the sockaddr pointer and length. */
+	struct sockaddr __user *usrsockaddr =
+	        (struct sockaddr __user *)bpf_syscall_get_argument(data, 1);
+	unsigned long usrsockaddr_len = bpf_syscall_get_argument(data, 2);
+
+	/* Evaluate socktuple, leveraging the user-provided sockaddr if possible */
+	struct sockaddr *ksockaddr = (struct sockaddr *)data->tmp_scratch;
+	bool use_sockaddr_user_data = false;
+	bool push_socktuple = true;
+	if(usrsockaddr != NULL && usrsockaddr_len != 0) {
+		/* Copy the address into kernel memory. */
+		res = bpf_addr_to_kernel(usrsockaddr, usrsockaddr_len, ksockaddr);
+		if(likely(res >= 0)) {
+			/* Convert the fd into socket endpoint information. */
+			use_sockaddr_user_data = true;
+		} else {
+			/* Do not send any socket endpoint information. */
+			push_socktuple = false;
 		}
 	}
 
-	/*
-	 * Copy the endpoint info into the ring
-	 */
+	uint32_t tuple_size = 0;
+	if(push_socktuple) {
+		/* Convert the fd into socket endpoint information */
+		tuple_size = bpf_fd_to_socktuple(data,
+		                                 fd,
+		                                 ksockaddr,
+		                                 usrsockaddr_len,
+		                                 use_sockaddr_user_data,
+		                                 false,
+		                                 data->tmp_scratch + sizeof(struct sockaddr_storage));
+	}
+
+	/* Parameter 2: tuple (type: PT_SOCKTUPLE) */
 	data->curarg_already_on_frame = true;
-	res = bpf_val_to_ring_len(data, 0, size);
+	res = bpf_val_to_ring_len(data, 0, tuple_size);
 	CHECK_RES(res);
 
-	/* Parameter 3: fd (type: PT_FD)*/
-	res = bpf_push_s64_to_ring(data, fd);
-	return res;
+	/* Parameter 3: fd (type: PT_FD) */
+	return bpf_push_s64_to_ring(data, fd);
 }
 
 FILLER(sys_socketpair_x, true) {
@@ -1905,7 +1893,6 @@ FILLER(sys_sendto_e, true) {
 	/* Get the address len */
 	unsigned long usrsockaddr_len = bpf_syscall_get_argument(data, 5);
 
-	/* Evaluate socktuple, leveraging the user-provided sockaddr if possible */
 	struct sockaddr *ksockaddr = (struct sockaddr *)data->tmp_scratch;
 	bool use_sockaddr_user_data = false;
 	bool push_socktuple = true;

driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/connect.bpf.c

@@ -24,11 +24,11 @@ int BPF_PROG(connect_e, struct pt_regs *regs, long id) {
 	unsigned long args[3] = {0};
 	extract__network_args(args, 3, regs);
 
-	/* Parameter 1: fd (type: PT_FD)*/
-	int32_t socket_fd = (int32_t)args[0];
-	auxmap__store_s64_param(auxmap, (int64_t)socket_fd);
+	/* Parameter 1: fd (type: PT_FD) */
+	int64_t socket_fd = (int64_t)(int32_t)args[0];
+	auxmap__store_s64_param(auxmap, socket_fd);
 
-	/* Parameter 2: addr (type: PT_SOCKADDR)*/
+	/* Parameter 2: addr (type: PT_SOCKADDR) */
 	unsigned long sockaddr_ptr = args[1];
 	uint16_t addrlen = (uint16_t)args[2];
 	auxmap__store_sockaddr_param(auxmap, sockaddr_ptr, addrlen);
@@ -57,25 +57,25 @@ int BPF_PROG(connect_x, struct pt_regs *regs, long ret) {
 
 	/*=============================== COLLECT PARAMETERS  ===========================*/
 
-	unsigned long socket_fd = 0;
-	extract__network_args(&socket_fd, 1, regs);
+	unsigned long args[2] = {0};
+	extract__network_args(args, 2, regs);
+	int64_t socket_fd = (int64_t)(int32_t)args[0];
 
 	/* Parameter 1: res (type: PT_ERRNO) */
 	auxmap__store_s64_param(auxmap, ret);
 
 	/* Parameter 2: tuple (type: PT_SOCKTUPLE) */
-	/* We need a valid sockfd to extract source data.*/
 	if(ret == 0 || ret == -EINPROGRESS) {
-		auxmap__store_socktuple_param(auxmap, (int32_t)socket_fd, OUTBOUND, NULL);
+		struct sockaddr *usrsockaddr = (struct sockaddr *)args[1];
+		/* Notice: the following will push an empty parameter if
+		 * something goes wrong (e.g.: fd not valid). */
+		auxmap__store_socktuple_param(auxmap, (int32_t)socket_fd, OUTBOUND, usrsockaddr);
 	} else {
 		auxmap__store_empty_param(auxmap);
 	}
 
-	/* Parameter 3: fd (type: PT_FD)*/
-	/* We need the double cast to extract the first 4 bytes and then
-	 * convert them to a signed integer on 64-bit
-	 */
-	auxmap__store_s64_param(auxmap, (int64_t)(int32_t)socket_fd);
+	/* Parameter 3: fd (type: PT_FD) */
+	auxmap__store_s64_param(auxmap, socket_fd);
 
 	/*=============================== COLLECT PARAMETERS  ===========================*/