From a02a0ec234d79aa6676c18d74deb5921edb83a07 Mon Sep 17 00:00:00 2001
From: Federico Di Pierro <nierro92@gmail.com>
Date: Wed, 23 Oct 2024 11:11:15 +0200
Subject: [PATCH] fix(userspace/libsinsp): multiple fixes related to rawargs.

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 <nierro92@gmail.com>

Co-authored-by: Andrea Terzolo <andreaterzolo3@gmail.com>
---
 userspace/libsinsp/sinsp_filtercheck.cpp      | 18 ++++++--
 .../libsinsp/sinsp_filtercheck_event.cpp      | 18 ++++++--
 userspace/libsinsp/sinsp_filtercheck_event.h  |  1 +
 userspace/libsinsp/test/filterchecks/evt.cpp  | 45 +++++++++++++++++++
 4 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/userspace/libsinsp/sinsp_filtercheck.cpp b/userspace/libsinsp/sinsp_filtercheck.cpp
index f7a281f4db0..0989108262d 100644
--- a/userspace/libsinsp/sinsp_filtercheck.cpp
+++ b/userspace/libsinsp/sinsp_filtercheck.cpp
@@ -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) {
@@ -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) {
@@ -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) {
@@ -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;
@@ -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;
@@ -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;
diff --git a/userspace/libsinsp/sinsp_filtercheck_event.cpp b/userspace/libsinsp/sinsp_filtercheck_event.cpp
index cb21635bbbd..36f3c1d85c9 100644
--- a/userspace/libsinsp/sinsp_filtercheck_event.cpp
+++ b/userspace/libsinsp/sinsp_filtercheck_event.cpp
@@ -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")) {
@@ -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 {
@@ -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;
diff --git a/userspace/libsinsp/sinsp_filtercheck_event.h b/userspace/libsinsp/sinsp_filtercheck_event.h
index 8dbea21d233..0c88733baa2 100644
--- a/userspace/libsinsp/sinsp_filtercheck_event.h
+++ b/userspace/libsinsp/sinsp_filtercheck_event.h
@@ -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;
diff --git a/userspace/libsinsp/test/filterchecks/evt.cpp b/userspace/libsinsp/test/filterchecks/evt.cpp
index 5924e47cd14..cb35bf8eb61 100644
--- a/userspace/libsinsp/test/filterchecks/evt.cpp
+++ b/userspace/libsinsp/test/filterchecks/evt.cpp
@@ -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
+}