Skip to content

Commit

Permalink
fix(userspace/libsinsp): multiple fixes related to rawargs.
Browse files Browse the repository at this point in the history
Firstly, properly refresh m_arginfo and m_customfield type and print format
given current event while extracting rawarg values.

Secondly, propelry support PT_FLAGS and PT_ENUMFLAGS types in `rawval_to_json` and `rawval_to_string`.

Lastly, honor PF_HEX print format for 8,16,32bits types.

Signed-off-by: Federico Di Pierro <[email protected]>

Co-authored-by: Andrea Terzolo <[email protected]>
  • Loading branch information
FedeDP and Andreagit97 committed Oct 23, 2024
1 parent b930204 commit a02a0ec
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 6 deletions.
18 changes: 15 additions & 3 deletions userspace/libsinsp/sinsp_filtercheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,8 @@ Json::Value sinsp_filter_check::rawval_to_json(uint8_t* rawval,

case PT_L4PROTO: // This can be resolved in the future
case PT_UINT8:
case PT_FLAGS8:
case PT_ENUMFLAGS8:
if(print_format == PF_DEC || print_format == PF_ID) {
return *(uint8_t*)rawval;
} else if(print_format == PF_OCT || print_format == PF_HEX) {
Expand All @@ -150,6 +152,8 @@ Json::Value sinsp_filter_check::rawval_to_json(uint8_t* rawval,

case PT_PORT: // This can be resolved in the future
case PT_UINT16:
case PT_FLAGS16:
case PT_ENUMFLAGS16:
if(print_format == PF_DEC || print_format == PF_ID) {
return *(uint16_t*)rawval;
} else if(print_format == PF_OCT || print_format == PF_HEX) {
Expand All @@ -160,6 +164,8 @@ Json::Value sinsp_filter_check::rawval_to_json(uint8_t* rawval,
}

case PT_UINT32:
case PT_FLAGS32:
case PT_ENUMFLAGS32:
if(print_format == PF_DEC || print_format == PF_ID) {
return *(uint32_t*)rawval;
} else if(print_format == PF_OCT || print_format == PF_HEX) {
Expand Down Expand Up @@ -292,12 +298,14 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
return m_getpropertystr_storage.data();
case PT_L4PROTO: // This can be resolved in the future
case PT_UINT8:
case PT_FLAGS8:
case PT_ENUMFLAGS8:
if(print_format == PF_OCT) {
prfmt = (char*)"%" PRIo8;
} else if(print_format == PF_DEC || print_format == PF_ID) {
prfmt = (char*)"%" PRIu8;
} else if(print_format == PF_HEX) {
prfmt = (char*)"%" PRIu8;
prfmt = (char*)"%" PRIX8;
} else {
ASSERT(false);
return NULL;
Expand All @@ -311,12 +319,14 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
return m_getpropertystr_storage.data();
case PT_PORT: // This can be resolved in the future
case PT_UINT16:
case PT_FLAGS16:
case PT_ENUMFLAGS16:
if(print_format == PF_OCT) {
prfmt = (char*)"%" PRIo16;
} else if(print_format == PF_DEC || print_format == PF_ID) {
prfmt = (char*)"%" PRIu16;
} else if(print_format == PF_HEX) {
prfmt = (char*)"%" PRIu16;
prfmt = (char*)"%" PRIX16;
} else {
ASSERT(false);
return NULL;
Expand All @@ -329,12 +339,14 @@ char* sinsp_filter_check::rawval_to_string(uint8_t* rawval,
rawval_cast<uint16_t>(rawval));
return m_getpropertystr_storage.data();
case PT_UINT32:
case PT_FLAGS32:
case PT_ENUMFLAGS32:
if(print_format == PF_OCT) {
prfmt = (char*)"%" PRIo32;
} else if(print_format == PF_DEC || print_format == PF_ID) {
prfmt = (char*)"%" PRIu32;
} else if(print_format == PF_HEX) {
prfmt = (char*)"%" PRIu32;
prfmt = (char*)"%" PRIX32;
} else {
ASSERT(false);
return NULL;
Expand Down
18 changes: 15 additions & 3 deletions userspace/libsinsp/sinsp_filtercheck_event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,13 @@ int32_t sinsp_filter_check_event::parse_field_name(std::string_view val,

res = extract_arg("evt.rawarg", val, &m_arginfo);

// These fields will be overridden with correct ones from the current event
// while extracting from event.
// But we also need to set them at init time to something different than `PT_DYN`
// to allow `eval_filter` expressions to be properly compiled.
// Note: even if they'll be overridden,
// it is safe to guess that eg: `size` param will always be a numeric type.
// Of course if we had a `size` param that is string, it would be troublesome.
m_customfield.m_type = m_arginfo->type;
m_customfield.m_print_format = m_arginfo->fmt;
} else if(STR_MATCH("evt.around")) {
Expand Down Expand Up @@ -650,10 +657,15 @@ void sinsp_filter_check_event::validate_filter_value(const char* str, uint32_t l
}
}

uint8_t* extract_argraw(sinsp_evt* evt, uint32_t* len, const char* argname) {
const sinsp_evt_param* pi = evt->get_param_by_name(argname);
uint8_t* sinsp_filter_check_event::extract_argraw(sinsp_evt* evt,
uint32_t* len,
const char* argname) {
auto pi = evt->get_param_by_name(argname);

if(pi != NULL) {
m_arginfo = pi->get_info();
m_customfield.m_type = m_arginfo->type;
m_customfield.m_print_format = m_arginfo->fmt;
*len = pi->m_len;
return (uint8_t*)pi->m_val;
} else {
Expand Down Expand Up @@ -1192,7 +1204,7 @@ uint8_t* sinsp_filter_check_event::extract_single(sinsp_evt* evt,
m_val.u16 = evt->get_cpuid();
RETURN_EXTRACT_VAR(m_val.u16);
case TYPE_ARGRAW:
return extract_argraw(evt, len, m_arginfo->name);
return extract_argraw(evt, len, m_argname.c_str());
break;
case TYPE_ARGSTR: {
const char* resolved_argstr;
Expand Down
1 change: 1 addition & 0 deletions userspace/libsinsp/sinsp_filtercheck_event.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ class sinsp_filter_check_event : public sinsp_filter_check {
uint8_t* extract_error_count(sinsp_evt* evt, uint32_t* len);
uint8_t* extract_abspath(sinsp_evt* evt, uint32_t* len);
inline uint8_t* extract_buflen(sinsp_evt* evt, uint32_t* len);
uint8_t* extract_argraw(sinsp_evt* evt, uint32_t* len, const char* argname);

union {
uint16_t u16;
Expand Down
45 changes: 45 additions & 0 deletions userspace/libsinsp/test/filterchecks/evt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -210,3 +210,48 @@ TEST_F(sinsp_with_test_input, EVT_FILTER_check_evt_arg_uid) {
ASSERT_EQ(get_field_as_string(evt, "evt.arg[0]"), "<NA>");
ASSERT_EQ(get_field_as_string(evt, "evt.args"), "uid=5(<NA>)");
}

// Test that for rawarg.X we are correctly retrieving the correct field type/format.
TEST_F(sinsp_with_test_input, rawarg_madness) {
add_default_init_thread();
open_inspector();

// [PPME_SYSCALL_EPOLL_CREATE_E] = {"epoll_create", EC_WAIT | EC_SYSCALL, EF_CREATES_FD |
// EF_MODIFIES_STATE, 1, { {"size", PT_INT32, PF_DEC} } },
sinsp_evt* evt =
add_event_advance_ts(increasing_ts(), 1, PPME_SYSCALL_EPOLL_CREATE_E, 1, (int32_t)-22);
ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.size"), "-22");
ASSERT_TRUE(eval_filter(evt, "evt.rawarg.size < -20"));

// [PPME_SYSCALL_SIGNALFD4_X] = {"signalfd4", EC_SIGNAL | EC_SYSCALL, EF_CREATES_FD |
// EF_MODIFIES_STATE, 2, {{"res", PT_FD, PF_DEC}, {"flags", PT_FLAGS16, PF_HEX, file_flags}}},
evt = add_event_advance_ts(increasing_ts(),
1,
PPME_SYSCALL_SIGNALFD4_X,
2,
(int64_t)-1,
(uint16_t)512);
// 512 in hex is 200
ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.flags"), "200");
ASSERT_TRUE(eval_filter(evt, "evt.rawarg.flags < 515"));

// [PPME_SYSCALL_TIMERFD_CREATE_E] = {"timerfd_create",EC_TIME | EC_SYSCALL,EF_CREATES_FD |
// EF_MODIFIES_STATE,2,{{"clockid", PT_UINT8, PF_DEC},{"flags", PT_UINT8, PF_HEX}}},
evt = add_event_advance_ts(increasing_ts(),
1,
PPME_SYSCALL_TIMERFD_CREATE_E,
2,
(uint8_t)-1,
(uint8_t)255);
// 255 in hex is FF
ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.flags"), "FF");
ASSERT_TRUE(eval_filter(evt, "evt.rawarg.flags <= 255"));

// [PPME_SYSCALL_BRK_4_E] = {"brk", EC_MEMORY | EC_SYSCALL, EF_NONE, 1, {{"addr", PT_UINT64,
// PF_HEX}}}
uint64_t addr = UINT64_MAX;
evt = add_event_advance_ts(increasing_ts(), 1, PPME_SYSCALL_BRK_4_E, 1, addr);
// UINT64_MAX is FFFFFFFFFFFFFFFF
ASSERT_EQ(get_field_as_string(evt, "evt.rawarg.addr"), "FFFFFFFFFFFFFFFF");
ASSERT_ANY_THROW(eval_filter(evt, "evt.rawarg.addr > 0")); // PT_SOCKADDR is not comparable
}

0 comments on commit a02a0ec

Please sign in to comment.