Reliably detect forks in containers

gnosek · gnosek · commit 3b7a794f8d8c · 2019-06-12T16:43:00.000+01:00
It's possible to have a process in a PID namespace that nevertheless
has tid == vtid (when the parent and nested PID counters happen to
overlap). In this case, we wrongly take the non-container path
while handling clone events and end up overwriting an unrelated
threadinfo (one that happened to have real pid == new child's vpid).

Fix by explicitly passing a flag meaning "the child process is in
a nested PID namespace" from the driver.
diff --git a/driver/bpf/fillers.h b/driver/bpf/fillers.h
@@ -1896,6 +1896,8 @@ FILLER(proc_startupdate_3, true)
 		kgid_t egid;
 		pid_t vtid;
 		pid_t vpid;
+		struct pid_namespace *pidns = bpf_task_active_pid_ns(task);
+		int pidns_level = _READ(pidns->level);
 
 		/*
 		 * flags
@@ -1907,6 +1909,18 @@ FILLER(proc_startupdate_3, true)
 
 		flags = clone_flags_to_scap(flags);
 
+		if(pidns_level != 0) {
+			flags |= PPM_CL_CHILD_IN_PIDNS;
+		} else {
+			struct nsproxy *nsproxy = _READ(task->nsproxy);
+			if(nsproxy) {
+				struct pid_namespace *pid_ns_for_children = _READ(nsproxy->pid_ns_for_children);
+				if(pid_ns_for_children != pidns) {
+					flags |= PPM_CL_CHILD_IN_PIDNS;
+				}
+			}
+		}
+
 		res = bpf_val_to_ring_type(data, flags, PT_FLAGS32);
 		if (res != PPM_SUCCESS)
 			return res;
diff --git a/driver/ppm_events_public.h b/driver/ppm_events_public.h
@@ -163,6 +163,8 @@ or GPL2.txt for full copies of the license.
 #define PPM_CL_CLONE_STOPPED (1 << 26)
 #define PPM_CL_CLONE_VFORK (1 << 27)
 #define PPM_CL_CLONE_NEWCGROUP (1 << 28)
+#define PPM_CL_CHILD_IN_PIDNS (1<<29)			/* true if the thread created by clone() is *not*
+									in the init pid namespace */
 
 /*
  * Futex Operations
diff --git a/driver/ppm_fillers.c b/driver/ppm_fillers.c
@@ -931,6 +931,10 @@ int f_proc_startupdate(struct event_filler_arguments *args)
 		uint64_t euid = current->euid;
 		uint64_t egid = current->egid;
 #endif
+		int64_t in_pidns = 0;
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2, 6, 20)
+		struct pid_namespace *pidns = task_active_pid_ns(current);
+#endif
 
 		/*
 		 * flags
@@ -944,7 +948,11 @@ int f_proc_startupdate(struct event_filler_arguments *args)
 		} else
 			val = 0;
 
-		res = val_to_ring(args, (uint64_t)clone_flags_to_scap(val), 0, false, 0);
+#if LINUX_VERSION_CODE > KERNEL_VERSION(2, 6, 20)
+		if(pidns != &init_pid_ns || current->nsproxy->pid_ns_for_children != pidns)
+			in_pidns = PPM_CL_CHILD_IN_PIDNS;
+#endif
+		res = val_to_ring(args, (uint64_t)clone_flags_to_scap(val) | in_pidns, 0, false, 0);
 		if (unlikely(res != PPM_SUCCESS))
 			return res;
 
diff --git a/userspace/libsinsp/parsers.cpp b/userspace/libsinsp/parsers.cpp
@@ -983,7 +983,8 @@ void sinsp_parser::parse_clone_exit(sinsp_evt *evt)
 		ASSERT(false);
 	}
 
-	if(tid != vtid)
+	// the flag check should always suffice but leave the tid/vtid check for older driver versions
+	if(flags & PPM_CL_CHILD_IN_PIDNS || tid != vtid)
 	{
 		in_container = true;
 	}

Original file line number	Diff line number	Diff line change
`@@ -983,7 +983,8 @@ void sinsp_parser::parse_clone_exit(sinsp_evt *evt)`
`983`	`983`	`ASSERT(false);`
`984`	`984`	`}`
`985`	`985`
`986`		`- if(tid != vtid)`
	`986`	`+ // the flag check should always suffice but leave the tid/vtid check for older driver versions`
	`987`	`+ if(flags & PPM_CL_CHILD_IN_PIDNS \|\| tid != vtid)`
`987`	`988`	`{`
`988`	`989`	`in_container = true;`
`989`	`990`	`}`