fix umount2 syscall flags type, add conversion helper function

- change the flags (param 1) from u32 to s32 - add a userspace to scap flag conversion helper routine Reported by: github issue #515 Signed-off-by: Ofer Heifetz <oheifetz@gmail.com>
falcosecurity · Jul 20, 2023 · 26481c7 · 26481c7
1 parent dca4292
commit 26481c7
Show file tree

Hide file tree

Showing 15 changed files with 60 additions and 18 deletions.
diff --git a/driver/bpf/fillers.h b/driver/bpf/fillers.h
@@ -6213,8 +6213,8 @@ FILLER(sys_umount_x, true)
 FILLER(sys_umount2_e, true)
 {
 	/* Parameter 1: flags (type: PT_FLAGS32) */
-	u32 flags = (u32)bpf_syscall_get_argument(data, 1);
-	return bpf_push_u32_to_ring(data, flags);
+	int flags = (int)bpf_syscall_get_argument(data, 1);
+	return bpf_push_s32_to_ring(data, flags);
 }
 
 FILLER(sys_umount2_x, true)

diff --git a/driver/event_table.c b/driver/event_table.c
@@ -439,8 +439,10 @@ const struct ppm_event_info g_event_info[] = {
 	[PPME_SYSCALL_UMOUNT_1_X] = {"umount", EC_FILE | EC_SYSCALL, EF_MODIFIES_STATE, 2, {{"res", PT_ERRNO, PF_DEC}, {"name", PT_FSPATH, PF_NA} } },
 	[PPME_SOCKET_ACCEPT4_6_E] = {"accept4", EC_NET | EC_SYSCALL, EF_CREATES_FD | EF_MODIFIES_STATE, 1, {{"flags", PT_INT32, PF_HEX} } },
 	[PPME_SOCKET_ACCEPT4_6_X] = {"accept4", EC_NET | EC_SYSCALL, EF_CREATES_FD | EF_MODIFIES_STATE, 5, {{"fd", PT_FD, PF_DEC}, {"tuple", PT_SOCKTUPLE, PF_NA}, {"queuepct", PT_UINT8, PF_DEC}, {"queuelen", PT_UINT32, PF_DEC}, {"queuemax", PT_UINT32, PF_DEC} } },
-	[PPME_SYSCALL_UMOUNT2_E] = {"umount2", EC_FILE | EC_SYSCALL, EF_MODIFIES_STATE, 1, {{"flags", PT_FLAGS32, PF_HEX, umount_flags} } },
-	[PPME_SYSCALL_UMOUNT2_X] = {"umount2", EC_FILE | EC_SYSCALL, EF_MODIFIES_STATE, 2, {{"res", PT_ERRNO, PF_DEC}, {"name", PT_FSPATH, PF_NA} } },
+	[PPME_SYSCALL_UMOUNT2_E] = {"umount2", EC_FILE | EC_SYSCALL, EF_MODIFIES_STATE | EF_OLD_VERSION, 1, {{"flags", PT_FLAGS32, PF_HEX, umount_flags} } },
+	[PPME_SYSCALL_UMOUNT2_X] = {"umount2", EC_FILE | EC_SYSCALL, EF_MODIFIES_STATE | EF_OLD_VERSION, 2, {{"res", PT_ERRNO, PF_DEC}, {"name", PT_FSPATH, PF_NA} } },
+	[PPME_SYSCALL_UMOUNT2_1_E] = {"umount2", EC_FILE | EC_SYSCALL, EF_MODIFIES_STATE, 1, {{"flags", PT_FLAGS32, PF_HEX, umount_flags} } },
+	[PPME_SYSCALL_UMOUNT2_1_X] = {"umount2", EC_FILE | EC_SYSCALL, EF_MODIFIES_STATE, 2, {{"res", PT_ERRNO, PF_DEC}, {"name", PT_FSPATH, PF_NA} } },
 	[PPME_SYSCALL_PIPE2_E] = {"pipe2", EC_IPC | EC_SYSCALL, EF_CREATES_FD | EF_MODIFIES_STATE, 0},
 	[PPME_SYSCALL_PIPE2_X] = {"pipe2", EC_IPC | EC_SYSCALL, EF_CREATES_FD | EF_MODIFIES_STATE, 5, {{"res", PT_ERRNO, PF_DEC}, {"fd1", PT_FD, PF_DEC}, {"fd2", PT_FD, PF_DEC}, {"ino", PT_UINT64, PF_DEC}, {"flags", PT_FLAGS32, PF_HEX, file_flags}} },
 	[PPME_SYSCALL_INOTIFY_INIT1_E] = {"inotify_init1", EC_IPC | EC_SYSCALL, EF_CREATES_FD | EF_MODIFIES_STATE, 0},

diff --git a/driver/fillers_table.c b/driver/fillers_table.c
@@ -326,8 +326,8 @@ const struct ppm_event_entry g_ppm_events[PPM_EVENT_MAX] = {
 	[PPME_SYSCALL_UMOUNT_1_X] = {FILLER_REF(sys_umount_x)},
 	[PPME_SOCKET_ACCEPT4_6_E] = {FILLER_REF(sys_accept4_e)},
 	[PPME_SOCKET_ACCEPT4_6_X] = {FILLER_REF(sys_accept_x)},
-	[PPME_SYSCALL_UMOUNT2_E] = {FILLER_REF(sys_umount2_e)},
-	[PPME_SYSCALL_UMOUNT2_X] = {FILLER_REF(sys_umount2_x)},
+	[PPME_SYSCALL_UMOUNT2_1_E] = {FILLER_REF(sys_umount2_e)},
+	[PPME_SYSCALL_UMOUNT2_1_X] = {FILLER_REF(sys_umount2_x)},
 	[PPME_SYSCALL_PIPE2_E] = {FILLER_REF(sys_empty)},
 	[PPME_SYSCALL_PIPE2_X] = {FILLER_REF(sys_pipe2_x)},
 	[PPME_SYSCALL_INOTIFY_INIT1_E] = {FILLER_REF(sys_empty)},

diff --git a/driver/main.c b/driver/main.c
@@ -1476,6 +1476,7 @@ static inline void drops_buffer_syscall_categories_counters(ppm_event_code event
 	case PPME_SYSCALL_UMOUNT_E:
 	case PPME_SYSCALL_UMOUNT_1_E:
 	case PPME_SYSCALL_UMOUNT2_E:
+	case PPME_SYSCALL_UMOUNT2_1_E:
 	case PPME_SYSCALL_RENAME_E:
 	case PPME_SYSCALL_RENAMEAT_E:
 	case PPME_SYSCALL_RENAMEAT2_E:
@@ -1548,6 +1549,7 @@ static inline void drops_buffer_syscall_categories_counters(ppm_event_code event
 	case PPME_SYSCALL_UMOUNT_X:
 	case PPME_SYSCALL_UMOUNT_1_X:
 	case PPME_SYSCALL_UMOUNT2_X:
+	case PPME_SYSCALL_UMOUNT2_1_X:
 	case PPME_SYSCALL_RENAME_X:
 	case PPME_SYSCALL_RENAMEAT_X:
 	case PPME_SYSCALL_RENAMEAT2_X:

diff --git a/driver/modern_bpf/definitions/events_dimensions.h b/driver/modern_bpf/definitions/events_dimensions.h
@@ -113,7 +113,7 @@
 #define UNSHARE_E_SIZE HEADER_LEN + sizeof(uint32_t) + PARAM_LEN
 #define UNSHARE_X_SIZE HEADER_LEN + sizeof(int64_t) + PARAM_LEN
 #define MOUNT_E_SIZE HEADER_LEN + sizeof(uint32_t) + PARAM_LEN
-#define UMOUNT2_E_SIZE HEADER_LEN + sizeof(uint32_t) + PARAM_LEN
+#define UMOUNT2_E_SIZE HEADER_LEN + sizeof(int32_t) + PARAM_LEN
 #define UMOUNT_E_SIZE HEADER_LEN
 #define LINK_E_SIZE HEADER_LEN
 #define LINKAT_E_SIZE HEADER_LEN

diff --git a/driver/modern_bpf/helpers/base/stats.h b/driver/modern_bpf/helpers/base/stats.h
@@ -51,6 +51,7 @@ static __always_inline void compute_event_types_stats(u16 event_type, struct cou
 	case PPME_SYSCALL_UMOUNT_E:
 	case PPME_SYSCALL_UMOUNT_1_E:
 	case PPME_SYSCALL_UMOUNT2_E:
+	case PPME_SYSCALL_UMOUNT2_1_E:
 	case PPME_SYSCALL_RENAME_E:
 	case PPME_SYSCALL_RENAMEAT_E:
 	case PPME_SYSCALL_RENAMEAT2_E:
@@ -123,6 +124,7 @@ static __always_inline void compute_event_types_stats(u16 event_type, struct cou
 	case PPME_SYSCALL_UMOUNT_X:
 	case PPME_SYSCALL_UMOUNT_1_X:
 	case PPME_SYSCALL_UMOUNT2_X:
+	case PPME_SYSCALL_UMOUNT2_1_X:
 	case PPME_SYSCALL_RENAME_X:
 	case PPME_SYSCALL_RENAMEAT_X:
 	case PPME_SYSCALL_RENAMEAT2_X:

diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/umount2.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/umount2.bpf.c
@@ -16,7 +16,7 @@ int BPF_PROG(umount2_e,
 	     long id)
 {
 	struct ringbuf_struct ringbuf;
-	if(!ringbuf__reserve_space(&ringbuf, ctx, UMOUNT2_E_SIZE, PPME_SYSCALL_UMOUNT2_E))
+	if(!ringbuf__reserve_space(&ringbuf, ctx, UMOUNT2_E_SIZE, PPME_SYSCALL_UMOUNT2_1_E))
 	{
 		return 0;
 	}
@@ -26,8 +26,8 @@ int BPF_PROG(umount2_e,
 	/*=============================== COLLECT PARAMETERS  ===========================*/
 
 	/* Parameter 1: flags (type: PT_FLAGS32) */
-	u32 flags = (u32)extract__syscall_argument(regs, 1);
-	ringbuf__store_u32(&ringbuf, flags);
+	s32 flags = (s32)extract__syscall_argument(regs, 1);
+	ringbuf__store_s32(&ringbuf, flags);
 
 	/*=============================== COLLECT PARAMETERS  ===========================*/
 
@@ -51,7 +51,7 @@ int BPF_PROG(umount2_x,
 		return 0;
 	}
 
-	auxmap__preload_event_header(auxmap, PPME_SYSCALL_UMOUNT2_X);
+	auxmap__preload_event_header(auxmap, PPME_SYSCALL_UMOUNT2_1_X);
 
 	/*=============================== COLLECT PARAMETERS  ===========================*/
 

diff --git a/driver/ppm_events_public.h b/driver/ppm_events_public.h
@@ -1383,7 +1383,9 @@ typedef enum {
 	PPME_SYSCALL_PIDFD_GETFD_X = 407,
 	PPME_SYSCALL_PIDFD_OPEN_E = 408,
 	PPME_SYSCALL_PIDFD_OPEN_X = 409,
-	PPM_EVENT_MAX = 410
+	PPME_SYSCALL_UMOUNT2_1_E = 410,
+	PPME_SYSCALL_UMOUNT2_1_X = 411,
+	PPM_EVENT_MAX = 412
 } ppm_event_code;
 /*@}*/
 

diff --git a/driver/ppm_flag_helpers.h b/driver/ppm_flag_helpers.h
@@ -32,6 +32,9 @@ or GPL2.txt for full copies of the license.
 #ifdef __NR_io_uring_register
 #include <uapi/linux/io_uring.h>
 #endif
+#ifdef __NR_umount2
+#include <linux/fs.h>
+#endif
 #endif // ifndef UDIG
 
 #define PPM_MS_MGC_MSK 0xffff0000
@@ -1822,6 +1825,31 @@ static __always_inline u32 chmod_mode_to_scap(unsigned long modes)
 	return res;
 }
 
+static __always_inline u32 umount2_flags_to_scap(unsigned long flags)
+{
+	u32 res = 0;
+
+#ifdef __NR_umount2
+#ifdef MNT_FORCE
+	if (flags & MNT_FORCE)
+		res |= PPM_MNT_FORCE;
+#endif
+#ifdef MNT_DETACH
+	if (flags & MNT_DETACH)
+		res |= PPM_MNT_DETACH;
+#endif
+#ifdef MNT_EXPIRE
+	if (flags & MNT_EXPIRE)
+		res |= PPM_MNT_EXPIRE;
+#endif
+#ifdef UMOUNT_NOFOLLOW
+	if (flags & UMOUNT_NOFOLLOW)
+		res |= PPM_UMOUNT_NOFOLLOW;
+#endif
+#endif
+	return res;
+}
+
 static __always_inline u32 fchownat_flags_to_scap(unsigned long flags)
 {
 	u32 res = 0;

diff --git a/driver/syscall_table.c b/driver/syscall_table.c
@@ -141,7 +141,7 @@ const struct syscall_evt_pair g_syscall_table[SYSCALL_TABLE_SIZE] = {
 #endif
 	[__NR_mount - SYSCALL_TABLE_ID0] =                      {UF_USED, PPME_SYSCALL_MOUNT_E, PPME_SYSCALL_MOUNT_X, PPM_SC_MOUNT},
 #ifdef __NR_umount2
-	[__NR_umount2 - SYSCALL_TABLE_ID0] =                    {UF_USED, PPME_SYSCALL_UMOUNT2_E, PPME_SYSCALL_UMOUNT2_X, PPM_SC_UMOUNT2},
+	[__NR_umount2 - SYSCALL_TABLE_ID0] =                    {UF_USED, PPME_SYSCALL_UMOUNT2_1_E, PPME_SYSCALL_UMOUNT2_1_X, PPM_SC_UMOUNT2},
 #endif
 	[__NR_ptrace - SYSCALL_TABLE_ID0] =                     {UF_USED, PPME_SYSCALL_PTRACE_E, PPME_SYSCALL_PTRACE_X, PPM_SC_PTRACE},
 #ifdef __NR_socket

diff --git a/test/drivers/test_suites/syscall_enter_suite/umount2_e.cpp b/test/drivers/test_suites/syscall_enter_suite/umount2_e.cpp
@@ -34,7 +34,7 @@ TEST(SyscallEnter, umount2E)
 	/*=============================== ASSERT PARAMETERS  ===========================*/
 
 	/* Parameter 1: flags (type: PT_FLAGS32) */
-	evt_test->assert_numeric_param(1, (uint32_t)(PPM_MNT_FORCE | PPM_MNT_DETACH | PPM_MNT_EXPIRE | PPM_UMOUNT_NOFOLLOW));
+	evt_test->assert_numeric_param(1, (int32_t)(PPM_MNT_FORCE | PPM_MNT_DETACH | PPM_MNT_EXPIRE | PPM_UMOUNT_NOFOLLOW));
 
 	/*=============================== ASSERT PARAMETERS  ===========================*/
 

diff --git a/userspace/libpman/src/events_prog_names.h b/userspace/libpman/src/events_prog_names.h
@@ -150,8 +150,8 @@ static const char* event_prog_names[PPM_EVENT_MAX] = {
 	[PPME_SYSCALL_UNSHARE_X] = "unshare_x",
 	[PPME_SYSCALL_MOUNT_E] = "mount_e",
 	[PPME_SYSCALL_MOUNT_X] = "mount_x",
-	[PPME_SYSCALL_UMOUNT2_E] = "umount2_e",
-	[PPME_SYSCALL_UMOUNT2_X] = "umount2_x",
+	[PPME_SYSCALL_UMOUNT2_1_E] = "umount2_e",
+	[PPME_SYSCALL_UMOUNT2_1_X] = "umount2_x",
 	[PPME_SYSCALL_LINK_2_E] = "link_e",
 	[PPME_SYSCALL_LINK_2_X] = "link_x",
 	[PPME_SYSCALL_LINKAT_2_E] = "linkat_e",

diff --git a/userspace/libscap/linux/scap_ppm_sc.c b/userspace/libscap/linux/scap_ppm_sc.c
@@ -423,6 +423,8 @@ static const ppm_sc_code *g_events_to_sc_map[] = {
 	[PPME_SOCKET_ACCEPT4_6_X] = (ppm_sc_code[]){PPM_SC_ACCEPT4, -1},
 	[PPME_SYSCALL_UMOUNT2_E] = (ppm_sc_code[]){PPM_SC_UMOUNT2, -1},
 	[PPME_SYSCALL_UMOUNT2_X] = (ppm_sc_code[]){PPM_SC_UMOUNT2, -1},
+	[PPME_SYSCALL_UMOUNT2_1_E] = (ppm_sc_code[]){PPM_SC_UMOUNT2, -1},
+	[PPME_SYSCALL_UMOUNT2_1_X] = (ppm_sc_code[]){PPM_SC_UMOUNT2, -1},
 	[PPME_SYSCALL_PIPE2_E] = (ppm_sc_code[]){PPM_SC_PIPE2, -1},
 	[PPME_SYSCALL_PIPE2_X] = (ppm_sc_code[]){PPM_SC_PIPE2, -1},
 	[PPME_SYSCALL_INOTIFY_INIT1_E] = (ppm_sc_code[]){PPM_SC_INOTIFY_INIT1, -1},

diff --git a/userspace/libsinsp/test/events_file.ut.cpp b/userspace/libsinsp/test/events_file.ut.cpp
@@ -293,8 +293,8 @@ TEST_F(sinsp_with_test_input, umount2)
 	int64_t res = 0;
 	const char* name = "/target_name";
 
-	add_event_advance_ts(increasing_ts(), 1, PPME_SYSCALL_UMOUNT2_E, 1, flags);
-	evt = add_event_advance_ts(increasing_ts(), 1, PPME_SYSCALL_UMOUNT2_X, 2, res, name);
+	add_event_advance_ts(increasing_ts(), 1, PPME_SYSCALL_UMOUNT2_1_E, 1, flags);
+	evt = add_event_advance_ts(increasing_ts(), 1, PPME_SYSCALL_UMOUNT2_1_X, 2, res, name);
 	ASSERT_EQ(get_field_as_string(evt, "evt.type"), "umount2");
 	ASSERT_EQ(get_field_as_string(evt, "evt.category"), "file");
 	ASSERT_EQ(get_field_as_string(evt, "evt.arg.res"), std::to_string(res));

diff --git a/userspace/libsinsp/test/public_sinsp_API/ppm_sc_codes.cpp b/userspace/libsinsp/test/public_sinsp_API/ppm_sc_codes.cpp
@@ -155,6 +155,8 @@ const libsinsp::events::set<ppm_event_code> expected_sinsp_state_event_set = {
 	PPME_SYSCALL_TIMERFD_CREATE_X,
 	PPME_SYSCALL_UMOUNT_E,
 	PPME_SYSCALL_UMOUNT_X,
+	PPME_SYSCALL_UMOUNT_1_E,
+	PPME_SYSCALL_UMOUNT_1_X,
 	PPME_SYSCALL_USERFAULTFD_E,
 	PPME_SYSCALL_USERFAULTFD_X,
 	PPME_SYSCALL_VFORK_E,
@@ -194,6 +196,8 @@ const libsinsp::events::set<ppm_event_code> expected_sinsp_state_event_set = {
 	PPME_SOCKET_ACCEPT4_6_X,
 	PPME_SYSCALL_UMOUNT2_E,
 	PPME_SYSCALL_UMOUNT2_X,
+	PPME_SYSCALL_UMOUNT2_1_E,
+	PPME_SYSCALL_UMOUNT2_1_X,
 	PPME_SYSCALL_PIPE2_E,
 	PPME_SYSCALL_PIPE2_X,
 	PPME_SYSCALL_INOTIFY_INIT1_E,