cleanup(libsinsp): cleanup unaligned access in plugin framework+tests

Signed-off-by: Luca Guerra <[email protected]>

userspace/libscap/engine/source_plugin/source_plugin.c

@@ -21,6 +21,7 @@ limitations under the License.
 #include <stdlib.h>
 #include <stdio.h>
 #include <stddef.h>
+#include <string.h>
 
 #include <libscap/engine/source_plugin/source_plugin.h>
 #include <libscap/engine/noop/noop.h>
@@ -209,7 +210,10 @@ static int32_t next(struct scap_engine_handle engine, OUT scap_evt** pevent, OUT
 	// Sanity checks in case a plugin implements a non-syscall event source.
 	// If a plugin has event sourcing capability and has a specific ID, then
 	// it is allowed to produce only plugin events of its own event source.
-	uint32_t* plugin_id = (uint32_t*)((uint8_t*) evt + sizeof(scap_evt) + 4 + 4);
+	uint32_t* pplugin_id = (uint32_t*)((uint8_t*) evt + sizeof(scap_evt) + 4 + 4);
+	uint32_t plugin_id;
+	memcpy(&plugin_id, pplugin_id, sizeof(plugin_id));
+
 	if (handle->m_input_plugin->id != 0)
 	{
 		/*
@@ -224,21 +228,22 @@ static int32_t next(struct scap_engine_handle engine, OUT scap_evt** pevent, OUT
 		}
 
 		// forcely setting plugin ID with the one of the open plugin
-		if (*plugin_id == 0)
+		if (plugin_id == 0)
 		{
-			*plugin_id = handle->m_input_plugin->id;
+			plugin_id = handle->m_input_plugin->id;
+			memcpy(pplugin_id, &plugin_id, sizeof(plugin_id));
 		}
-		else if (*plugin_id != handle->m_input_plugin->id)
+		else if (plugin_id != handle->m_input_plugin->id)
 		{
-			snprintf(lasterr, SCAP_LASTERR_SIZE, "unexpected plugin ID in plugin event: plugin='%s', expected_id=%d, actual_id=%d", handle->m_input_plugin->name, handle->m_input_plugin->id, *plugin_id);
+			snprintf(lasterr, SCAP_LASTERR_SIZE, "unexpected plugin ID in plugin event: plugin='%s', expected_id=%d, actual_id=%d", handle->m_input_plugin->name, handle->m_input_plugin->id, plugin_id);
 			return SCAP_FAILURE;
 		}
 	}
 
 	if (evt->type == PPME_PLUGINEVENT_E)
 	{
 		// a zero plugin ID is not allowed for PPME_PLUGINEVENT_E
-		if (*plugin_id == 0)
+		if (plugin_id == 0)
 		{
 			snprintf(lasterr, SCAP_LASTERR_SIZE, "malformed plugin event produced by plugin (no ID): '%s'", handle->m_input_plugin->name);
 			return SCAP_FAILURE;

userspace/libscap/scap_event.c

@@ -331,7 +331,10 @@ int32_t scap_event_encode_params_v(const struct scap_sized_buffer event_buf, siz
 	len = len + sizeof(uint32_t);
 #endif
 
-	*event_size = len;
+	if (event_size != NULL)
+	{
+		*event_size = len;
+	}
 
 	// we were not able to write the event to the buffer
 	if (!scap_buffer_can_fit(event_buf, len))

userspace/libsinsp/sinsp.cpp

@@ -18,6 +18,7 @@ limitations under the License.
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <string.h>
 #ifndef _WIN32
 #include <unistd.h>
 #include <sys/stat.h>
@@ -2222,7 +2223,7 @@ void sinsp::handle_plugin_async_event(const sinsp_plugin& p, std::unique_ptr<sin
 
 		// write plugin ID and timestamp in the event and kick it in the queue
 		auto plid = (uint32_t*)((uint8_t*) evt->get_scap_evt() + sizeof(scap_evt) + 4+4+4);
-		*plid = cur_plugin_id;
+		memcpy(plid, &cur_plugin_id, sizeof(cur_plugin_id));
 		handle_async_event(std::move(evt));
 	}
 }

userspace/libsinsp/test/plugins/plugin_source.cpp

@@ -21,6 +21,7 @@ limitations under the License.
 #include <sstream>
 
 #include <driver/ppm_events_public.h>
+#include <scap.h>
 
 #include "test_plugins.h"
 
@@ -139,27 +140,21 @@ static ss_plugin_rc plugin_next_batch(ss_plugin_t* s, ss_instance_t* i, uint32_t
 
     *nevts = 1;
     *evts = &istate->evt;
-    istate->evt->type = PPME_PLUGINEVENT_E;
-    istate->evt->tid = -1;
-    istate->evt->ts = UINT64_MAX;
-    istate->evt->len = sizeof(ss_plugin_event);
-    istate->evt->nparams = 2;
 
-    uint8_t* parambuf = &istate->evt_buf[0] + sizeof(ss_plugin_event);
+    char error[SCAP_LASTERR_SIZE];
 
-    // lenghts
-    *((uint32_t*) parambuf) = sizeof(uint32_t);
-    parambuf += sizeof(uint32_t);
-    *((uint32_t*) parambuf) = strlen(s_evt_data) + 1;
-    parambuf += sizeof(uint32_t);
+    int32_t encode_res = scap_event_encode_params(scap_sized_buffer{istate->evt, sizeof(istate->evt_buf)},
+        nullptr, error, PPME_PLUGINEVENT_E, 2,
+        plugin_get_id(), s_evt_data);
 
-    // params
-    *((uint32_t*) parambuf) = plugin_get_id();
-    parambuf += sizeof(uint32_t);
-    strcpy((char*) parambuf, s_evt_data);
-    parambuf += strlen(s_evt_data) + 1;
+    if (encode_res == SCAP_FAILURE)
+    {
+        return SS_PLUGIN_FAILURE;
+    }
+
+    istate->evt->tid = -1;
+    istate->evt->ts = UINT64_MAX;
 
-    istate->evt->len += parambuf - (&istate->evt_buf[0] + sizeof(ss_plugin_event));
     istate->count--;
     return SS_PLUGIN_SUCCESS;
 }

userspace/libsinsp/test/plugins/syscall_async.cpp

@@ -23,6 +23,7 @@ limitations under the License.
 #include <atomic>
 
 #include <driver/ppm_events_public.h>
+#include <scap.h>
 
 #include "test_plugins.h"
 
@@ -124,35 +125,6 @@ static const char* plugin_get_last_error(ss_plugin_t* s)
     return ((plugin_state *) s)->lasterr.c_str();
 }
 
-static void encode_async_event(ss_plugin_event* evt, uint64_t tid, const char* name, const char* data)
-{
-    // set event info
-    evt->type = PPME_ASYNCEVENT_E;
-    evt->tid = tid;
-    evt->len = sizeof(ss_plugin_event);
-    evt->nparams = 3;
-
-    // lenghts
-    uint8_t* parambuf = (uint8_t*) evt + sizeof(ss_plugin_event);
-    *((uint32_t*) parambuf) = sizeof(uint32_t);
-    parambuf += sizeof(uint32_t);
-    *((uint32_t*) parambuf) = strlen(name) + 1;
-    parambuf += sizeof(uint32_t);
-    *((uint32_t*) parambuf) = strlen(data) + 1;
-    parambuf += sizeof(uint32_t);
-
-    // params
-    // skip plugin ID, it will be filled by the framework
-    parambuf += sizeof(uint32_t);
-    strcpy((char*) parambuf, name);
-    parambuf += strlen(name) + 1;
-    strcpy((char*) parambuf, data);
-    parambuf += strlen(data) + 1;
-
-    // update event's len
-    evt->len += parambuf - ((uint8_t*) evt + sizeof(ss_plugin_event));
-}
-
 static ss_plugin_rc plugin_set_async_event_handler(ss_plugin_t* s, ss_plugin_owner_t* owner, const ss_plugin_async_event_handler_t handler)
 {
     plugin_state *ps = (plugin_state *) s;
@@ -179,7 +151,10 @@ static ss_plugin_rc plugin_set_async_event_handler(ss_plugin_t* s, ss_plugin_own
             for (uint64_t i = 0; i < ps->async_maxevts && ps->async_thread_run; i++)
             {
                 // attempt sending an event that is not in the allowed name list
-                encode_async_event(ps->async_evt, 1, "unsupportedname", data);
+                scap_event_encode_params(scap_sized_buffer{ps->async_evt, sizeof(ps->async_evt_buf)}, nullptr, err,
+                    PPME_ASYNCEVENT_E, 3, 0, "unsupportedname", scap_const_sized_buffer{data, strlen(data) + 1});
+                ps->async_evt->tid = 1;
+
                 if (SS_PLUGIN_SUCCESS == handler(owner, ps->async_evt, err))
                 {
                     printf("sample_syscall_async: unexpected success in sending unsupported asynchronous event from plugin\n");
@@ -189,7 +164,10 @@ static ss_plugin_rc plugin_set_async_event_handler(ss_plugin_t* s, ss_plugin_own
                 // send an event in the allowed name list
                 // note: we set a tid=1 to test that async events can have
                 // either an empty (-1) or a non-empty tid value
-                encode_async_event(ps->async_evt, 1, name, data);
+                scap_event_encode_params(scap_sized_buffer{ps->async_evt, sizeof(ps->async_evt_buf)}, nullptr, err,
+                    PPME_ASYNCEVENT_E, 3, 0, name, scap_const_sized_buffer{data, strlen(data) + 1});
+                ps->async_evt->tid = 1;
+
                 if (SS_PLUGIN_SUCCESS != handler(owner, ps->async_evt, err))
                 {
                     printf("sample_syscall_async: unexpected failure in sending asynchronous event from plugin: %s\n", err);

userspace/libsinsp/test/plugins/syscall_source.cpp

@@ -23,6 +23,7 @@ limitations under the License.
 #include <driver/ppm_events_public.h>
 
 #include "test_plugins.h"
+#include <scap.h>
 
 /**
  * Example of plugin implementing only the event sourcing capability, which:
@@ -127,43 +128,21 @@ static ss_plugin_rc plugin_next_batch(ss_plugin_t* s, ss_instance_t* i, uint32_t
 
     *nevts = 1;
     *evts = &istate->evt;
-    istate->evt->type = PPME_SYSCALL_OPEN_X;
+
+    char error[SCAP_LASTERR_SIZE];
+
+    int32_t encode_res = scap_event_encode_params(scap_sized_buffer{istate->evt, sizeof(istate->evt_buf)},
+        nullptr, error, PPME_SYSCALL_OPEN_X, 6,
+        (uint64_t) 3, "/tmp/the_file", ((1 << 0) | (1 << 1)), 0, 5, (uint64_t) 123);
+
+    if (encode_res == SCAP_FAILURE)
+    {
+        return SS_PLUGIN_FAILURE;
+    }
+
     istate->evt->tid = 1;
     istate->evt->ts = UINT64_MAX;
-    istate->evt->len = sizeof(ss_plugin_event);
-    istate->evt->nparams = 6;
-
-    uint8_t* parambuf = &istate->evt_buf[0] + sizeof(ss_plugin_event);
-
-    // lenghts
-    *((uint16_t*) parambuf) = sizeof(uint64_t);
-    parambuf += sizeof(uint16_t);
-    *((uint16_t*) parambuf) = strlen("/tmp/the_file") + 1;
-    parambuf += sizeof(uint16_t);
-    *((uint16_t*) parambuf) = sizeof(uint32_t);
-    parambuf += sizeof(uint16_t);
-    *((uint16_t*) parambuf) = sizeof(uint32_t);
-    parambuf += sizeof(uint16_t);
-    *((uint16_t*) parambuf) = sizeof(uint32_t);
-    parambuf += sizeof(uint16_t);
-    *((uint16_t*) parambuf) = sizeof(uint64_t);
-    parambuf += sizeof(uint16_t);
-
-    // params
-    *((uint64_t*) parambuf) = 3;
-    parambuf += sizeof(uint64_t);
-    strcpy((char*) parambuf, "/tmp/the_file");
-    parambuf += strlen("/tmp/the_file") + 1;
-    *((uint32_t*) parambuf) = ((1 << 0) | (1 << 1));
-    parambuf += sizeof(uint32_t);
-    *((uint32_t*) parambuf) = 0;
-    parambuf += sizeof(uint32_t);
-    *((uint32_t*) parambuf) = 5;
-    parambuf += sizeof(uint32_t);
-    *((uint64_t*) parambuf) = 123;
-    parambuf += sizeof(uint64_t);
-
-    istate->evt->len += parambuf - (&istate->evt_buf[0] + sizeof(ss_plugin_event));
+
     istate->count--;
     return SS_PLUGIN_SUCCESS;
 }