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

fix(driver/kmod): don't truncate path at 256 #1954

Merged
merged 6 commits into from
Jul 17, 2024
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
2 changes: 1 addition & 1 deletion driver/SCHEMA_VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.21.0
2.21.1
1 change: 0 additions & 1 deletion driver/ppm_events_public.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ or GPL2.txt for full copies of the license.
* Limits
*/
#define PPM_MAX_EVENT_PARAMS (1 << 5) /* Max number of parameters an event can have */
#define PPM_MAX_PATH_SIZE 256 /* Max size that an event parameter can have in the circular buffer, in bytes */
#define PPM_MAX_NAME_LEN 32

/*
Expand Down
32 changes: 4 additions & 28 deletions driver/ppm_fillers.c
Original file line number Diff line number Diff line change
Expand Up @@ -289,19 +289,13 @@ int f_sys_open_e(struct event_filler_arguments *args)
unsigned long val;
unsigned long flags;
unsigned long modes;
char *name = NULL;
int res;

/*
* name
*/
syscall_get_arguments_deprecated(args, 0, 1, &val);
if(likely(ppm_strncpy_from_user(args->str_storage, (const void __user *)val, PPM_MAX_PATH_SIZE) >= 0))
{
name = args->str_storage;
name[PPM_MAX_PATH_SIZE - 1] = '\0';
}
res = val_to_ring(args, (int64_t)(long)name, 0, false, 0);
res = val_to_ring(args, val, 0, true, 0);
CHECK_RES(res);

/*
Expand Down Expand Up @@ -2947,19 +2941,13 @@ int f_sys_creat_e(struct event_filler_arguments *args)
{
unsigned long val;
unsigned long modes;
char *name = NULL;
int res;

/*
* name
*/
syscall_get_arguments_deprecated(args, 0, 1, &val);
if(likely(ppm_strncpy_from_user(args->str_storage, (const void __user *)val, PPM_MAX_PATH_SIZE) >= 0))
{
name = args->str_storage;
name[PPM_MAX_PATH_SIZE - 1] = '\0';
}
res = val_to_ring(args, (int64_t)(long)name, 0, false, 0);
res = val_to_ring(args, val, 0, true, 0);
CHECK_RES(res);

/*
Expand Down Expand Up @@ -3500,7 +3488,6 @@ int f_sys_openat_e(struct event_filler_arguments *args)
unsigned long flags;
unsigned long modes;
int32_t fd;
char *name = NULL;
int res;

/*
Expand All @@ -3518,12 +3505,7 @@ int f_sys_openat_e(struct event_filler_arguments *args)
* name
*/
syscall_get_arguments_deprecated(args, 1, 1, &val);
if(likely(ppm_strncpy_from_user(args->str_storage, (const void __user *)val, PPM_MAX_PATH_SIZE) >= 0))
{
name = args->str_storage;
name[PPM_MAX_PATH_SIZE - 1] = '\0';
}
res = val_to_ring(args, (int64_t)(long)name, 0, false, 0);
res = val_to_ring(args, val, 0, true, 0);
CHECK_RES(res);
/*
* Flags
Expand Down Expand Up @@ -4900,7 +4882,6 @@ int f_sys_openat2_e(struct event_filler_arguments *args)
unsigned long flags;
unsigned long val;
unsigned long mode;
char *name = NULL;
int32_t fd;
int res;
#ifdef __NR_openat2
Expand All @@ -4922,12 +4903,7 @@ int f_sys_openat2_e(struct event_filler_arguments *args)
* name
*/
syscall_get_arguments_deprecated(args, 1, 1, &val);
if(likely(ppm_strncpy_from_user(args->str_storage, (const void __user *)val, PPM_MAX_PATH_SIZE) >= 0))
{
name = args->str_storage;
name[PPM_MAX_PATH_SIZE - 1] = '\0';
}
res = val_to_ring(args, (int64_t)(long)name, 0, false, 0);
res = val_to_ring(args, val, 0, true, 0);
CHECK_RES(res);


