Skip to content

Commit 37359ae

Browse files
irozzo-1Airozzo-1A
committed
fix(modern_bpf): resolve dirfd path in kernel space to prevent race conditions
This patch resolves the directory file descriptor path in kernel space at syscall time, preventing race conditions where the dirfd may point to a different directory by the time user space processes the event. The race condition occurs when: 1. openat(dirfd=3, name="hosts") is called with dirfd pointing to /etc 2. Event is captured in kernel space with dirfd=3 3. Process closes dirfd=3 and opens a new file, reusing FD number 3 for a different directory (e.g., /dev) 4. User space resolves dirfd=3 at processing time via /proc/<pid>/fd/3, which now points to /dev instead of /etc 5. Result: /dev/hosts instead of /etc/hosts By resolving the path in kernel space, we capture the actual directory at the moment of the syscall, eliminating the race condition. Changes: - Append new parameter 'dirfdpath' (PT_FSPATH) to existing event versions (PPME_SYSCALL_OPENAT_2_X and PPME_SYSCALL_OPENAT2_X) for backward compatibility with old scap files - Resolve dirfd path in BPF programs using extract__file_struct_from_fd() and auxmap__store_d_path_approx() - For AT_FDCWD, capture CWD from task_struct->fs->pwd in kernel space - Update user space to prefer kernel-resolved path over user-space resolution - Add comprehensive test coverage for both AT_FDCWD and real dirfd cases The new parameter is appended at the end of the parameter list, allowing older libsinsp versions to safely ignore it when processing old scap files with fewer parameters. Related to: falcosecurity/falco#3789
1 parent 0ea6d55 commit 37359ae

File tree

9 files changed

+464
-18
lines changed

9 files changed

+464
-18
lines changed

