Fix pgid ns (#1080)

* Rename pgid to vpgid to reflect it's in-namespace

#1044 added tracking of a process's
process group id (pgid). However, this change didn't compeltely handle
process group ids in a namespace. When reading process information from
/proc, it was using the process group id from the global namespace. When
tracking execve()s and setpgid()s it would take the pgid from the
namespace of the calling process.

To fix this, define the pgid to be the pgid from its current pid
namespace. And to make this clear, rename it to vpgid everywhere.

This commit handles renaming the variable at the scap/sinsp
levels. Other changes will fix setpgid() handling to handle cases when
it's called in a pid namespace.

* Rename the proc.pgid filtercheck to proc.vpgid

This reflects its real meaning (from the namespace)

* Removing event parsing for setpgid

This means that if a setpgid occurs, the pgid of the process won't be
in-sync again until the next execve(), where the process's pgid is
returned by the driver in the exit event. However, we expect that this
will cover most cases, and is dramatically simpler.

* Fix logic for setting vpgid

Comment was right, but test was backwards.

Author: Mark Stemm

userspace/libscap/scap.h

@@ -206,7 +206,7 @@ typedef struct scap_threadinfo
 	uint64_t pid; ///< The id of the process containing this thread. In single thread processes, this is equal to tid.
 	uint64_t ptid; ///< The id of the thread that created this thread.
 	uint64_t sid; ///< The session id of the process containing this thread.
-	uint64_t pgid; ///< The process group of this thread.
+	uint64_t vpgid; ///< The process group of this thread, as seen from its current pid namespace
 	char comm[SCAP_MAX_PATH_SIZE+1]; ///< Command name (e.g. "top")
 	char exe[SCAP_MAX_PATH_SIZE+1]; ///< argv[0] (e.g. "sshd: user@pts/4")
 	char exepath[SCAP_MAX_PATH_SIZE+1]; ///< full executable path

userspace/libscap/scap_procs.c

@@ -67,6 +67,7 @@ int32_t scap_proc_fill_info_from_stats(char* procdirname, struct scap_threadinfo
 	uint64_t vtid;
 	int64_t sid;
 	int64_t pgid;
+	int64_t vpgid;
 	uint32_t vmsize_kb;
 	uint32_t vmrss_kb;
 	uint32_t vmswap_kb;
@@ -80,7 +81,7 @@ int32_t scap_proc_fill_info_from_stats(char* procdirname, struct scap_threadinfo
 	tinfo->uid = (uint32_t)-1;
 	tinfo->ptid = (uint32_t)-1LL;
 	tinfo->sid = 0;
-	tinfo->pgid = 0;
+	tinfo->vpgid = 0;
 	tinfo->vmsize_kb = 0;
 	tinfo->vmrss_kb = 0;
 	tinfo->vmswap_kb = 0;
@@ -190,6 +191,14 @@ int32_t scap_proc_fill_info_from_stats(char* procdirname, struct scap_threadinfo
 				tinfo->vtid = tinfo->tid;
 			}
 		}
+		else if(strstr(line, "NSpgid:") == line)
+		{
+			nfound++;
+			if(sscanf(line, "NSpgid: %*u %" PRIu64, &vpgid) == 1)
+			{
+				tinfo->vpgid = vpgid;
+			}
+		}
 		else if(strstr(line, "NStgid:") == line)
 		{
 			nfound++;
@@ -203,13 +212,13 @@ int32_t scap_proc_fill_info_from_stats(char* procdirname, struct scap_threadinfo
 			}
 		}
 
-		if(nfound == 8)
+		if(nfound == 9)
 		{
 			break;
 		}
 	}
 
-	ASSERT(nfound == 8 || nfound == 6 || nfound == 5);
+	ASSERT(nfound == 9 || nfound == 6 || nfound == 5);
 
 	fclose(f);
 
@@ -260,7 +269,14 @@ int32_t scap_proc_fill_info_from_stats(char* procdirname, struct scap_threadinfo
 	tinfo->pfmajor = pfmajor;
 	tinfo->pfminor = pfminor;
 	tinfo->sid = (uint64_t) sid;
-	tinfo->pgid = (uint64_t) pgid;
+
+	// If we did not find vpgid above, set it to pgid from the
+	// global namespace.
+	if(tinfo->vpgid == 0)
+	{
+		tinfo->vpgid = pgid;
+	}
+
 	tinfo->tty = tty;
 
 	fclose(f);

userspace/libscap/scap_savefile.c

@@ -290,7 +290,7 @@ int32_t scap_write_proclist_entry(scap_t *handle, scap_dumper_t *d, struct scap_
 		    scap_dump_write(d, &(tinfo->pid), sizeof(uint64_t)) != sizeof(uint64_t) ||
 		    scap_dump_write(d, &(tinfo->ptid), sizeof(uint64_t)) != sizeof(uint64_t) ||
 		    scap_dump_write(d, &(tinfo->sid), sizeof(uint64_t)) != sizeof(uint64_t) ||
-		    scap_dump_write(d, &(tinfo->pgid), sizeof(uint64_t)) != sizeof(uint64_t) ||
+		    scap_dump_write(d, &(tinfo->vpgid), sizeof(uint64_t)) != sizeof(uint64_t) ||
 		    scap_dump_write(d, &commlen, sizeof(uint16_t)) != sizeof(uint16_t) ||
 		    scap_dump_write(d, tinfo->comm, commlen) != commlen ||
 		    scap_dump_write(d, &exelen, sizeof(uint16_t)) != sizeof(uint16_t) ||
@@ -347,7 +347,7 @@ static int32_t scap_write_proclist(scap_t *handle, scap_dumper_t *d)
 				sizeof(uint64_t) +	// pid
 				sizeof(uint64_t) +	// ptid
 				sizeof(uint64_t) +	// sid
-				sizeof(uint64_t) +	// pgid
+				sizeof(uint64_t) +	// vpgid
 				2 + strnlen(tinfo->comm, SCAP_MAX_PATH_SIZE) +
 				2 + strnlen(tinfo->exe, SCAP_MAX_PATH_SIZE) +
 				2 + strnlen(tinfo->exepath, SCAP_MAX_PATH_SIZE) +
@@ -1074,7 +1074,7 @@ static int32_t scap_read_proclist(scap_t *handle, gzFile f, uint32_t block_lengt
 	tinfo.filtered_out = 0;
 	tinfo.root[0] = 0;
 	tinfo.sid = -1;
-	tinfo.pgid = -1;
+	tinfo.vpgid = -1;
 	tinfo.clone_ts = 0;
 	tinfo.tty = 0;
 	tinfo.exepath[0] = 0;
@@ -1131,7 +1131,7 @@ static int32_t scap_read_proclist(scap_t *handle, gzFile f, uint32_t block_lengt
 		}
 
 		//
-		// pgid
+		// vpgid
 		//
 		switch(block_type)
 		{
@@ -1147,7 +1147,7 @@ static int32_t scap_read_proclist(scap_t *handle, gzFile f, uint32_t block_lengt
 		case PL_BLOCK_TYPE_V7:
 			break;
 		case PL_BLOCK_TYPE_V8:
-			readsize = gzread(f, &(tinfo.pgid), sizeof(uint64_t));
+			readsize = gzread(f, &(tinfo.vpgid), sizeof(uint64_t));
 			CHECK_READ_SIZE(readsize, sizeof(uint64_t));
 
 			totreadsize += readsize;

userspace/libsinsp/filterchecks.cpp

@@ -1353,7 +1353,7 @@ const filtercheck_field_info sinsp_filter_check_thread_fields[] =
 	{PT_INT32, EPF_NONE, PF_ID, "proc.tty", "The controlling terminal of the process. 0 for processes without a terminal."},
 	{PT_CHARBUF, EPF_NONE, PF_NA, "proc.exepath", "The full executable path of the process."},
 	{PT_CHARBUF, EPF_TABLE_ONLY, PF_NA, "thread.nametid", "this field chains the process name and tid of a thread and can be used as a specific identifier of a thread for a specific execve."},
-	{PT_INT64, EPF_NONE, PF_ID, "proc.pgid", "the process group id of the process generating the event."},
+	{PT_INT64, EPF_NONE, PF_ID, "proc.vpgid", "the process group id of the process generating the event, as seen from its current PID namespace."},
 };
 
 sinsp_filter_check_thread::sinsp_filter_check_thread()
@@ -1629,8 +1629,8 @@ uint8_t* sinsp_filter_check_thread::extract(sinsp_evt *evt, OUT uint32_t* len, b
 		RETURN_EXTRACT_VAR(tinfo->m_pid);
 	case TYPE_SID:
 		RETURN_EXTRACT_VAR(tinfo->m_sid);
-	case TYPE_PGID:
-		RETURN_EXTRACT_VAR(tinfo->m_pgid);
+	case TYPE_VPGID:
+		RETURN_EXTRACT_VAR(tinfo->m_vpgid);
 	case TYPE_SNAME:
 		{
 			//

userspace/libsinsp/filterchecks.h

@@ -376,7 +376,7 @@ class sinsp_filter_check_thread : public sinsp_filter_check
 		TYPE_TTY = 42,
 		TYPE_EXEPATH = 43,
 		TYPE_NAMETID = 44,
-		TYPE_PGID = 45,
+		TYPE_VPGID = 45,
 	};
 
 	sinsp_filter_check_thread();

userspace/libsinsp/parsers.cpp

@@ -465,9 +465,6 @@ void sinsp_parser::process_event(sinsp_evt *evt)
 	case PPME_SYSCALL_SETSID_X:
 		parse_setsid_exit(evt);
 		break;
-	case PPME_SYSCALL_SETPGID_X:
-		parse_setpgid_exit(evt);
-		break;
 	default:
 		break;
 	}
@@ -1140,7 +1137,7 @@ void sinsp_parser::parse_clone_exit(sinsp_evt *evt)
 		tinfo.m_sid = ptinfo->m_sid;
 
 		// Copy the process group id from the parent
-		tinfo.m_pgid = ptinfo->m_pgid;
+		tinfo.m_vpgid = ptinfo->m_vpgid;
 
 		tinfo.m_tty = ptinfo->m_tty;
 	}
@@ -1176,7 +1173,7 @@ void sinsp_parser::parse_clone_exit(sinsp_evt *evt)
 			tinfo.m_args = ptinfo->m_args;
 			tinfo.m_root = ptinfo->m_root;
 			tinfo.m_sid = ptinfo->m_sid;
-			tinfo.m_pgid = ptinfo->m_pgid;
+			tinfo.m_vpgid = ptinfo->m_vpgid;
 			tinfo.m_tty = ptinfo->m_tty;
 		}
 		else
@@ -1715,10 +1712,10 @@ void sinsp_parser::parse_execve_exit(sinsp_evt *evt)
 	case PPME_SYSCALL_EXECVE_18_X:
 		break;
 	case PPME_SYSCALL_EXECVE_19_X:
-		// Get the pgid
+		// Get the vpgid
 		parinfo = evt->get_param(17);
 		ASSERT(parinfo->m_len == sizeof(int64_t));
-		evt->m_tinfo->m_pgid = *(int64_t *) parinfo->m_val;
+		evt->m_tinfo->m_vpgid = *(int64_t *) parinfo->m_val;
 		break;
 	default:
 		ASSERT(false);
@@ -4493,70 +4490,6 @@ void sinsp_parser::parse_setsid_exit(sinsp_evt *evt)
 	}
 }
 
-void sinsp_parser::parse_setpgid_exit(sinsp_evt *evt)
-{
-	sinsp_evt *enter_evt = &m_tmp_evt;
-	sinsp_evt_param *parinfo;
-	int64_t retval, pid, pgid;
-
-	//
-	// Extract the return value
-	//
-	parinfo = evt->get_param(0);
-	retval = *(int64_t *)parinfo->m_val;
-	ASSERT(parinfo->m_len == sizeof(int64_t));
-
-	if(retval >= 0 && retrieve_enter_event(enter_evt, evt))
-	{
-		//
-		// Extract the pid
-		//
-		parinfo = enter_evt->get_param(0);
-		pid = *(int64_t *)parinfo->m_val;
-		ASSERT(parinfo->m_len == sizeof(int64_t));
-
-		//
-		// Extract the pgid
-		//
-		parinfo = enter_evt->get_param(1);
-		pgid = *(int64_t *)parinfo->m_val;
-		ASSERT(parinfo->m_len == sizeof(int64_t));
-
-		//
-		// If pid is zero, then the process ID of the calling process is used.
-		//
-		sinsp_threadinfo* ptinfo = NULL;
-		if (pid == 0)
-		{
-			ptinfo = evt->get_thread_info();
-		}
-		else
-		{
-			ptinfo = m_inspector->get_thread(pid, false, true);
-		}
-
-		if(ptinfo == NULL)
-		{
-			ASSERT(false);
-			return;
-		}
-
-		//
-		// If pgid is zero, then the PGID of the process specified
-		// by pid is made the same as its process ID.
-		//
-		if (pgid == 0)
-		{
-			ptinfo->m_pgid = ptinfo->m_pid;
-		}
-		else
-		{
-			ptinfo->m_pgid = pgid;
-		}
-
-	}
-}
-
 void sinsp_parser::free_event_buffer(uint8_t *ptr)
 {
 	if(m_tmp_events_buffer.size() < m_inspector->m_thread_manager->m_threadtable.size())

userspace/libsinsp/parsers.h

@@ -141,7 +141,6 @@ class sinsp_parser
 #endif
 	void parse_chroot_exit(sinsp_evt *evt);
 	void parse_setsid_exit(sinsp_evt *evt);
-	void parse_setpgid_exit(sinsp_evt *evt);
 
 	inline void add_socket(sinsp_evt* evt, int64_t fd, uint32_t domain, uint32_t type, uint32_t protocol);
 	inline void add_pipe(sinsp_evt *evt, int64_t tid, int64_t fd, uint64_t ino);

userspace/libsinsp/threadinfo.cpp

@@ -59,7 +59,7 @@ void sinsp_threadinfo::init()
 {
 	m_pid = (uint64_t) - 1LL;
 	m_sid = (uint64_t) - 1LL;
-	m_pgid = (uint64_t) - 1LL;
+	m_vpgid = (uint64_t) - 1LL;
 	set_lastevent_data_validity(false);
 	m_lastevent_type = -1;
 	m_lastevent_ts = 0;
@@ -374,7 +374,7 @@ void sinsp_threadinfo::init(scap_threadinfo* pi)
 	m_pid = pi->pid;
 	m_ptid = pi->ptid;
 	m_sid = pi->sid;
-	m_pgid = pi->pgid;
+	m_vpgid = pi->vpgid;
 
 	m_comm = pi->comm;
 	m_exe = pi->exe;
@@ -1385,7 +1385,7 @@ void sinsp_thread_manager::thread_to_scap(sinsp_threadinfo& tinfo, 	scap_threadi
 	sctinfo->pid = tinfo.m_pid;
 	sctinfo->ptid = tinfo.m_ptid;
 	sctinfo->sid = tinfo.m_sid;
-	sctinfo->pgid = tinfo.m_pgid;
+	sctinfo->vpgid = tinfo.m_vpgid;
 
 	strncpy(sctinfo->comm, tinfo.m_comm.c_str(), SCAP_MAX_PATH_SIZE);
 	strncpy(sctinfo->exe, tinfo.m_exe.c_str(), SCAP_MAX_PATH_SIZE);

userspace/libsinsp/threadinfo.h

@@ -226,7 +226,6 @@ class SINSP_PUBLIC sinsp_threadinfo
 	int64_t m_pid; ///< The id of the process containing this thread. In single thread threads, this is equal to tid.
 	int64_t m_ptid; ///< The id of the process that started this thread.
 	int64_t m_sid; ///< The session id of the process containing this thread.
-	int64_t m_pgid; ///< The process group of this thread.
 	string m_comm; ///< Command name (e.g. "top")
 	string m_exe; ///< argv[0] (e.g. "sshd: user@pts/4")
 	string m_exepath; ///< full executable path
@@ -246,6 +245,7 @@ class SINSP_PUBLIC sinsp_threadinfo
 	uint64_t m_pfminor; ///< number of minor page faults since start.
 	int64_t m_vtid;  ///< The virtual id of this thread.
 	int64_t m_vpid; ///< The virtual id of the process containing this thread. In single thread threads, this is equal to vtid.
+	int64_t m_vpgid; // The virtual process group id, as seen from its pid namespace
 	string m_root;
 	size_t m_program_hash;
 	size_t m_program_hash_falco;