Expand Down
1 change: 1 addition & 0 deletions test/drivers/event_class/event_class.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

#define CURRENT_PID -1
#define CURRENT_EVENT_TYPE -1
#define PPM_MAX_PATH_SIZE 1024

extern "C"
{
Expand Down
41 changes: 41 additions & 0 deletions test/drivers/test_suites/syscall_enter_suite/creat_e.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,4 +40,45 @@ TEST(SyscallEnter, creatE)

evt_test->assert_num_params_pushed(2);
}

TEST(SyscallEnter, creatE_max_path)
{
auto evt_test = get_syscall_event_test(__NR_creat, ENTER_EVENT);

evt_test->enable_capture();

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

std::string path("");
path.insert(0, PPM_MAX_PATH_SIZE - 1, 'A');
mode_t mode = S_IRGRP;
assert_syscall_state(SYSCALL_FAILURE, "creat", syscall(__NR_creat, path.c_str(), mode));

/*=============================== 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 1: name (type: PT_FSPATH) */
evt_test->assert_charbuf_param(1, path.c_str());

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

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

evt_test->assert_num_params_pushed(2);
}
#endif
49 changes: 49 additions & 0 deletions test/drivers/test_suites/syscall_enter_suite/open_e.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,53 @@ TEST(SyscallEnter, openE)

evt_test->assert_num_params_pushed(3);
}

TEST(SyscallEnter, openE_max_path)
{
auto evt_test = get_syscall_event_test(__NR_open, ENTER_EVENT);

evt_test->enable_capture();

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

/* Syscall special notes:
* With `O_TMPFILE` flag the pathname must be a directory
* but here it is a filename so the call will fail!
*/
std::string pathname("");
pathname.insert(0, PPM_MAX_PATH_SIZE - 1, 'A');
int flags = O_RDWR | O_TMPFILE | O_DIRECTORY;
mode_t mode = 0;
assert_syscall_state(SYSCALL_FAILURE, "open", syscall(__NR_open, pathname.c_str(), flags, mode));

/*=============================== 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 1: name (type: PT_FSPATH) */
evt_test->assert_charbuf_param(1, pathname.c_str());

/* Parameter 2: flags (type: PT_FLAGS32) */
evt_test->assert_numeric_param(2, (uint32_t)PPM_O_RDWR | PPM_O_TMPFILE | PPM_O_DIRECTORY);

/* Parameter 3: mode (type: PT_UINT32) */
evt_test->assert_numeric_param(3, (uint32_t)mode);

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

evt_test->assert_num_params_pushed(3);
}
#endif
59 changes: 59 additions & 0 deletions test/drivers/test_suites/syscall_enter_suite/openat2_e.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,63 @@ TEST(SyscallEnter, openat2E)

evt_test->assert_num_params_pushed(5);
}

TEST(SyscallEnter, openat2E_max_path)
{
auto evt_test = get_syscall_event_test(__NR_openat2, ENTER_EVENT);

evt_test->enable_capture();

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

/* Syscall special notes:
* With `O_TMPFILE` flag the pathname must be a directory
* but here it is a filename so the call will fail!
*/

int dirfd = AT_FDCWD;
std::string pathname("");
pathname.insert(0, PPM_MAX_PATH_SIZE - 1, 'A');
struct open_how how;
how.flags = O_RDWR | O_TMPFILE | O_DIRECTORY;
how.mode = 0;
how.resolve = RESOLVE_BENEATH | RESOLVE_NO_MAGICLINKS;
assert_syscall_state(SYSCALL_FAILURE, "openat2", syscall(__NR_openat2, dirfd, pathname.c_str(), &how, sizeof(struct open_how)));

/*=============================== 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 1: dirfd (type: PT_FD) */
evt_test->assert_numeric_param(1, (int64_t)PPM_AT_FDCWD);

/* Parameter 2: name (type: PT_FSPATH) */
evt_test->assert_charbuf_param(2, pathname.c_str());

/* Parameter 3: flags (type: PT_FLAGS32) */
evt_test->assert_numeric_param(3, (uint32_t)PPM_O_RDWR | PPM_O_TMPFILE | PPM_O_DIRECTORY);

/* Parameter 4: mode (type: PT_UINT32) */
evt_test->assert_numeric_param(4, (uint32_t)how.mode);

/* Parameter 5: resolve (type: PT_FLAGS32) */
evt_test->assert_numeric_param(5, (uint32_t)PPM_RESOLVE_BENEATH | PPM_RESOLVE_NO_MAGICLINKS);

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

evt_test->assert_num_params_pushed(5);
}
#endif
54 changes: 54 additions & 0 deletions test/drivers/test_suites/syscall_enter_suite/openat_e.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,4 +53,58 @@ TEST(SyscallEnter, openatE)

