From db5f0cc8c71103e6119f25a1bf58b3cdafd59350 Mon Sep 17 00:00:00 2001 From: rohith-raju Date: Mon, 30 Oct 2023 09:13:56 +0000 Subject: [PATCH] update: address review requests Signed-off-by: rohith-raju --- driver/SCHEMA_VERSION | 2 +- driver/bpf/fillers.h | 36 +++++------- driver/fillers_table.c | 2 +- .../syscall_dispatched_events/prlimit64.bpf.c | 4 +- .../syscall_dispatched_events/setrlimit.bpf.c | 4 +- driver/ppm_fillers.c | 28 ++++----- driver/ppm_fillers.h | 2 +- driver/ppm_flag_helpers.h | 2 +- .../syscall_exit_suite/setrlimit_x.cpp | 57 +++++++++++++++++-- 9 files changed, 85 insertions(+), 52 deletions(-) diff --git a/driver/SCHEMA_VERSION b/driver/SCHEMA_VERSION index 56beced9a1..965a689ec0 100644 --- a/driver/SCHEMA_VERSION +++ b/driver/SCHEMA_VERSION @@ -1 +1 @@ -2.12.4 +2.13.4 diff --git a/driver/bpf/fillers.h b/driver/bpf/fillers.h index 9d988982be..d08251bcc4 100644 --- a/driver/bpf/fillers.h +++ b/driver/bpf/fillers.h @@ -1031,7 +1031,7 @@ FILLER(sys_access_e, true) FILLER(sys_getrlimit_setrlimit_e, true) { /* Parameter 1: resource (type: PT_ENUMFLAGS8) */ - unsigned long resource = bpf_syscall_get_argument(data, 0); + uint32_t resource = bpf_syscall_get_argument(data, 0); return bpf_push_u8_to_ring(data, rlimit_resource_to_scap(resource)); } @@ -1051,8 +1051,7 @@ FILLER(sys_getrlimit_x, true) /* * Copy the user structure and extract cur and max */ - if(retval >= 0 || data->state->tail_ctx.evt_type == PPME_SYSCALL_SETRLIMIT_X) - { + if(retval >= 0){ struct rlimit rl; val = bpf_syscall_get_argument(data, 1); @@ -1061,14 +1060,13 @@ FILLER(sys_getrlimit_x, true) cur = rl.rlim_cur; max = rl.rlim_max; - } - else - { + + } else { cur = -1; max = -1; } - /* Parameter 2: cur (type: PT_ERRNO) */ + /* Parameter 2: cur (type: PT_INT64) */ res = bpf_push_s64_to_ring(data, cur); CHECK_RES(res); @@ -1076,7 +1074,7 @@ FILLER(sys_getrlimit_x, true) return bpf_push_s64_to_ring(data, max); } -FILLER(sys_setrlrimit_x, true) +FILLER(sys_setrlimit_x, true) { unsigned long val; long retval; @@ -1092,22 +1090,14 @@ FILLER(sys_setrlrimit_x, true) /* * Copy the user structure and extract cur and max */ - if (retval >= 0 || - data->state->tail_ctx.evt_type == PPME_SYSCALL_SETRLIMIT_X) { - struct rlimit rl; - + struct rlimit rl = {0}; + val = bpf_syscall_get_argument(data, 1); - if (bpf_probe_read_user(&rl, sizeof(rl), (void *)val)) - return PPM_FAILURE_INVALID_USER_MEMORY; - + bpf_probe_read_user(&rl, sizeof(rl), (void *)val); cur = rl.rlim_cur; max = rl.rlim_max; - } else { - cur = -1; - max = -1; - } - /* Parameter 2: cur (type: PT_ERRNO) */ + /* Parameter 2: cur (type: PT_INT64) */ res = bpf_push_s64_to_ring(data, cur); CHECK_RES(res); @@ -1116,7 +1106,7 @@ FILLER(sys_setrlrimit_x, true) CHECK_RES(res); /* Parameter 4: resource (type: PT_ERRNO) */ - unsigned long resource = bpf_syscall_get_argument(data, 0); + uint32_t resource = bpf_syscall_get_argument(data, 0); return bpf_push_u8_to_ring(data, rlimit_resource_to_scap(resource)); } @@ -3862,7 +3852,7 @@ FILLER(sys_prlimit_e, true) CHECK_RES(res); /* Parameter 2: resource (type: PT_ENUMFLAGS8) */ - unsigned long resource = bpf_syscall_get_argument(data, 1); + uint32_t resource = bpf_syscall_get_argument(data, 1); return bpf_push_u8_to_ring(data, rlimit_resource_to_scap(resource)); } @@ -3930,7 +3920,7 @@ FILLER(sys_prlimit_x, true) CHECK_RES(res); /* Parameter 7: resource */ - unsigned long resource = bpf_syscall_get_argument(data, 1); + uint32_t resource = bpf_syscall_get_argument(data, 1); return bpf_push_u8_to_ring(data, rlimit_resource_to_scap(resource)); } diff --git a/driver/fillers_table.c b/driver/fillers_table.c index 6e855f15f4..805e2e8257 100644 --- a/driver/fillers_table.c +++ b/driver/fillers_table.c @@ -147,7 +147,7 @@ const struct ppm_event_entry g_ppm_events[PPM_EVENT_MAX] = { [PPME_SYSCALL_GETRLIMIT_E] = {FILLER_REF(sys_getrlimit_setrlimit_e)}, [PPME_SYSCALL_GETRLIMIT_X] = {FILLER_REF(sys_getrlimit_x)}, [PPME_SYSCALL_SETRLIMIT_E] = {FILLER_REF(sys_getrlimit_setrlimit_e)}, - [PPME_SYSCALL_SETRLIMIT_X] = {FILLER_REF(sys_setrlrimit_x)}, + [PPME_SYSCALL_SETRLIMIT_X] = {FILLER_REF(sys_setrlimit_x)}, [PPME_SYSCALL_PRLIMIT_E] = {FILLER_REF(sys_prlimit_e)}, [PPME_SYSCALL_PRLIMIT_X] = {FILLER_REF(sys_prlimit_x)}, [PPME_DROP_E] = {FILLER_REF(sched_drop)}, diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/prlimit64.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/prlimit64.bpf.c index 42d09961d9..fd4399fe5e 100644 --- a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/prlimit64.bpf.c +++ b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/prlimit64.bpf.c @@ -30,7 +30,7 @@ int BPF_PROG(prlimit64_e, ringbuf__store_s64(&ringbuf, (int64_t)pid); /* Parameter 2: resource (type: PT_ENUMFLAGS8) */ - unsigned long resource = extract__syscall_argument(regs, 1); + uint32_t resource = extract__syscall_argument(regs, 1); ringbuf__store_u8(&ringbuf, rlimit_resource_to_scap(resource)); /*=============================== COLLECT PARAMETERS ===========================*/ @@ -93,7 +93,7 @@ int BPF_PROG(prlimit64_x, ringbuf__store_s64(&ringbuf, (s64)pid); /* Parameter 7: resource (type: PT_ENUMFLAGS8) */ - unsigned long resource = extract__syscall_argument(regs, 1); + uint32_t resource = extract__syscall_argument(regs, 1); ringbuf__store_u8(&ringbuf, rlimit_resource_to_scap(resource)); /*=============================== COLLECT PARAMETERS ===========================*/ diff --git a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/setrlimit.bpf.c b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/setrlimit.bpf.c index bb0a9373f7..48f59ebf97 100644 --- a/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/setrlimit.bpf.c +++ b/driver/modern_bpf/programs/tail_called/events/syscall_dispatched_events/setrlimit.bpf.c @@ -26,7 +26,7 @@ int BPF_PROG(setrlimit_e, /*=============================== COLLECT PARAMETERS ===========================*/ /* Parameter 1: resource (type: PT_ENUMFLAGS8) */ - unsigned long resource = extract__syscall_argument(regs, 0); + uint32_t resource = extract__syscall_argument(regs, 0); ringbuf__store_u8(&ringbuf, rlimit_resource_to_scap(resource)); /*=============================== COLLECT PARAMETERS ===========================*/ @@ -69,7 +69,7 @@ int BPF_PROG(setrlimit_x, ringbuf__store_s64(&ringbuf, rl.rlim_max); /* Parameter 4: resource (type: PT_ENUMFLAGS8) */ - unsigned long resource = extract__syscall_argument(regs, 0); + uint32_t resource = extract__syscall_argument(regs, 0); ringbuf__store_u8(&ringbuf, rlimit_resource_to_scap(resource)); /*=============================== COLLECT PARAMETERS ===========================*/ diff --git a/driver/ppm_fillers.c b/driver/ppm_fillers.c index 9186cb8542..6a329497c8 100644 --- a/driver/ppm_fillers.c +++ b/driver/ppm_fillers.c @@ -4073,7 +4073,7 @@ int f_sys_getrlimit_setrlimit_e(struct event_filler_arguments *args) */ syscall_get_arguments_deprecated(args, 0, 1, &val); - ppm_resource = rlimit_resource_to_scap(val); + ppm_resource = rlimit_resource_to_scap((uint32_t)val); res = val_to_ring(args, (uint64_t)ppm_resource, 0, false, 0); CHECK_RES(res); @@ -4100,8 +4100,8 @@ int f_sys_getrlimit_x(struct event_filler_arguments *args) { /* * Copy the user structure and extract cur and max */ - if(retval >= 0 || args->event_type == PPME_SYSCALL_SETRLIMIT_X) - { +if(retval >= 0) { + syscall_get_arguments_deprecated(args, 1, 1, &val); #ifdef CONFIG_COMPAT @@ -4109,7 +4109,7 @@ int f_sys_getrlimit_x(struct event_filler_arguments *args) { { #endif if(unlikely(ppm_copy_from_user(&rl, (const void __user *)val, sizeof(struct rlimit)))) - return PPM_FAILURE_INVALID_USER_MEMORY; + return PPM_FAILURE_INVALID_USER_MEMORY; cur = rl.rlim_cur; max = rl.rlim_max; #ifdef CONFIG_COMPAT @@ -4143,17 +4143,17 @@ int f_sys_getrlimit_x(struct event_filler_arguments *args) { -int f_sys_setrlrimit_x(struct event_filler_arguments *args) +int f_sys_setrlimit_x(struct event_filler_arguments *args) { unsigned long val; int res; int64_t retval; - struct rlimit rl; #ifdef CONFIG_COMPAT struct compat_rlimit compat_rl; #endif int64_t cur; int64_t max; + struct rlimit rl = {0}; /* Parameter 1: res (type: PT_ERRNO) */ retval = (int64_t)(long)syscall_get_return_value(current, args->regs); @@ -4163,14 +4163,12 @@ int f_sys_setrlrimit_x(struct event_filler_arguments *args) /* * Copy the user structure and extract cur and max */ - if (retval >= 0 || args->event_type == PPME_SYSCALL_SETRLIMIT_X) { syscall_get_arguments_deprecated(args, 1, 1, &val); #ifdef CONFIG_COMPAT if (!args->compat) { #endif - if (unlikely(ppm_copy_from_user(&rl, (const void __user *)val, sizeof(struct rlimit)))) - return PPM_FAILURE_INVALID_USER_MEMORY; + ppm_copy_from_user(&rl, (const void __user *)val, sizeof(struct rlimit)); cur = rl.rlim_cur; max = rl.rlim_max; #ifdef CONFIG_COMPAT @@ -4181,12 +4179,8 @@ int f_sys_setrlrimit_x(struct event_filler_arguments *args) max = compat_rl.rlim_max; } #endif - } else { - cur = -1; - max = -1; - } - /* Parameter 2: (type: PT_INT64) */ + /* Parameter 2: curr (type: PT_INT64) */ res = val_to_ring(args, cur, 0, false, 0); CHECK_RES(res); @@ -4196,7 +4190,7 @@ int f_sys_setrlrimit_x(struct event_filler_arguments *args) /* Parameter 4: resource (type: PT_ENUMFLAGS8) */ syscall_get_arguments_deprecated(args, 0, 1, &val); - res = val_to_ring(args, rlimit_resource_to_scap(val), 0, false, 0); + res = val_to_ring(args, rlimit_resource_to_scap((uint32_t)val), 0, false, 0); CHECK_RES(res); return add_sentinel(args); @@ -4216,7 +4210,7 @@ int f_sys_prlimit_e(struct event_filler_arguments *args) /* Parameter 2: resource (type: PT_ENUMFLAGS8) */ syscall_get_arguments_deprecated(args, 1, 1, &val); - res = val_to_ring(args, rlimit_resource_to_scap(val), 0, false, 0); + res = val_to_ring(args, rlimit_resource_to_scap((uint32_t)val), 0, false, 0); CHECK_RES(res); return add_sentinel(args); @@ -4321,7 +4315,7 @@ int f_sys_prlimit_x(struct event_filler_arguments *args) /* Parameter 7: resource (type: PT_ENUMFLAGS8) */ syscall_get_arguments_deprecated(args, 1, 1, &val); - res = val_to_ring(args, rlimit_resource_to_scap(val), 0, false, 0); + res = val_to_ring(args, rlimit_resource_to_scap((uint32_t)val), 0, false, 0); CHECK_RES(res); return add_sentinel(args); diff --git a/driver/ppm_fillers.h b/driver/ppm_fillers.h index d26f0876ec..5efbdd873f 100644 --- a/driver/ppm_fillers.h +++ b/driver/ppm_fillers.h @@ -68,7 +68,7 @@ or GPL2.txt for full copies of the license. FN(sys_nanosleep_e) \ FN(sys_getrlimit_setrlimit_e) \ FN(sys_getrlimit_x) \ - FN(sys_setrlrimit_x) \ + FN(sys_setrlimit_x) \ FN(sys_prlimit_e) \ FN(sys_prlimit_x) \ FN(sched_switch_e) \ diff --git a/driver/ppm_flag_helpers.h b/driver/ppm_flag_helpers.h index 040d3e47af..fb1c03f03e 100644 --- a/driver/ppm_flag_helpers.h +++ b/driver/ppm_flag_helpers.h @@ -1293,7 +1293,7 @@ static __always_inline uint32_t access_flags_to_scap(unsigned flags) return res; } -static __always_inline uint8_t rlimit_resource_to_scap(unsigned long rresource) +static __always_inline u8 rlimit_resource_to_scap(uint32_t rresource) { switch (rresource) { case RLIMIT_CPU: diff --git a/test/drivers/test_suites/syscall_exit_suite/setrlimit_x.cpp b/test/drivers/test_suites/syscall_exit_suite/setrlimit_x.cpp index 6a7a1c839d..7339e31b65 100644 --- a/test/drivers/test_suites/syscall_exit_suite/setrlimit_x.cpp +++ b/test/drivers/test_suites/syscall_exit_suite/setrlimit_x.cpp @@ -4,7 +4,7 @@ #include -TEST(SyscallExit, setrlimitX) +TEST(SyscallExit, setrlimitX_failure) { auto evt_test = get_syscall_event_test(__NR_setrlimit, EXIT_EVENT); @@ -13,9 +13,7 @@ TEST(SyscallExit, setrlimitX) /*=============================== TRIGGER SYSCALL ===========================*/ int resource = -1; - struct rlimit rlim; - rlim.rlim_cur = 50; - rlim.rlim_max = 10020; + struct rlimit rlim = {0}; assert_syscall_state(SYSCALL_FAILURE, "setrlimit", syscall(__NR_setrlimit, resource, &rlim)); int64_t errno_value = -errno; @@ -52,4 +50,55 @@ TEST(SyscallExit, setrlimitX) evt_test->assert_num_params_pushed(4); } + +TEST(SyscallExit, setrlimitX_success) +{ + auto evt_test = get_syscall_event_test(__NR_setrlimit, EXIT_EVENT); + + evt_test->enable_capture(); + + /*=============================== TRIGGER SYSCALL ===========================*/ + + int resource = RLIMIT_MEMLOCK; + struct rlimit rlim; + rlim.rlim_cur = 50; + rlim.rlim_max = 10020; + + int ret = syscall(__NR_setrlimit, resource, &rlim); + assert_syscall_state(SYSCALL_SUCCESS, "setrlimit", ret, NOT_EQUAL, -1); + // On success, setrlimit return 0. + + /*=============================== 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: res (type: PT_ERRNO) */ + evt_test->assert_numeric_param(1, (int64_t)ret); + + /* Parameter 2: cur (type: PT_INT64) */ + evt_test->assert_numeric_param(2, (int64_t)rlim.rlim_cur); + + /* Parameter 3: max (type: PT_INT64) */ + evt_test->assert_numeric_param(3, (int64_t)rlim.rlim_max); + + /* Parameter 4: resource (type: PT_ENUMFLAGS8) */ + evt_test->assert_numeric_param(4, (uint8_t)resource); + + /*=============================== ASSERT PARAMETERS ===========================*/ + + evt_test->assert_num_params_pushed(4); +} #endif