cleanup(libsinsp): do not rely on null-terminated string_views

userspace/libsinsp/fdinfo.cpp

@@ -169,14 +169,14 @@ template<> std::string sinsp_fdinfo_t::tostring_clean()
 	return m_tstr;
 }
 
-template<> void sinsp_fdinfo_t::add_filename_raw(const char* rawpath)
+template<> void sinsp_fdinfo_t::add_filename_raw(std::string_view rawpath)
 {
-	m_name_raw = rawpath;
+	m_name_raw = std::string(rawpath);
 }
 
-template<> void sinsp_fdinfo_t::add_filename(const char* fullpath)
+template<> void sinsp_fdinfo_t::add_filename(std::string_view fullpath)
 {
-	m_name = fullpath;
+	m_name = std::string(fullpath);
 }
 
 template<> bool sinsp_fdinfo_t::set_net_role_by_guessing(sinsp* inspector,

userspace/libsinsp/fdinfo.h

@@ -369,8 +369,8 @@ class SINSP_PUBLIC sinsp_fdinfo
 		FLAGS_CONNECTION_FAILED = (1 << 16),
 	};
 
-	void add_filename_raw(const char* rawpath);
-	void add_filename(const char* fullpath);
+	void add_filename_raw(std::string_view rawpath);
+	void add_filename(std::string_view fullpath);
 
 	inline bool is_transaction() const
 	{

userspace/libsinsp/parsers.cpp

@@ -2307,17 +2307,14 @@ void sinsp_parser::parse_execve_exit(sinsp_evt *evt)
 				/* If the pathname is `<NA>` here we shouldn't have problems during `parse_dirfd`.
 				* It doesn't start with "/" so it is not considered an absolute path.
 				*/
-				std::string sdir;
-				parse_dirfd(evt, pathname.data(), dirfd, &sdir);
+				std::string sdir = parse_dirfd(evt, pathname, dirfd);
 
 				/* (4) In this case, we were not able to recover the pathname from the kernel or
 				* we are not able to recover information about `dirfd` in our `sinsp` state.
 				* Fallback to `<NA>`.
 				*/
-				if((!(flags & PPM_EXVAT_AT_EMPTY_PATH) && strncmp(pathname.data(), "<NA>", 5) == 0) ||
-				sdir.compare("<UNKNOWN>") == 0)
+				if((!(flags & PPM_EXVAT_AT_EMPTY_PATH) && pathname == "<NA>") || sdir == "<UNKNOWN>")
 				{
-					/* we copy also the string terminator `\0`. */
 					fullpath = "<NA>";
 				}
 				/* (3) In this case we have already obtained the `exepath` and it is `sdir`, we just need
@@ -2514,11 +2511,11 @@ void sinsp_parser::parse_execve_exit(sinsp_evt *evt)
  *   - if we have no information about `dirfd` -> sdir = "<UNKNOWN>".
  *   - if `dirfd` has a valid vaule for us -> sdir = path + "/" at the end.
  */
-void sinsp_parser::parse_dirfd(sinsp_evt *evt, const char* name, int64_t dirfd, OUT std::string* sdir)
+std::string sinsp_parser::parse_dirfd(sinsp_evt *evt, std::string_view name, int64_t dirfd)
 {
 	bool is_absolute = false;
 	/* This should never happen but just to be sure. */
-	if(name != NULL)
+	if(name.data() != nullptr)
 	{
 		is_absolute = (name[0] == '/');
 	}
@@ -2532,40 +2529,33 @@ void sinsp_parser::parse_dirfd(sinsp_evt *evt, const char* name, int64_t dirfd,
 		// Some processes (e.g. irqbalance) actually do this: they pass an invalid fd and
 		// and absolute path, and openat succeeds.
 		//
-		*sdir = ".";
+		return ".";
 	}
-	else if(dirfd == PPM_AT_FDCWD)
+
+	if(dirfd == PPM_AT_FDCWD)
 	{
 		if(evt->m_tinfo != NULL)
 		{
-			*sdir = evt->m_tinfo->get_cwd();
-		}
-		else
-		{
-			*sdir = "<UNKNOWN>";
+			return evt->m_tinfo->get_cwd();
 		}
+
+		return "<UNKNOWN>";
 	}
-	else
+
+	evt->m_fdinfo = evt->m_tinfo->get_fd(dirfd);
+
+	if(evt->m_fdinfo == NULL)
 	{
-		evt->m_fdinfo = evt->m_tinfo->get_fd(dirfd);
+		return "<UNKNOWN>";
+	}
 
-		if(evt->m_fdinfo == NULL)
-		{
-			*sdir = "<UNKNOWN>";
-		}
-		else
-		{
-			if(evt->m_fdinfo->m_name[evt->m_fdinfo->m_name.length()] == '/')
-			{
-				*sdir = evt->m_fdinfo->m_name;
-			}
-			else
-			{
-				tdirstr = evt->m_fdinfo->m_name + '/';
-				*sdir = tdirstr;
-			}
-		}
+	if(evt->m_fdinfo->m_name[evt->m_fdinfo->m_name.length()] == '/')
+	{
+		return evt->m_fdinfo->m_name;
 	}
+
+	tdirstr = evt->m_fdinfo->m_name + '/';
+	return tdirstr;
 }
 
 void sinsp_parser::parse_open_openat_creat_exit(sinsp_evt *evt)
@@ -2626,7 +2616,7 @@ void sinsp_parser::parse_open_openat_creat_exit(sinsp_evt *evt)
 			enter_evt_name = enter_evt->get_param(0)->as<std::string_view>();
 			enter_evt_flags = enter_evt->get_param(1)->as<uint32_t>();
 
-			if(enter_evt_name.data() != nullptr && strncmp(enter_evt_name.data(), "<NA>", 5) != 0)
+			if(enter_evt_name.data() != nullptr && enter_evt_name != "<NA>")
 			{
 				name = enter_evt_name;
 
@@ -2660,7 +2650,7 @@ void sinsp_parser::parse_open_openat_creat_exit(sinsp_evt *evt)
 			enter_evt_name = enter_evt->get_param(0)->as<std::string_view>();
 			enter_evt_flags = 0;
 
-			if(enter_evt_name.data() != nullptr && strncmp(enter_evt_name.data(), "<NA>", 5) != 0)
+			if(enter_evt_name.data() != nullptr && enter_evt_name != "<NA>")
 			{
 				name = enter_evt_name;
 
@@ -2682,7 +2672,7 @@ void sinsp_parser::parse_open_openat_creat_exit(sinsp_evt *evt)
 
 		int64_t dirfd = enter_evt->get_param(0)->as<int64_t>();
 
-		parse_dirfd(evt, name.data(), dirfd, &sdir);
+		sdir = parse_dirfd(evt, name, dirfd);
 	}
 	else if(etype == PPME_SYSCALL_OPENAT_2_X || etype == PPME_SYSCALL_OPENAT2_X)
 	{
@@ -2710,7 +2700,7 @@ void sinsp_parser::parse_open_openat_creat_exit(sinsp_evt *evt)
 			enter_evt_flags = enter_evt->get_param(2)->as<uint32_t>();
 			int64_t enter_evt_dirfd = enter_evt->get_param(0)->as<int64_t>();
 
-			if(enter_evt_name.data() != nullptr && strncmp(enter_evt_name.data(), "<NA>", 5) != 0)
+			if(enter_evt_name.data() != nullptr && enter_evt_name != "<NA>")
 			{
 				name = enter_evt_name;
 
@@ -2724,7 +2714,7 @@ void sinsp_parser::parse_open_openat_creat_exit(sinsp_evt *evt)
 			}
 		}
 
-		parse_dirfd(evt, name.data(), dirfd, &sdir);
+		sdir = parse_dirfd(evt, name, dirfd);
 	}
 	else if (etype == PPME_SYSCALL_OPEN_BY_HANDLE_AT_X)
 	{
@@ -2772,8 +2762,8 @@ void sinsp_parser::parse_open_openat_creat_exit(sinsp_evt *evt)
 		fdi.m_mount_id = 0;
 		fdi.m_dev = dev;
 		fdi.m_ino = ino;
-		fdi.add_filename_raw(name.data());
-		fdi.add_filename(fullpath.c_str());
+		fdi.add_filename_raw(name);
+		fdi.add_filename(fullpath);
 
 		//
 		// Add the fd to the table.
@@ -5320,10 +5310,10 @@ void sinsp_parser::parse_user_evt(sinsp_evt *evt)
 
 	if (evt->m_pevt->type == PPME_USER_ADDED_E)
 	{
-		m_inspector->m_usergroup_manager.add_user(container_id.data(), -1, uid, gid, name.data(), home.data(), shell.data());
+		m_inspector->m_usergroup_manager.add_user(std::string(container_id), -1, uid, gid, name, home, shell);
 	} else
 	{
-		m_inspector->m_usergroup_manager.rm_user(container_id.data(), uid);
+		m_inspector->m_usergroup_manager.rm_user(std::string(container_id), uid);
 	}
 }
 
@@ -5647,15 +5637,15 @@ void sinsp_parser::parse_memfd_create_exit(sinsp_evt *evt, scap_fd_type type)
 	Suppose you create a memfd named libstest resulting in a fd.name libstest while on disk 
 	(e.g. ls -l /proc/$PID/fd/$FD_NUM) it may look like /memfd:libstest (deleted)
 	*/
-	std::string name = std::string(evt->get_param(1)->as<std::string_view>());
+	auto name = evt->get_param(1)->as<std::string_view>();
 
 	/* flags */
 	flags = evt->get_param(2)->as<uint32_t>();
 
 	if(fd >= 0)
 	{
 		fdi.m_type = type;
-		fdi.add_filename(name.c_str());
+		fdi.add_filename(name);
 		fdi.m_openflags = flags;
 	}
 
@@ -5689,12 +5679,7 @@ void sinsp_parser::parse_pidfd_open_exit(sinsp_evt *evt)
 	{
 		// note: approximating equivalent filename as in:
 		// https://man7.org/linux/man-pages/man2/pidfd_getfd.2.html
-		char fname[SCAP_MAX_PATH_SIZE];
-		snprintf(fname,
-		         sizeof(fname),
-		         "%s/proc/%lld",
-		         scap_get_host_root(),
-		         (long long) pid);
+		std::string fname = std::string(scap_get_host_root()) + "/proc/" + std::to_string(pid);
 		fdi.m_type = scap_fd_type::SCAP_FD_PIDFD;
 		fdi.add_filename(fname);
 		fdi.m_openflags = flags;

userspace/libsinsp/parsers.h

@@ -45,7 +45,7 @@ class sinsp_parser
 	//
 	// Combine the openat arguments into a full file name
 	//
-	static void parse_dirfd(sinsp_evt *evt, const char* name, int64_t dirfd, OUT std::string* sdir);
+	std::string parse_dirfd(sinsp_evt *evt, std::string_view name, int64_t dirfd);
 
 	void set_track_connection_status(bool enabled);
 	bool get_track_connection_status() { return m_track_connection_status; }

userspace/libsinsp/sinsp_filtercheck_fd.cpp

@@ -214,7 +214,6 @@ bool sinsp_filter_check_fd::extract_fdname_from_creator(sinsp_evt *evt, OUT uint
 		{
 			sinsp_evt enter_evt;
 			const sinsp_evt_param *parinfo;
-			std::string sdir;
 
 			if(etype == PPME_SYSCALL_OPENAT_X)
 			{
@@ -235,7 +234,7 @@ bool sinsp_filter_check_fd::extract_fdname_from_creator(sinsp_evt *evt, OUT uint
 			parinfo = etype == PPME_SYSCALL_OPENAT_X ? enter_evt.get_param(0) : evt->get_param(1);
 			int64_t dirfd = parinfo->as<int64_t>();
 
-			sinsp_parser::parse_dirfd(evt, name.data(), dirfd, &sdir);
+			std::string sdir = m_inspector->get_parser()->parse_dirfd(evt, name, dirfd);
 
 			if(fd_nameraw)
 			{

userspace/libsinsp/test/user.ut.cpp

@@ -89,7 +89,7 @@ TEST_F(usergroup_manager_test, system_lookup)
 
 	sinsp_usergroup_manager mgr(&m_inspector);
 
-	mgr.add_user(container_id, -1, 0, 0, nullptr, nullptr, nullptr);
+	mgr.add_user(container_id, -1, 0, 0, {}, {}, {});
 	auto* user = mgr.get_user(container_id, 0);
 	ASSERT_NE(user, nullptr);
 	ASSERT_EQ(user->uid, 0);
@@ -105,7 +105,7 @@ TEST_F(usergroup_manager_test, system_lookup)
 #endif
 	ASSERT_EQ(std::string(user->shell).empty(), false);
 
-	mgr.add_group(container_id, -1, 0, nullptr);
+	mgr.add_group(container_id, -1, 0, {});
 	auto* group = mgr.get_group(container_id, 0);
 	ASSERT_NE(group, nullptr);
 	ASSERT_EQ(group->gid, 0);
@@ -196,7 +196,7 @@ TEST_F(usergroup_manager_host_root_test, host_root_lookup)
 
 	sinsp_usergroup_manager mgr(&m_inspector);
 
-	mgr.add_user(container_id, -1, 0, 0, nullptr, nullptr, nullptr);
+	mgr.add_user(container_id, -1, 0, 0, {}, {}, {});
 	auto* user = mgr.get_user(container_id, 0);
 	ASSERT_NE(user, nullptr);
 	ASSERT_EQ(user->uid, 0);
@@ -205,7 +205,7 @@ TEST_F(usergroup_manager_host_root_test, host_root_lookup)
 	ASSERT_STREQ(user->homedir, "/toor");
 	ASSERT_STREQ(user->shell, "/bin/ash");
 
-	mgr.add_group(container_id, -1, 0, nullptr);
+	mgr.add_group(container_id, -1, 0, {});
 	auto* group = mgr.get_group(container_id, 0);
 	ASSERT_NE(group, nullptr);
 	ASSERT_EQ(group->gid, 0);

userspace/libsinsp/threadinfo.cpp

@@ -516,7 +516,7 @@ void sinsp_threadinfo::set_user(uint32_t uid)
 	if (!user)
 	{
 		auto notify = m_inspector->is_live() || m_inspector->is_syscall_plugin();
-		user = m_inspector->m_usergroup_manager.add_user(m_container_id, m_pid, uid, m_group.gid, NULL, NULL, NULL, notify);
+		user = m_inspector->m_usergroup_manager.add_user(m_container_id, m_pid, uid, m_group.gid, {}, {}, {}, notify);
 	}
 	if (user)
 	{
@@ -538,7 +538,7 @@ void sinsp_threadinfo::set_group(uint32_t gid)
 	if (!group)
 	{
 		auto notify = m_inspector->is_live() || m_inspector->is_syscall_plugin();
-		group = m_inspector->m_usergroup_manager.add_group(m_container_id, m_pid, gid, NULL, notify);
+		group = m_inspector->m_usergroup_manager.add_group(m_container_id, m_pid, gid, {}, notify);
 	}
 	if (group)
 	{