evt_test->assert_num_params_pushed(4);
}

TEST(SyscallEnter, openatE_max_path)
{
auto evt_test = get_syscall_event_test(__NR_openat, ENTER_EVENT);

evt_test->enable_capture();

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

/* Syscall special notes:
* With `O_TMPFILE` flag the pathname must be a directory
* but here it is a filename so the call will fail!
*/

int dirfd = AT_FDCWD;
std::string pathname("");
pathname.insert(0, PPM_MAX_PATH_SIZE - 1, 'A');
int flags = O_RDWR | O_TMPFILE | O_DIRECTORY;
mode_t mode = 0;
assert_syscall_state(SYSCALL_FAILURE, "openat", syscall(__NR_openat, dirfd, pathname.c_str(), flags, mode));

/*=============================== 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 1: dirfd (type: PT_FD) */
evt_test->assert_numeric_param(1, (int64_t)PPM_AT_FDCWD);

/* Parameter 2: name (type: PT_FSPATH) */
evt_test->assert_charbuf_param(2, pathname.c_str());

/* Parameter 3: flags (type: PT_FLAGS32) */
evt_test->assert_numeric_param(3, (uint32_t)PPM_O_RDWR | PPM_O_TMPFILE | PPM_O_DIRECTORY);

/* Parameter 4: mode (type: PT_UINT32) */
evt_test->assert_numeric_param(4, (uint32_t)mode);

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

evt_test->assert_num_params_pushed(4);
}
#endif
38 changes: 38 additions & 0 deletions test/libsinsp_e2e/sys_call_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2509,3 +2509,41 @@ TEST_F(sys_call_test, thread_lookup_live)
EXPECT_EQ(0, scap_tinfo.ptid);

}

TEST_F(sys_call_test, fd_name_max_path)
{
int callnum = 0;
std::string pathname("/");
// Using only 1022 chars otherwise the path will be "/PATH_TOO_LONG".
pathname.insert(1, 1021, 'A');

event_filter_t filter = [&](sinsp_evt* evt)
{
return (0 == strcmp(evt->get_name(), "open") || 0 == strcmp(evt->get_name(), "openat"))
&& m_tid_filter(evt);
};

run_callback_t test = [&](concurrent_object_handle<sinsp> inspector_handle)
{
open(pathname.c_str(), O_RDONLY);
};

sinsp_filter_check_list m_filterlist;

captured_event_callback_t callback = [&](const callback_param& param)
{
if((0 == strcmp(param.m_evt->get_name(), "open")) ||
(0 == strcmp(param.m_evt->get_name(), "openat")))
{
std::string output;
sinsp_evt_formatter(param.m_inspector, "*%fd.name", m_filterlist).tostring(param.m_evt, &output);
if(pathname == output)
{
callnum++;
}
}
};

ASSERT_NO_FATAL_FAILURE({ event_capture::run(test, callback, filter); });
EXPECT_EQ(1, callnum);
}
Loading