‎driver/event_table.c‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2083,14 +2083,15 @@ const struct ppm_event_info g_event_info[] = {
20832083
[PPME_SYSCALL_OPENAT_2_X] = {"openat",
20842084
EC_FILE | EC_SYSCALL,
20852085
EF_CREATES_FD | EF_MODIFIES_STATE | EF_CONVERTER_MANAGED,
2086-
7,
2086+
8,
20872087
{{"fd", PT_FD, PF_DEC},
20882088
{"dirfd", PT_FD, PF_DEC},
20892089
{"name", PT_FSRELPATH, PF_NA, DIRFD_PARAM(1)},
20902090
{"flags", PT_FLAGS32, PF_HEX, file_flags},
20912091
{"mode", PT_UINT32, PF_OCT},
20922092
{"dev", PT_UINT32, PF_HEX},
2093-
{"ino", PT_UINT64, PF_DEC}}},
2093+
{"ino", PT_UINT64, PF_DEC},
2094+
{"dirfdpath", PT_FSPATH, PF_NA}}},
20942095
[PPME_SYSCALL_LINK_2_E] = {"link", EC_FILE | EC_SYSCALL, EF_OLD_VERSION, 0},
20952096
[PPME_SYSCALL_LINK_2_X] = {"link",
20962097
EC_FILE | EC_SYSCALL,
@@ -2182,15 +2183,16 @@ const struct ppm_event_info g_event_info[] = {
21822183
[PPME_SYSCALL_OPENAT2_X] = {"openat2",
21832184
EC_FILE | EC_SYSCALL,
21842185
EF_CREATES_FD | EF_MODIFIES_STATE | EF_CONVERTER_MANAGED,
2185-
8,
2186+
9,
21862187
{{"fd", PT_FD, PF_DEC},
21872188
{"dirfd", PT_FD, PF_DEC},
21882189
{"name", PT_FSRELPATH, PF_NA, DIRFD_PARAM(1)},
21892190
{"flags", PT_FLAGS32, PF_HEX, file_flags},
21902191
{"mode", PT_UINT32, PF_OCT},
21912192
{"resolve", PT_FLAGS32, PF_HEX, openat2_flags},
21922193
{"dev", PT_UINT32, PF_HEX},
2193-
{"ino", PT_UINT64, PF_DEC}}},
2194+
{"ino", PT_UINT64, PF_DEC},
2195+
{"dirfdpath", PT_FSPATH, PF_NA}}},
21942196
[PPME_SYSCALL_MPROTECT_E] = {"mprotect",
21952197
EC_MEMORY | EC_SYSCALL,
21962198
EF_OLD_VERSION | EF_CONVERTER_MANAGED,

‎driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/openat.bpf.c‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,49 @@ int BPF_PROG(openat_x, struct pt_regs *regs, long ret) {
6565
/* Parameter 7: ino (type: PT_UINT64) */
6666
auxmap__store_u64_param(auxmap, ino);
6767

68+
/* Parameter 8: dirfdpath (type: PT_FSPATH) - kernel-resolved dirfd path */
69+
if(dirfd == PPM_AT_FDCWD) {
70+
/* For AT_FDCWD, capture the process's current working directory
71+
* in kernel space to prevent race conditions. This captures the
72+
* actual CWD path at syscall time, before the process may exec
73+
* or change directories.
74+
*/
75+
struct task_struct *task = get_current_task();
76+
struct fs_struct *fs = NULL;
77+
struct path cwd_path = {};
78+
79+
BPF_CORE_READ_INTO(&fs, task, fs);
80+
if(fs != NULL) {
81+
BPF_CORE_READ_INTO(&cwd_path, fs, pwd);
82+
auxmap__store_d_path_approx(auxmap, &cwd_path);
83+
} else {
84+
/* If we cannot access fs_struct, store empty and fall back
85+
* to user space resolution.
86+
*/
87+
auxmap__store_empty_param(auxmap);
88+
}
89+
} else if(dirfd >= 0) {
90+
/* Resolve the dirfd path in kernel space to prevent race conditions.
91+
* This captures the actual directory path at syscall time, before
92+
* the process may exec or the FD table may change.
93+
*/
94+
struct file *dir_file = extract__file_struct_from_fd(dirfd);
95+
if(dir_file != NULL) {
96+
struct path dir_path = {};
97+
BPF_CORE_READ_INTO(&dir_path, dir_file, f_path);
98+
auxmap__store_d_path_approx(auxmap, &dir_path);
99+
} else {
100+
/* If we cannot resolve the FD (e.g., it was closed between
101+
* syscall entry and exit), store empty and fall back to
102+
* user space resolution.
103+
*/
104+
auxmap__store_empty_param(auxmap);
105+
}
106+
} else {
107+
/* Invalid FD, store empty */
108+
auxmap__store_empty_param(auxmap);
109+
}
110+
68111
/*=============================== COLLECT PARAMETERS ===========================*/
69112

70113
auxmap__finalize_event_header(auxmap);

‎driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/openat2.bpf.c‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,49 @@ int BPF_PROG(openat2_x, struct pt_regs *regs, long ret) {
7474
/* Parameter 8: ino (type: PT_UINT64) */
7575
auxmap__store_u64_param(auxmap, ino);
7676

77+
/* Parameter 9: dirfdpath (type: PT_FSPATH) - kernel-resolved dirfd path */
78+
if(dirfd == PPM_AT_FDCWD) {
79+
/* For AT_FDCWD, capture the process's current working directory
80+
* in kernel space to prevent race conditions. This captures the
81+
* actual CWD path at syscall time, before the process may exec
82+
* or change directories.
83+
*/
84+
struct task_struct *task = get_current_task();
85+
struct fs_struct *fs = NULL;
86+
struct path cwd_path = {};
87+
88+
BPF_CORE_READ_INTO(&fs, task, fs);
89+
if(fs != NULL) {
90+
BPF_CORE_READ_INTO(&cwd_path, fs, pwd);
91+
auxmap__store_d_path_approx(auxmap, &cwd_path);
92+
} else {
93+
/* If we cannot access fs_struct, store empty and fall back
94+
* to user space resolution.
95+
*/
96+
auxmap__store_empty_param(auxmap);
97+
}
98+
} else if(dirfd >= 0) {
99+
/* Resolve the dirfd path in kernel space to prevent race conditions.
100+
* This captures the actual directory path at syscall time, before
101+
* the process may exec or the FD table may change.
102+
*/
103+
struct file *dir_file = extract__file_struct_from_fd(dirfd);
104+
if(dir_file != NULL) {
105+
struct path dir_path = {};
106+
BPF_CORE_READ_INTO(&dir_path, dir_file, f_path);
107+
auxmap__store_d_path_approx(auxmap, &dir_path);
108+
} else {
109+
/* If we cannot resolve the FD (e.g., it was closed between
110+
* syscall entry and exit), store empty and fall back to
111+
* user space resolution.
112+
*/
113+
auxmap__store_empty_param(auxmap);
114+
}
115+
} else {
116+
/* Invalid FD, store empty */
117+
auxmap__store_empty_param(auxmap);
118+
}
119+
77120
/*=============================== COLLECT PARAMETERS ===========================*/
78121

79122
auxmap__finalize_event_header(auxmap);

‎test/drivers/event_class/event_class.cpp‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
#include <time.h>
44
#include <sys/vfs.h> /* or <sys/statfs.h> */
55
#include <linux/magic.h>
6+
#include <filesystem>
67

78
#define MAX_CHARBUF_NUM 16
89
#define CGROUP_NUMBER 5
@@ -1157,6 +1158,39 @@ void event_test::assert_charbuf_param_any_of(const int param_num,
11571158
FAIL() << oss.str();
11581159
}
11591160

1161+
std::string event_test::get_charbuf_param(int param_num) {
1162+
assert_param_boundaries(param_num);
1163+
const auto& [param_value, param_size] = m_event_params[m_current_param];
1164+
1165+
if(param_size == 0) {
1166+
return std::string();
1167+
}
1168+
1169+
// Handle null termination
1170+
size_t str_len =
1171+
(param_size > 0 && param_value[param_size - 1] == '\0') ? param_size - 1 : param_size;
1172+
1173+
return std::string(param_value, str_len);
1174+
}
1175+
1176+
void event_test::assert_dirfdpath_matches_cwd(int param_num, const char* expected_cwd) {
1177+
std::string captured_cwd = get_charbuf_param(param_num);
1178+
ASSERT_FALSE(captured_cwd.empty()) << "dirfdpath (parameter " << param_num
1179+
<< ") should contain the CWD path, but it's empty";
1180+
1181+
/* Use std::filesystem::path for proper path normalization and comparison */
1182+
std::filesystem::path expected_path(expected_cwd);
1183+
std::filesystem::path captured_path(captured_cwd);
1184+
1185+
/* Normalize paths (remove trailing slashes, resolve . and .., etc.) */
1186+
expected_path = expected_path.lexically_normal();
1187+
captured_path = captured_path.lexically_normal();
1188+
1189+
ASSERT_TRUE(captured_path == expected_path)
1190+
<< "Captured CWD path '" << captured_path.string() << "' should match expected CWD '"
1191+
<< expected_path.string() << "'";
1192+
}
1193+
11601194
void event_test::assert_charbuf_array_param(int param_num, const char** param) {
11611195
assert_param_boundaries(param_num);
11621196
uint16_t total_len = 0;

‎test/drivers/event_class/event_class.h‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,16 @@ enum direction {
9898
/*
9999
* Macro that wraps internal _assert_syscall_state,
100100
* dealing with ENOSYS syscalls, ie: syscalls that are defined but unimplemented,
101+
* and EOPNOTSUPP when the syscall exists but the operation is not supported,
101102
* skipping the test.
102103
*/
103-
#define assert_syscall_state(syscall_state, syscall_name, ...) \
104-
do { \
105-
_assert_syscall_state(syscall_state, syscall_name, __VA_ARGS__); \
106-
if(errno == ENOSYS) \
107-
GTEST_SKIP() << "Syscall " << syscall_name << " not implemented" << std::endl; \
104+
#define assert_syscall_state(syscall_state, syscall_name, ...) \
105+
do { \
106+
_assert_syscall_state(syscall_state, syscall_name, __VA_ARGS__); \
107+
if(errno == ENOSYS) \
108+
GTEST_SKIP() << "Syscall " << syscall_name << " not implemented" << std::endl; \
109+
if(errno == EOPNOTSUPP || errno == 95) \
110+
GTEST_SKIP() << "Syscall " << syscall_name << " operation not supported" << std::endl; \
108111
} while(0)
109112

110113
/////////////////////////////////
@@ -569,6 +572,22 @@ class event_test {
569572

570573
void assert_charbuf_param_any_of(int param_num, const std::vector<const char*>& candidates);
571574

575+
/**
576+
* @brief Get the value of a charbuf parameter as a string
577+
*
578+
* @param param_num number of the parameter to get
579+
* @return std::string the parameter value
580+
*/
581+
std::string get_charbuf_param(int param_num);
582+
583+
/**
584+
* @brief Assert that a dirfdpath parameter matches the expected CWD path
585+
*
586+
* @param param_num number of the dirfdpath parameter to verify
587+
* @param expected_cwd the expected current working directory path
588+
*/
589+
void assert_dirfdpath_matches_cwd(int param_num, const char* expected_cwd);
590+
572591
/**
573592
* @brief Assert that the parameter is a `charbuf` array and
574593
* compare element per element the array with the one passed as a parameter `param`.

‎test/drivers/flags/flags_definitions.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33
#include <sys/syscall.h>
44

55
/* capabilities */
6-
#if defined(__NR_capget) && defined(__NR_capset)
6+
#if defined(__NR_capget) && defined(__NR_capset) && defined(__linux__) && \
7+
__has_include(<sys/capability.h>)
78

89
#include <sys/capability.h>
910
uint64_t capabilities_to_scap(unsigned long caps);
1011

11-
#endif /* __NR_capget */
12+
#endif /* __NR_capget && __NR_capset && __linux__ && __has_include(<sys/capability.h>) */

0 commit comments

Comments
 (0)