[PATCH bpf-next 0/3] bpf: add progenyof helper

[PATCH bpf-next 0/3] bpf: add progenyof helper

[Javier Honduvilla Coto]

Hi all,

This patch add the bpf_progenyof helper which receives a PID and returns
1 if the process currently being executed is in the process hierarchy,
including itself or 0 if not.

This is very useful in tracing programs when we want to filter by a
given PID and all the children it might have. The current workarounds
most people implement for this purpose have issues:

- Attaching to process spawning syscalls and dynamically add those PIDs
  to  some bpf map that would be used to filter is cumbersome and
potentially racy.
- Unrolling some loop to perform what this helper is doing consumes lots
  of instructions. That and the impossibility to jump backwards makes it
really hard to be correct in really large process chains.

Let me know what do you think!

Thanks,

Javier Honduvilla Coto (3):
  bpf: add bpf_progenyof helper
  bpf: sync kernel uapi headers
  bpf: add tests for bpf_progenyof

 include/linux/bpf.h                           |   1 +
 include/uapi/linux/bpf.h                      |   3 +-
 kernel/bpf/core.c                             |   1 +
 kernel/bpf/helpers.c                          |  29 ++
 kernel/trace/bpf_trace.c                      |   2 +
 tools/include/uapi/linux/bpf.h                |  11 +-
 tools/testing/selftests/bpf/.gitignore        |   1 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   1 +
 .../selftests/bpf/progs/test_progenyof_kern.c |  46 ++++
 .../selftests/bpf/test_progenyof_user.c       | 249 ++++++++++++++++++
 11 files changed, 343 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_progenyof_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_progenyof_user.c

-- 
2.17.1

[PATCH bpf-next 1/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

This patch adds the bpf_progenyof helper which receives a PID and returns
1 if the process currently being executed is in the process hierarchy
including itself or 0 if not.

This is very useful in tracing programs when we want to filter by a
given PID and all the children it might spawn. The current workarounds
most people implement for this purpose have issues:

- Attaching to process spawning syscalls and dynamically add those PIDs
  to some bpf map that would be used to filter is cumbersome and
potentially racy.
- Unrolling some loop to perform what this helper is doing consumes lots
  of instructions. That and the impossibility to jump backwards makes it
really hard to be correct in really large process chains.

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 include/linux/bpf.h      |  1 +
 include/uapi/linux/bpf.h |  3 ++-
 kernel/bpf/core.c        |  1 +
 kernel/bpf/helpers.c     | 29 +++++++++++++++++++++++++++++
 kernel/trace/bpf_trace.c |  2 ++
 5 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index de18227b3d95..447395ba202b 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -921,6 +921,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
 extern const struct bpf_func_proto bpf_spin_lock_proto;
 extern const struct bpf_func_proto bpf_spin_unlock_proto;
 extern const struct bpf_func_proto bpf_get_local_storage_proto;
+extern const struct bpf_func_proto bpf_progenyof_proto;
 
 /* Shared helpers among cBPF and eBPF. */
 void bpf_user_rnd_init_once(void);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index bcdd2474eee7..804e4218eb28 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2457,7 +2457,8 @@ union bpf_attr {
 	FN(spin_lock),			\
 	FN(spin_unlock),		\
 	FN(sk_fullsock),		\
-	FN(tcp_sock),
+	FN(tcp_sock),			\
+	FN(progenyof),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ef88b167959d..69e209fbd128 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2015,6 +2015,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
 const struct bpf_func_proto bpf_get_current_comm_proto __weak;
 const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
 const struct bpf_func_proto bpf_get_local_storage_proto __weak;
+const struct bpf_func_proto bpf_progenyof_proto __weak;
 
 const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
 {
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index a411fc17d265..3899787e8dbf 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -18,6 +18,7 @@
 #include <linux/sched.h>
 #include <linux/uidgid.h>
 #include <linux/filter.h>
+#include <linux/init_task.h>
 
 /* If kernel subsystem is allowing eBPF programs to call this function,
  * inside its own verifier_ops->get_func_proto() callback it should return
@@ -364,3 +365,31 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
 };
 #endif
 #endif
+
+BPF_CALL_1(bpf_progenyof, int, pid)
+{
+	int result = 0;
+	struct task_struct *task = current;
+
+	if (unlikely(!task))
+		return -EINVAL;
+
+	rcu_read_lock();
+	while (task != &init_task) {
+		if (task->pid == pid) {
+			result = 1;
+			break;
+		}
+		task = rcu_dereference(task->real_parent);
+	}
+	rcu_read_unlock();
+
+	return result;
+}
+
+const struct bpf_func_proto bpf_progenyof_proto = {
+	.func		= bpf_progenyof,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_ANYTHING,
+};
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index f1a86a0d881d..8602ae83c799 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -600,6 +600,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_prandom_u32_proto;
 	case BPF_FUNC_probe_read_str:
 		return &bpf_probe_read_str_proto;
+	case BPF_FUNC_progenyof:
+		return &bpf_progenyof_proto;
 #ifdef CONFIG_CGROUPS
 	case BPF_FUNC_get_current_cgroup_id:
 		return &bpf_get_current_cgroup_id_proto;
-- 
2.17.1

[PATCH bpf-next 2/3] bpf: sync kernel uapi headers

[Javier Honduvilla Coto]

Sync kernel uapi headers.

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 tools/include/uapi/linux/bpf.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index bcdd2474eee7..354ec295864c 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2359,6 +2359,14 @@ union bpf_attr {
  *	Return
  *		A **struct bpf_tcp_sock** pointer on success, or NULL in
  *		case of failure.
+ *
+ * int bpf_progenyof(int pid)
+ *	Description
+ *		This helper is useful in programs that want to filter events
+ *		happening to a pid of any of its descendants.
+ *	Return
+ *		1 if the currently executing process' pid is in the process
+ *		hierarchy of the passed pid. 0 Otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2457,7 +2465,8 @@ union bpf_attr {
 	FN(spin_lock),			\
 	FN(spin_unlock),		\
 	FN(sk_fullsock),		\
-	FN(tcp_sock),
+	FN(tcp_sock),			\
+	FN(progenyof),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
-- 
2.17.1

[PATCH bpf-next 3/3] bpf: add tests for bpf_progenyof

[Javier Honduvilla Coto]

Adding the following test cases:

- progenyof(current->pid) == 1
- progenyof(current->real_parent->pid) == 1
- progenyof(1) == 1
- progenyof(0) == 0
- progenyof(current->children[0]->pid) == 0

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 tools/testing/selftests/bpf/.gitignore        |   1 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   1 +
 .../selftests/bpf/progs/test_progenyof_kern.c |  46 ++++
 .../selftests/bpf/test_progenyof_user.c       | 249 ++++++++++++++++++
 5 files changed, 298 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_progenyof_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_progenyof_user.c

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index e47168d1257d..677be3b05cd2 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -30,3 +30,4 @@ test_section_names
 test_tcpnotify_user
 test_libbpf
 alu32
+test_progenyof_user
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index ccffaa0a0787..8a8d4b2a5b74 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -23,7 +23,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
 	test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \
 	test_sock test_btf test_sockmap test_lirc_mode2_user get_cgroup_id_user \
 	test_socket_cookie test_cgroup_storage test_select_reuseport test_section_names \
-	test_netcnt test_tcpnotify_user test_sock_fields
+	test_netcnt test_tcpnotify_user test_sock_fields test_progenyof_user
 
 BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
 TEST_GEN_FILES = $(BPF_OBJ_FILES)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index d9999f1ed1d2..1f3e0d1ad159 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -180,6 +180,7 @@ static struct bpf_sock *(*bpf_sk_fullsock)(struct bpf_sock *sk) =
 	(void *) BPF_FUNC_sk_fullsock;
 static struct bpf_tcp_sock *(*bpf_tcp_sock)(struct bpf_sock *sk) =
 	(void *) BPF_FUNC_tcp_sock;
+static int (*bpf_progenyof)(int pid) = (void *) BPF_FUNC_progenyof;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions
diff --git a/tools/testing/selftests/bpf/progs/test_progenyof_kern.c b/tools/testing/selftests/bpf/progs/test_progenyof_kern.c
new file mode 100644
index 000000000000..c4be2c794d83
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_progenyof_kern.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct bpf_map_def SEC("maps") pidmap = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(__u32),
+	.value_size = sizeof(__u32),
+	.max_entries = 2,
+};
+
+struct bpf_map_def SEC("maps") resultmap = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(__u32),
+	.value_size = sizeof(__u32),
+	.max_entries = 1,
+};
+
+SEC("tracepoint/syscalls/sys_enter_open")
+int trace(void *ctx)
+{
+	__u32 pid = bpf_get_current_pid_tgid();
+	__u32 current_key = 0, ancestor_key = 1, *expected_pid, *ancestor_pid;
+	__u32 *val;
+
+	expected_pid = bpf_map_lookup_elem(&pidmap, &current_key);
+	if (!expected_pid || *expected_pid != pid)
+		return 0;
+
+	ancestor_pid = bpf_map_lookup_elem(&pidmap, &ancestor_key);
+	if (!ancestor_pid)
+		return 0;
+
+	if (!bpf_progenyof(*ancestor_pid))
+		return 0;
+
+	val = bpf_map_lookup_elem(&resultmap, &current_key);
+	if (val)
+		*val = *ancestor_pid;
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
diff --git a/tools/testing/selftests/bpf/test_progenyof_user.c b/tools/testing/selftests/bpf/test_progenyof_user.c
new file mode 100644
index 000000000000..0f9572986a11
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_progenyof_user.c
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <syscall.h>
+#include <unistd.h>
+#include <linux/perf_event.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#define CHECK(condition, tag, format...)                                       \
+	({                                                                     \
+		int __ret = !!(condition);                                     \
+		if (__ret) {                                                   \
+			printf("%s:FAIL:%s ", __func__, tag);                  \
+			printf(format);                                        \
+		} else {                                                       \
+			printf("%s:PASS:%s\n", __func__, tag);                 \
+		}                                                              \
+		__ret;                                                         \
+	})
+
+static int bpf_find_map(const char *test, struct bpf_object *obj,
+			const char *name)
+{
+	struct bpf_map *map;
+
+	map = bpf_object__find_map_by_name(obj, name);
+	if (!map)
+		return -1;
+	return bpf_map__fd(map);
+}
+
+int main(int argc, char **argv)
+{
+	const char *probe_name = "syscalls/sys_enter_open";
+	const char *file = "test_progenyof_kern.o";
+	int err, bytes, efd, prog_fd, pmu_fd;
+	int resultmap_fd, pidmap_fd;
+	struct perf_event_attr attr = {};
+	struct bpf_object *obj;
+	__u32 retrieved_ancestor_pid = 0;
+	__u32 key = 0, pid;
+	int exit_code = EXIT_FAILURE;
+	char buf[256];
+
+	int child_pid, ancestor_pid, root_fd;
+	__u32 ancestor_key = 1;
+	int pipefd[2];
+	char marker[1];
+
+	err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
+	if (CHECK(err, "bpf_prog_load", "err %d errno %d\n", err, errno))
+		goto fail;
+
+	resultmap_fd = bpf_find_map(__func__, obj, "resultmap");
+	if (CHECK(resultmap_fd < 0, "bpf_find_map", "err %d errno %d\n",
+		  resultmap_fd, errno))
+		goto close_prog;
+
+	pidmap_fd = bpf_find_map(__func__, obj, "pidmap");
+	if (CHECK(pidmap_fd < 0, "bpf_find_map", "err %d errno %d\n", pidmap_fd,
+		  errno))
+		goto close_prog;
+
+	pid = getpid();
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+
+	snprintf(buf, sizeof(buf), "/sys/kernel/debug/tracing/events/%s/id",
+		 probe_name);
+	efd = open(buf, O_RDONLY, 0);
+	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+		goto close_prog;
+	bytes = read(efd, buf, sizeof(buf));
+	close(efd);
+	if (CHECK(bytes <= 0 || bytes >= sizeof(buf), "read",
+		  "bytes %d errno %d\n", bytes, errno))
+		goto close_prog;
+
+	attr.config = strtol(buf, NULL, 0);
+	attr.type = PERF_TYPE_TRACEPOINT;
+	attr.sample_type = PERF_SAMPLE_RAW;
+	attr.sample_period = 1;
+	attr.wakeup_events = 1;
+
+	pmu_fd = syscall(__NR_perf_event_open, &attr, getpid(), -1, -1, 0);
+	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n", pmu_fd,
+		  errno))
+		goto close_prog;
+
+	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
+	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n", err,
+		  errno))
+		goto close_pmu;
+
+	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
+	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n", err,
+		  errno))
+		goto close_pmu;
+
+	// Test on itself: progenyof(current->pid) is true
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &key, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != pid,
+		  "progenyof is true with same pid", "%d == %d\n",
+		  retrieved_ancestor_pid, pid))
+		goto close_pmu;
+
+	// Test that PID 1 is among our progeny
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = 1;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &key, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != ancestor_pid,
+		  "progenyof reaches init", "%d == %d\n",
+		  retrieved_ancestor_pid, ancestor_pid))
+		goto close_pmu;
+
+	// Test we don't go over PID 1
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = 0;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &key, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != 0,
+		  "progenyof does not go over init", "%d == %d\n",
+		  retrieved_ancestor_pid, 0))
+		goto close_pmu;
+
+	// Test that we are the progeny of our child
+	pipe(pipefd);
+	child_pid = fork();
+	if (child_pid == -1) {
+		printf("fork failed\n");
+		goto close_pmu;
+	} else if (child_pid == 0) {
+		close(pipefd[1]);
+		read(pipefd[0], &marker, 1);
+
+		root_fd = open("/", O_RDONLY);
+		if (CHECK(efd < 0, "open", "errno %d\n", errno))
+			goto close_prog;
+		close(root_fd);
+
+		close(pipefd[0]);
+		_exit(EXIT_SUCCESS);
+	} else {
+		close(pipefd[0]);
+		bpf_map_update_elem(resultmap_fd, &key, &key, 0);
+		bpf_map_update_elem(pidmap_fd, &key, &child_pid, 0);
+		bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+
+		write(pipefd[1], &marker, 1);
+		wait(NULL);
+		close(pipefd[1]);
+
+		err = bpf_map_lookup_elem(resultmap_fd, &key,
+					  &retrieved_ancestor_pid);
+		if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err,
+			  errno))
+			goto close_pmu;
+		if (CHECK(retrieved_ancestor_pid != pid, "progenyof of parent",
+			  "%d == %d\n", retrieved_ancestor_pid, pid))
+			goto close_pmu;
+	}
+
+	// Test that a child of ours doesn't belong to our progeny
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &key, 0);
+
+	pipe(pipefd);
+	child_pid = fork();
+	if (child_pid == -1) {
+		printf("fork failed\n");
+		goto close_pmu;
+	} else if (child_pid == 0) {
+		close(pipefd[1]);
+		read(pipefd[0], marker, 1);
+		close(pipefd[0]);
+		_exit(EXIT_SUCCESS);
+	} else {
+		close(pipefd[0]);
+
+		bpf_map_update_elem(pidmap_fd, &ancestor_key, &child_pid, 0);
+
+		root_fd = open("/", O_RDONLY);
+		if (CHECK(efd < 0, "open", "errno %d\n", errno))
+			goto close_prog;
+		close(root_fd);
+
+		write(pipefd[1], marker, 1);
+		wait(NULL);
+		close(pipefd[1]);
+
+		err = bpf_map_lookup_elem(resultmap_fd, &key,
+					  &retrieved_ancestor_pid);
+		if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err,
+			  errno))
+			goto close_pmu;
+		if (CHECK(retrieved_ancestor_pid != 0, "progenyof of child",
+			  "%d == %d\n", retrieved_ancestor_pid, 0))
+			goto close_pmu;
+	}
+
+	exit_code = EXIT_SUCCESS;
+	printf("%s:PASS\n", argv[0]);
+
+close_pmu:
+	close(pmu_fd);
+close_prog:
+	bpf_object__close(obj);
+fail:
+	return exit_code;
+}
-- 
2.17.1

Re: [PATCH bpf-next 1/3] bpf: add bpf_progenyof helper

[Martin Lau]

On Tue, Feb 26, 2019 at 02:36:49PM -0800, Javier Honduvilla Coto wrote:
> This patch adds the bpf_progenyof helper which receives a PID and returns
What is progenof?

> 1 if the process currently being executed is in the process hierarchy
> including itself or 0 if not.
> 
> This is very useful in tracing programs when we want to filter by a
> given PID and all the children it might spawn. The current workarounds
> most people implement for this purpose have issues:
> 
> - Attaching to process spawning syscalls and dynamically add those PIDs
>   to some bpf map that would be used to filter is cumbersome and
> potentially racy.
> - Unrolling some loop to perform what this helper is doing consumes lots
>   of instructions. That and the impossibility to jump backwards makes it
> really hard to be correct in really large process chains.
> 
> Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> ---
>  include/linux/bpf.h      |  1 +
>  include/uapi/linux/bpf.h |  3 ++-
>  kernel/bpf/core.c        |  1 +
>  kernel/bpf/helpers.c     | 29 +++++++++++++++++++++++++++++
>  kernel/trace/bpf_trace.c |  2 ++
>  5 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index de18227b3d95..447395ba202b 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -921,6 +921,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
>  extern const struct bpf_func_proto bpf_spin_lock_proto;
>  extern const struct bpf_func_proto bpf_spin_unlock_proto;
>  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> +extern const struct bpf_func_proto bpf_progenyof_proto;
It seems only used in bpf_trace.c.  Does it have to be here?

>  
>  /* Shared helpers among cBPF and eBPF. */
>  void bpf_user_rnd_init_once(void);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index bcdd2474eee7..804e4218eb28 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2457,7 +2457,8 @@ union bpf_attr {
>  	FN(spin_lock),			\
>  	FN(spin_unlock),		\
>  	FN(sk_fullsock),		\
> -	FN(tcp_sock),
> +	FN(tcp_sock),			\
> +	FN(progenyof),
Please add doc like other helpers do.

>  
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index ef88b167959d..69e209fbd128 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2015,6 +2015,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
>  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
>  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
>  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> +const struct bpf_func_proto bpf_progenyof_proto __weak;
>  
>  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
>  {
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index a411fc17d265..3899787e8dbf 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -18,6 +18,7 @@
>  #include <linux/sched.h>
>  #include <linux/uidgid.h>
>  #include <linux/filter.h>
> +#include <linux/init_task.h>
>  
>  /* If kernel subsystem is allowing eBPF programs to call this function,
>   * inside its own verifier_ops->get_func_proto() callback it should return
> @@ -364,3 +365,31 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
>  };
>  #endif
>  #endif
> +
> +BPF_CALL_1(bpf_progenyof, int, pid)
> +{
> +	int result = 0;
> +	struct task_struct *task = current;
> +
> +	if (unlikely(!task))
hmm.... Could current be NULL?

> +		return -EINVAL;
> +
> +	rcu_read_lock();
> +	while (task != &init_task) {
I don't know the details of init_task, so qq:
Could the passed in "pid" be the init_task->pid?
If possible, what is the expected "result"?

> +		if (task->pid == pid) {
> +			result = 1;
> +			break;
> +		}
> +		task = rcu_dereference(task->real_parent);
> +	}
> +	rcu_read_unlock();
> +
> +	return result;
> +}
> +
> +const struct bpf_func_proto bpf_progenyof_proto = {
> +	.func		= bpf_progenyof,
> +	.gpl_only	= false,
> +	.ret_type	= RET_INTEGER,
> +	.arg1_type	= ARG_ANYTHING,
> +};
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index f1a86a0d881d..8602ae83c799 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -600,6 +600,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		return &bpf_get_prandom_u32_proto;
>  	case BPF_FUNC_probe_read_str:
>  		return &bpf_probe_read_str_proto;
> +	case BPF_FUNC_progenyof:
> +		return &bpf_progenyof_proto;
>  #ifdef CONFIG_CGROUPS
>  	case BPF_FUNC_get_current_cgroup_id:
>  		return &bpf_get_current_cgroup_id_proto;
> -- 
> 2.17.1
> 

Re: [PATCH bpf-next 1/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

On Wed, Feb 27, 2019 at 06:26:41AM +0000, Martin Lau wrote:
> On Tue, Feb 26, 2019 at 02:36:49PM -0800, Javier Honduvilla Coto wrote:
> > This patch adds the bpf_progenyof helper which receives a PID and returns
> What is progenof?
>
> > 1 if the process currently being executed is in the process hierarchy
> > including itself or 0 if not.
> >
> > This is very useful in tracing programs when we want to filter by a
> > given PID and all the children it might spawn. The current workarounds
> > most people implement for this purpose have issues:
> >
> > - Attaching to process spawning syscalls and dynamically add those PIDs
> >   to some bpf map that would be used to filter is cumbersome and
> > potentially racy.
> > - Unrolling some loop to perform what this helper is doing consumes lots
> >   of instructions. That and the impossibility to jump backwards makes it
> > really hard to be correct in really large process chains.
> >
> > Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> > ---
> >  include/linux/bpf.h      |  1 +
> >  include/uapi/linux/bpf.h |  3 ++-
> >  kernel/bpf/core.c        |  1 +
> >  kernel/bpf/helpers.c     | 29 +++++++++++++++++++++++++++++
> >  kernel/trace/bpf_trace.c |  2 ++
> >  5 files changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index de18227b3d95..447395ba202b 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -921,6 +921,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
> >  extern const struct bpf_func_proto bpf_spin_lock_proto;
> >  extern const struct bpf_func_proto bpf_spin_unlock_proto;
> >  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> > +extern const struct bpf_func_proto bpf_progenyof_proto;
> It seems only used in bpf_trace.c.  Does it have to be here?
>
> >
> >  /* Shared helpers among cBPF and eBPF. */
> >  void bpf_user_rnd_init_once(void);
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index bcdd2474eee7..804e4218eb28 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -2457,7 +2457,8 @@ union bpf_attr {
> >  	FN(spin_lock),			\
> >  	FN(spin_unlock),		\
> >  	FN(sk_fullsock),		\
> > -	FN(tcp_sock),
> > +	FN(tcp_sock),			\
> > +	FN(progenyof),
> Please add doc like other helpers do.

Oops, good catch, thanks! Will send v2 soon!!

>
> >
> >  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> >   * function eBPF program intends to call
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index ef88b167959d..69e209fbd128 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -2015,6 +2015,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
> >  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
> >  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
> >  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> > +const struct bpf_func_proto bpf_progenyof_proto __weak;
> >
> >  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
> >  {
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index a411fc17d265..3899787e8dbf 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/sched.h>
> >  #include <linux/uidgid.h>
> >  #include <linux/filter.h>
> > +#include <linux/init_task.h>
> >
> >  /* If kernel subsystem is allowing eBPF programs to call this function,
> >   * inside its own verifier_ops->get_func_proto() callback it should return
> > @@ -364,3 +365,31 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
> >  };
> >  #endif
> >  #endif
> > +
> > +BPF_CALL_1(bpf_progenyof, int, pid)
> > +{
> > +	int result = 0;
> > +	struct task_struct *task = current;
> > +
> > +	if (unlikely(!task))
> hmm.... Could current be NULL?

Wasn't sure about this but added as bpf_get_current_pid_tgid,
bpf_get_current_uid_gid, and bpf_get_current_comm check for this. Texted Alexei
about this and he told me this is probably not necessary anymore, but I
guess it doesn't hurt leaving it?

>
> > +		return -EINVAL;
> > +
> > +	rcu_read_lock();
> > +	while (task != &init_task) {
> I don't know the details of init_task, so qq:
> Could the passed in "pid" be the init_task->pid?
> If possible, what is the expected "result"?
>

Yep! init_task doesn't set a pid for what I could see, so I guess it
will be PID=0. The test in the last patch check bpf_progenyof(0) :)

bpf_progenyof with 0 or 1 will always return 1

> > +		if (task->pid == pid) {
> > +			result = 1;
> > +			break;
> > +		}
> > +		task = rcu_dereference(task->real_parent);
> > +	}
> > +	rcu_read_unlock();
> > +
> > +	return result;
> > +}
> > +
> > +const struct bpf_func_proto bpf_progenyof_proto = {
> > +	.func		= bpf_progenyof,
> > +	.gpl_only	= false,
> > +	.ret_type	= RET_INTEGER,
> > +	.arg1_type	= ARG_ANYTHING,
> > +};
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index f1a86a0d881d..8602ae83c799 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -600,6 +600,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >  		return &bpf_get_prandom_u32_proto;
> >  	case BPF_FUNC_probe_read_str:
> >  		return &bpf_probe_read_str_proto;
> > +	case BPF_FUNC_progenyof:
> > +		return &bpf_progenyof_proto;
> >  #ifdef CONFIG_CGROUPS
> >  	case BPF_FUNC_get_current_cgroup_id:
> >  		return &bpf_get_current_cgroup_id_proto;
> > --
> > 2.17.1
> >

Re: [PATCH bpf-next 1/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

On Wed, Feb 27, 2019 at 06:26:41AM +0000, Martin Lau wrote:

Sorry, accidentally did not reply to all your comments:
> On Tue, Feb 26, 2019 at 02:36:49PM -0800, Javier Honduvilla Coto wrote:
> > This patch adds the bpf_progenyof helper which receives a PID and returns
> What is progenof?

progenyof is a helper we'd love to have in thr kernel as right now
checking if the current process pid is among the progeny of some
arbitrary pid has several disadvantages. There are more details in the
first commit message

>
> > 1 if the process currently being executed is in the process hierarchy
> > including itself or 0 if not.
> >
> > This is very useful in tracing programs when we want to filter by a
> > given PID and all the children it might spawn. The current workarounds
> > most people implement for this purpose have issues:
> >
> > - Attaching to process spawning syscalls and dynamically add those PIDs
> >   to some bpf map that would be used to filter is cumbersome and
> > potentially racy.
> > - Unrolling some loop to perform what this helper is doing consumes lots
> >   of instructions. That and the impossibility to jump backwards makes it
> > really hard to be correct in really large process chains.
> >
> > Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> > ---
> >  include/linux/bpf.h      |  1 +
> >  include/uapi/linux/bpf.h |  3 ++-
> >  kernel/bpf/core.c        |  1 +
> >  kernel/bpf/helpers.c     | 29 +++++++++++++++++++++++++++++
> >  kernel/trace/bpf_trace.c |  2 ++
> >  5 files changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index de18227b3d95..447395ba202b 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -921,6 +921,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
> >  extern const struct bpf_func_proto bpf_spin_lock_proto;
> >  extern const struct bpf_func_proto bpf_spin_unlock_proto;
> >  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> > +extern const struct bpf_func_proto bpf_progenyof_proto;
> It seems only used in bpf_trace.c.  Does it have to be here?

Yes, we need it in bpf_trace.c where we are requiring it

[PATCH v2 bpf-next 0/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

Hi all,

This patch add the bpf_progenyof helper which receives a PID and returns
1 if the process currently being executed is in the process hierarchy,
including itself or 0 if not.

This is very useful in tracing programs when we want to filter by a
given PID and all the children it might have. The current workarounds
most people implement for this purpose have issues:

- Attaching to process spawning syscalls and dynamically add those PIDs
  to  some bpf map that would be used to filter is cumbersome and
potentially racy.
- Unrolling some loop to perform what this helper is doing consumes lots
  of instructions. That and the impossibility to jump backwards makes it
really hard to be correct in really large process chains.

Let me know what do you think!

Thanks,

---

Changed in V2:
        - Adding missing docs in include/uapi/linux/bpf.h

Javier Honduvilla Coto (3):
  bpf: add bpf_progenyof helper
  bpf: sync kernel uapi headers
  bpf: add tests for bpf_progenyof

 include/linux/bpf.h                           |   1 +
 include/uapi/linux/bpf.h                      |  11 +-
 kernel/bpf/core.c                             |   1 +
 kernel/bpf/helpers.c                          |  29 ++
 kernel/trace/bpf_trace.c                      |   2 +
 tools/include/uapi/linux/bpf.h                |  11 +-
 tools/testing/selftests/bpf/.gitignore        |   1 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   1 +
 .../selftests/bpf/progs/test_progenyof_kern.c |  46 ++++
 .../selftests/bpf/test_progenyof_user.c       | 249 ++++++++++++++++++
 11 files changed, 351 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_progenyof_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_progenyof_user.c

-- 
2.17.1

[PATCH v2 bpf-next 1/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

From: Javier Honduvilla Coto <javierhonduco@gmail.com>

This patch adds the bpf_progenyof helper which receives a PID and returns
1 if the process currently being executed is in the process hierarchy
including itself or 0 if not.

This is very useful in tracing programs when we want to filter by a
given PID and all the children it might spawn. The current workarounds
most people implement for this purpose have issues:

- Attaching to process spawning syscalls and dynamically add those PIDs
  to some bpf map that would be used to filter is cumbersome and
potentially racy.
- Unrolling some loop to perform what this helper is doing consumes lots
  of instructions. That and the impossibility to jump backwards makes it
really hard to be correct in really large process chains.

Signed-off-by: Javier Honduvilla Coto <javierhonduco@gmail.com>
---
 include/linux/bpf.h      |  1 +
 include/uapi/linux/bpf.h | 11 ++++++++++-
 kernel/bpf/core.c        |  1 +
 kernel/bpf/helpers.c     | 29 +++++++++++++++++++++++++++++
 kernel/trace/bpf_trace.c |  2 ++
 5 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index de18227b3d95..447395ba202b 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -921,6 +921,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
 extern const struct bpf_func_proto bpf_spin_lock_proto;
 extern const struct bpf_func_proto bpf_spin_unlock_proto;
 extern const struct bpf_func_proto bpf_get_local_storage_proto;
+extern const struct bpf_func_proto bpf_progenyof_proto;
 
 /* Shared helpers among cBPF and eBPF. */
 void bpf_user_rnd_init_once(void);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index bcdd2474eee7..354ec295864c 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2359,6 +2359,14 @@ union bpf_attr {
  *	Return
  *		A **struct bpf_tcp_sock** pointer on success, or NULL in
  *		case of failure.
+ *
+ * int bpf_progenyof(int pid)
+ *	Description
+ *		This helper is useful in programs that want to filter events
+ *		happening to a pid of any of its descendants.
+ *	Return
+ *		1 if the currently executing process' pid is in the process
+ *		hierarchy of the passed pid. 0 Otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2457,7 +2465,8 @@ union bpf_attr {
 	FN(spin_lock),			\
 	FN(spin_unlock),		\
 	FN(sk_fullsock),		\
-	FN(tcp_sock),
+	FN(tcp_sock),			\
+	FN(progenyof),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ef88b167959d..69e209fbd128 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2015,6 +2015,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
 const struct bpf_func_proto bpf_get_current_comm_proto __weak;
 const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
 const struct bpf_func_proto bpf_get_local_storage_proto __weak;
+const struct bpf_func_proto bpf_progenyof_proto __weak;
 
 const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
 {
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index a411fc17d265..3899787e8dbf 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -18,6 +18,7 @@
 #include <linux/sched.h>
 #include <linux/uidgid.h>
 #include <linux/filter.h>
+#include <linux/init_task.h>
 
 /* If kernel subsystem is allowing eBPF programs to call this function,
  * inside its own verifier_ops->get_func_proto() callback it should return
@@ -364,3 +365,31 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
 };
 #endif
 #endif
+
+BPF_CALL_1(bpf_progenyof, int, pid)
+{
+	int result = 0;
+	struct task_struct *task = current;
+
+	if (unlikely(!task))
+		return -EINVAL;
+
+	rcu_read_lock();
+	while (task != &init_task) {
+		if (task->pid == pid) {
+			result = 1;
+			break;
+		}
+		task = rcu_dereference(task->real_parent);
+	}
+	rcu_read_unlock();
+
+	return result;
+}
+
+const struct bpf_func_proto bpf_progenyof_proto = {
+	.func		= bpf_progenyof,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_ANYTHING,
+};
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index f1a86a0d881d..8602ae83c799 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -600,6 +600,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_prandom_u32_proto;
 	case BPF_FUNC_probe_read_str:
 		return &bpf_probe_read_str_proto;
+	case BPF_FUNC_progenyof:
+		return &bpf_progenyof_proto;
 #ifdef CONFIG_CGROUPS
 	case BPF_FUNC_get_current_cgroup_id:
 		return &bpf_get_current_cgroup_id_proto;
-- 
2.17.1

[PATCH v2 bpf-next 2/3] bpf: sync kernel uapi headers

[Javier Honduvilla Coto]

From: Javier Honduvilla Coto <javierhonduco@gmail.com>

Sync kernel uapi headers.

Signed-off-by: Javier Honduvilla Coto <javierhonduco@gmail.com>
---
 tools/include/uapi/linux/bpf.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index bcdd2474eee7..354ec295864c 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2359,6 +2359,14 @@ union bpf_attr {
  *	Return
  *		A **struct bpf_tcp_sock** pointer on success, or NULL in
  *		case of failure.
+ *
+ * int bpf_progenyof(int pid)
+ *	Description
+ *		This helper is useful in programs that want to filter events
+ *		happening to a pid of any of its descendants.
+ *	Return
+ *		1 if the currently executing process' pid is in the process
+ *		hierarchy of the passed pid. 0 Otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2457,7 +2465,8 @@ union bpf_attr {
 	FN(spin_lock),			\
 	FN(spin_unlock),		\
 	FN(sk_fullsock),		\
-	FN(tcp_sock),
+	FN(tcp_sock),			\
+	FN(progenyof),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
-- 
2.17.1

[PATCH v2 bpf-next 3/3] bpf: add tests for bpf_progenyof

[Javier Honduvilla Coto]

From: Javier Honduvilla Coto <javierhonduco@gmail.com>

Adding the following test cases:

- progenyof(current->pid) == 1
- progenyof(current->real_parent->pid) == 1
- progenyof(1) == 1
- progenyof(0) == 0
- progenyof(current->children[0]->pid) == 0

Signed-off-by: Javier Honduvilla Coto <javierhonduco@gmail.com>
---
 tools/testing/selftests/bpf/.gitignore        |   1 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   1 +
 .../selftests/bpf/progs/test_progenyof_kern.c |  46 ++++
 .../selftests/bpf/test_progenyof_user.c       | 249 ++++++++++++++++++
 5 files changed, 298 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_progenyof_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_progenyof_user.c

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index e47168d1257d..677be3b05cd2 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -30,3 +30,4 @@ test_section_names
 test_tcpnotify_user
 test_libbpf
 alu32
+test_progenyof_user
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index ccffaa0a0787..8a8d4b2a5b74 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -23,7 +23,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
 	test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \
 	test_sock test_btf test_sockmap test_lirc_mode2_user get_cgroup_id_user \
 	test_socket_cookie test_cgroup_storage test_select_reuseport test_section_names \
-	test_netcnt test_tcpnotify_user test_sock_fields
+	test_netcnt test_tcpnotify_user test_sock_fields test_progenyof_user
 
 BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
 TEST_GEN_FILES = $(BPF_OBJ_FILES)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index d9999f1ed1d2..1f3e0d1ad159 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -180,6 +180,7 @@ static struct bpf_sock *(*bpf_sk_fullsock)(struct bpf_sock *sk) =
 	(void *) BPF_FUNC_sk_fullsock;
 static struct bpf_tcp_sock *(*bpf_tcp_sock)(struct bpf_sock *sk) =
 	(void *) BPF_FUNC_tcp_sock;
+static int (*bpf_progenyof)(int pid) = (void *) BPF_FUNC_progenyof;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions
diff --git a/tools/testing/selftests/bpf/progs/test_progenyof_kern.c b/tools/testing/selftests/bpf/progs/test_progenyof_kern.c
new file mode 100644
index 000000000000..c4be2c794d83
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_progenyof_kern.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct bpf_map_def SEC("maps") pidmap = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(__u32),
+	.value_size = sizeof(__u32),
+	.max_entries = 2,
+};
+
+struct bpf_map_def SEC("maps") resultmap = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(__u32),
+	.value_size = sizeof(__u32),
+	.max_entries = 1,
+};
+
+SEC("tracepoint/syscalls/sys_enter_open")
+int trace(void *ctx)
+{
+	__u32 pid = bpf_get_current_pid_tgid();
+	__u32 current_key = 0, ancestor_key = 1, *expected_pid, *ancestor_pid;
+	__u32 *val;
+
+	expected_pid = bpf_map_lookup_elem(&pidmap, &current_key);
+	if (!expected_pid || *expected_pid != pid)
+		return 0;
+
+	ancestor_pid = bpf_map_lookup_elem(&pidmap, &ancestor_key);
+	if (!ancestor_pid)
+		return 0;
+
+	if (!bpf_progenyof(*ancestor_pid))
+		return 0;
+
+	val = bpf_map_lookup_elem(&resultmap, &current_key);
+	if (val)
+		*val = *ancestor_pid;
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
diff --git a/tools/testing/selftests/bpf/test_progenyof_user.c b/tools/testing/selftests/bpf/test_progenyof_user.c
new file mode 100644
index 000000000000..0f9572986a11
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_progenyof_user.c
@@ -0,0 +1,249 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <syscall.h>
+#include <unistd.h>
+#include <linux/perf_event.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#define CHECK(condition, tag, format...)                                       \
+	({                                                                     \
+		int __ret = !!(condition);                                     \
+		if (__ret) {                                                   \
+			printf("%s:FAIL:%s ", __func__, tag);                  \
+			printf(format);                                        \
+		} else {                                                       \
+			printf("%s:PASS:%s\n", __func__, tag);                 \
+		}                                                              \
+		__ret;                                                         \
+	})
+
+static int bpf_find_map(const char *test, struct bpf_object *obj,
+			const char *name)
+{
+	struct bpf_map *map;
+
+	map = bpf_object__find_map_by_name(obj, name);
+	if (!map)
+		return -1;
+	return bpf_map__fd(map);
+}
+
+int main(int argc, char **argv)
+{
+	const char *probe_name = "syscalls/sys_enter_open";
+	const char *file = "test_progenyof_kern.o";
+	int err, bytes, efd, prog_fd, pmu_fd;
+	int resultmap_fd, pidmap_fd;
+	struct perf_event_attr attr = {};
+	struct bpf_object *obj;
+	__u32 retrieved_ancestor_pid = 0;
+	__u32 key = 0, pid;
+	int exit_code = EXIT_FAILURE;
+	char buf[256];
+
+	int child_pid, ancestor_pid, root_fd;
+	__u32 ancestor_key = 1;
+	int pipefd[2];
+	char marker[1];
+
+	err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
+	if (CHECK(err, "bpf_prog_load", "err %d errno %d\n", err, errno))
+		goto fail;
+
+	resultmap_fd = bpf_find_map(__func__, obj, "resultmap");
+	if (CHECK(resultmap_fd < 0, "bpf_find_map", "err %d errno %d\n",
+		  resultmap_fd, errno))
+		goto close_prog;
+
+	pidmap_fd = bpf_find_map(__func__, obj, "pidmap");
+	if (CHECK(pidmap_fd < 0, "bpf_find_map", "err %d errno %d\n", pidmap_fd,
+		  errno))
+		goto close_prog;
+
+	pid = getpid();
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+
+	snprintf(buf, sizeof(buf), "/sys/kernel/debug/tracing/events/%s/id",
+		 probe_name);
+	efd = open(buf, O_RDONLY, 0);
+	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+		goto close_prog;
+	bytes = read(efd, buf, sizeof(buf));
+	close(efd);
+	if (CHECK(bytes <= 0 || bytes >= sizeof(buf), "read",
+		  "bytes %d errno %d\n", bytes, errno))
+		goto close_prog;
+
+	attr.config = strtol(buf, NULL, 0);
+	attr.type = PERF_TYPE_TRACEPOINT;
+	attr.sample_type = PERF_SAMPLE_RAW;
+	attr.sample_period = 1;
+	attr.wakeup_events = 1;
+
+	pmu_fd = syscall(__NR_perf_event_open, &attr, getpid(), -1, -1, 0);
+	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n", pmu_fd,
+		  errno))
+		goto close_prog;
+
+	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
+	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n", err,
+		  errno))
+		goto close_pmu;
+
+	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
+	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n", err,
+		  errno))
+		goto close_pmu;
+
+	// Test on itself: progenyof(current->pid) is true
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &key, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != pid,
+		  "progenyof is true with same pid", "%d == %d\n",
+		  retrieved_ancestor_pid, pid))
+		goto close_pmu;
+
+	// Test that PID 1 is among our progeny
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = 1;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &key, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != ancestor_pid,
+		  "progenyof reaches init", "%d == %d\n",
+		  retrieved_ancestor_pid, ancestor_pid))
+		goto close_pmu;
+
+	// Test we don't go over PID 1
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = 0;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &key, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != 0,
+		  "progenyof does not go over init", "%d == %d\n",
+		  retrieved_ancestor_pid, 0))
+		goto close_pmu;
+
+	// Test that we are the progeny of our child
+	pipe(pipefd);
+	child_pid = fork();
+	if (child_pid == -1) {
+		printf("fork failed\n");
+		goto close_pmu;
+	} else if (child_pid == 0) {
+		close(pipefd[1]);
+		read(pipefd[0], &marker, 1);
+
+		root_fd = open("/", O_RDONLY);
+		if (CHECK(efd < 0, "open", "errno %d\n", errno))
+			goto close_prog;
+		close(root_fd);
+
+		close(pipefd[0]);
+		_exit(EXIT_SUCCESS);
+	} else {
+		close(pipefd[0]);
+		bpf_map_update_elem(resultmap_fd, &key, &key, 0);
+		bpf_map_update_elem(pidmap_fd, &key, &child_pid, 0);
+		bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+
+		write(pipefd[1], &marker, 1);
+		wait(NULL);
+		close(pipefd[1]);
+
+		err = bpf_map_lookup_elem(resultmap_fd, &key,
+					  &retrieved_ancestor_pid);
+		if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err,
+			  errno))
+			goto close_pmu;
+		if (CHECK(retrieved_ancestor_pid != pid, "progenyof of parent",
+			  "%d == %d\n", retrieved_ancestor_pid, pid))
+			goto close_pmu;
+	}
+
+	// Test that a child of ours doesn't belong to our progeny
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &key, 0);
+
+	pipe(pipefd);
+	child_pid = fork();
+	if (child_pid == -1) {
+		printf("fork failed\n");
+		goto close_pmu;
+	} else if (child_pid == 0) {
+		close(pipefd[1]);
+		read(pipefd[0], marker, 1);
+		close(pipefd[0]);
+		_exit(EXIT_SUCCESS);
+	} else {
+		close(pipefd[0]);
+
+		bpf_map_update_elem(pidmap_fd, &ancestor_key, &child_pid, 0);
+
+		root_fd = open("/", O_RDONLY);
+		if (CHECK(efd < 0, "open", "errno %d\n", errno))
+			goto close_prog;
+		close(root_fd);
+
+		write(pipefd[1], marker, 1);
+		wait(NULL);
+		close(pipefd[1]);
+
+		err = bpf_map_lookup_elem(resultmap_fd, &key,
+					  &retrieved_ancestor_pid);
+		if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err,
+			  errno))
+			goto close_pmu;
+		if (CHECK(retrieved_ancestor_pid != 0, "progenyof of child",
+			  "%d == %d\n", retrieved_ancestor_pid, 0))
+			goto close_pmu;
+	}
+
+	exit_code = EXIT_SUCCESS;
+	printf("%s:PASS\n", argv[0]);
+
+close_pmu:
+	close(pmu_fd);
+close_prog:
+	bpf_object__close(obj);
+fail:
+	return exit_code;
+}
-- 
2.17.1

Re: [PATCH bpf-next 1/3] bpf: add bpf_progenyof helper

[Martin Lau]

On Fri, Mar 01, 2019 at 09:28:39AM -0800, Javier Honduvilla Coto wrote:
> On Wed, Feb 27, 2019 at 06:26:41AM +0000, Martin Lau wrote:
> > On Tue, Feb 26, 2019 at 02:36:49PM -0800, Javier Honduvilla Coto wrote:
> > > This patch adds the bpf_progenyof helper which receives a PID and returns
> > What is progenof?
> >
> > > 1 if the process currently being executed is in the process hierarchy
> > > including itself or 0 if not.
> > >
> > > This is very useful in tracing programs when we want to filter by a
> > > given PID and all the children it might spawn. The current workarounds
> > > most people implement for this purpose have issues:
> > >
> > > - Attaching to process spawning syscalls and dynamically add those PIDs
> > >   to some bpf map that would be used to filter is cumbersome and
> > > potentially racy.
> > > - Unrolling some loop to perform what this helper is doing consumes lots
> > >   of instructions. That and the impossibility to jump backwards makes it
> > > really hard to be correct in really large process chains.
> > >
> > > Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> > > ---
> > >  include/linux/bpf.h      |  1 +
> > >  include/uapi/linux/bpf.h |  3 ++-
> > >  kernel/bpf/core.c        |  1 +
> > >  kernel/bpf/helpers.c     | 29 +++++++++++++++++++++++++++++
> > >  kernel/trace/bpf_trace.c |  2 ++
> > >  5 files changed, 35 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > > index de18227b3d95..447395ba202b 100644
> > > --- a/include/linux/bpf.h
> > > +++ b/include/linux/bpf.h
> > > @@ -921,6 +921,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
> > >  extern const struct bpf_func_proto bpf_spin_lock_proto;
> > >  extern const struct bpf_func_proto bpf_spin_unlock_proto;
> > >  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> > > +extern const struct bpf_func_proto bpf_progenyof_proto;
> > It seems only used in bpf_trace.c.  Does it have to be here?
> >
> > >
> > >  /* Shared helpers among cBPF and eBPF. */
> > >  void bpf_user_rnd_init_once(void);
> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index bcdd2474eee7..804e4218eb28 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -2457,7 +2457,8 @@ union bpf_attr {
> > >  	FN(spin_lock),			\
> > >  	FN(spin_unlock),		\
> > >  	FN(sk_fullsock),		\
> > > -	FN(tcp_sock),
> > > +	FN(tcp_sock),			\
> > > +	FN(progenyof),
> > Please add doc like other helpers do.
> 
> Oops, good catch, thanks! Will send v2 soon!!
> 
> >
> > >
> > >  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> > >   * function eBPF program intends to call
> > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > > index ef88b167959d..69e209fbd128 100644
> > > --- a/kernel/bpf/core.c
> > > +++ b/kernel/bpf/core.c
> > > @@ -2015,6 +2015,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
> > >  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
> > >  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
> > >  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> > > +const struct bpf_func_proto bpf_progenyof_proto __weak;
> > >
> > >  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
> > >  {
> > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > > index a411fc17d265..3899787e8dbf 100644
> > > --- a/kernel/bpf/helpers.c
> > > +++ b/kernel/bpf/helpers.c
> > > @@ -18,6 +18,7 @@
> > >  #include <linux/sched.h>
> > >  #include <linux/uidgid.h>
> > >  #include <linux/filter.h>
> > > +#include <linux/init_task.h>
> > >
> > >  /* If kernel subsystem is allowing eBPF programs to call this function,
> > >   * inside its own verifier_ops->get_func_proto() callback it should return
> > > @@ -364,3 +365,31 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
> > >  };
> > >  #endif
> > >  #endif
> > > +
> > > +BPF_CALL_1(bpf_progenyof, int, pid)
> > > +{
> > > +	int result = 0;
> > > +	struct task_struct *task = current;
> > > +
> > > +	if (unlikely(!task))
> > hmm.... Could current be NULL?
> 
> Wasn't sure about this but added as bpf_get_current_pid_tgid,
> bpf_get_current_uid_gid, and bpf_get_current_comm check for this. Texted Alexei
> about this and he told me this is probably not necessary anymore, but I
> guess it doesn't hurt leaving it?
> 
> >
> > > +		return -EINVAL;
> > > +
> > > +	rcu_read_lock();
> > > +	while (task != &init_task) {
> > I don't know the details of init_task, so qq:
> > Could the passed in "pid" be the init_task->pid?
> > If possible, what is the expected "result"?
> >
> 
> Yep! init_task doesn't set a pid for what I could see, so I guess it
> will be PID=0. The test in the last patch check bpf_progenyof(0) :)
> 
> bpf_progenyof with 0 or 1 will always return 1
the test in patch 3 commit message has this though:
"- progenyof(0) == 0"

so the intention for progenyof(0) is to always return 0 or 1?

A random ps output from my vm:
[root@arch-fb-vm1 bpf]# ps -eaf | head -3
UID        PID  PPID  C STIME TTY          TIME CMD
root         1     0  0 11:45 ?        00:00:12 /sbin/init
root         2     0  0 11:45 ?        00:00:00 [kthreadd]

I was asking because,
after reading the loop, it seems all tasks tracing back to init_task.
so my intuitive thinking is progenyof(init_task.pid) should always
return 1.  If it is otherwise, some comments and doc would be useful
to explain why treating init_task.pid differently.

> 
> > > +		if (task->pid == pid) {
> > > +			result = 1;
> > > +			break;
> > > +		}
> > > +		task = rcu_dereference(task->real_parent);
> > > +	}
> > > +	rcu_read_unlock();
> > > +
> > > +	return result;
> > > +}
> > > +
> > > +const struct bpf_func_proto bpf_progenyof_proto = {
> > > +	.func		= bpf_progenyof,
> > > +	.gpl_only	= false,
> > > +	.ret_type	= RET_INTEGER,
> > > +	.arg1_type	= ARG_ANYTHING,
> > > +};
> > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > > index f1a86a0d881d..8602ae83c799 100644
> > > --- a/kernel/trace/bpf_trace.c
> > > +++ b/kernel/trace/bpf_trace.c
> > > @@ -600,6 +600,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > >  		return &bpf_get_prandom_u32_proto;
> > >  	case BPF_FUNC_probe_read_str:
> > >  		return &bpf_probe_read_str_proto;
> > > +	case BPF_FUNC_progenyof:
> > > +		return &bpf_progenyof_proto;
> > >  #ifdef CONFIG_CGROUPS
> > >  	case BPF_FUNC_get_current_cgroup_id:
> > >  		return &bpf_get_current_cgroup_id_proto;
> > > --
> > > 2.17.1
> > >

Re: [PATCH v2 bpf-next 1/3] bpf: add bpf_progenyof helper

[Martin Lau]

On Fri, Mar 01, 2019 at 10:06:12AM -0800, Javier Honduvilla Coto wrote:
> From: Javier Honduvilla Coto <javierhonduco@gmail.com>
> 
> This patch adds the bpf_progenyof helper which receives a PID and returns
> 1 if the process currently being executed is in the process hierarchy
> including itself or 0 if not.
> 
> This is very useful in tracing programs when we want to filter by a
> given PID and all the children it might spawn. The current workarounds
> most people implement for this purpose have issues:
> 
> - Attaching to process spawning syscalls and dynamically add those PIDs
>   to some bpf map that would be used to filter is cumbersome and
> potentially racy.
> - Unrolling some loop to perform what this helper is doing consumes lots
>   of instructions. That and the impossibility to jump backwards makes it
> really hard to be correct in really large process chains.
> 
> Signed-off-by: Javier Honduvilla Coto <javierhonduco@gmail.com>
> ---
>  include/linux/bpf.h      |  1 +
>  include/uapi/linux/bpf.h | 11 ++++++++++-
>  kernel/bpf/core.c        |  1 +
>  kernel/bpf/helpers.c     | 29 +++++++++++++++++++++++++++++
>  kernel/trace/bpf_trace.c |  2 ++
>  5 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index de18227b3d95..447395ba202b 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -921,6 +921,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
>  extern const struct bpf_func_proto bpf_spin_lock_proto;
>  extern const struct bpf_func_proto bpf_spin_unlock_proto;
>  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> +extern const struct bpf_func_proto bpf_progenyof_proto;
>  
>  /* Shared helpers among cBPF and eBPF. */
>  void bpf_user_rnd_init_once(void);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index bcdd2474eee7..354ec295864c 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2359,6 +2359,14 @@ union bpf_attr {
>   *	Return
>   *		A **struct bpf_tcp_sock** pointer on success, or NULL in
>   *		case of failure.
> + *
> + * int bpf_progenyof(int pid)
> + *	Description
> + *		This helper is useful in programs that want to filter events
> + *		happening to a pid of any of its descendants.
> + *	Return
> + *		1 if the currently executing process' pid is in the process
> + *		hierarchy of the passed pid. 0 Otherwise.
>   */
>  #define __BPF_FUNC_MAPPER(FN)		\
>  	FN(unspec),			\
> @@ -2457,7 +2465,8 @@ union bpf_attr {
>  	FN(spin_lock),			\
>  	FN(spin_unlock),		\
>  	FN(sk_fullsock),		\
> -	FN(tcp_sock),
> +	FN(tcp_sock),			\
> +	FN(progenyof),
>  
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index ef88b167959d..69e209fbd128 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2015,6 +2015,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
>  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
>  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
>  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> +const struct bpf_func_proto bpf_progenyof_proto __weak;
>  
>  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
>  {
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index a411fc17d265..3899787e8dbf 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -18,6 +18,7 @@
>  #include <linux/sched.h>
>  #include <linux/uidgid.h>
>  #include <linux/filter.h>
> +#include <linux/init_task.h>
>  
>  /* If kernel subsystem is allowing eBPF programs to call this function,
>   * inside its own verifier_ops->get_func_proto() callback it should return
> @@ -364,3 +365,31 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
>  };
>  #endif
>  #endif
> +
> +BPF_CALL_1(bpf_progenyof, int, pid)
> +{
> +	int result = 0;
> +	struct task_struct *task = current;
> +
> +	if (unlikely(!task))
> +		return -EINVAL;
> +
> +	rcu_read_lock();
I don't think it is needed.  It should already be under rcu_read_lock().
You may add 'WARN_ON_ONCE(!rcu_read_lock_held());' to be safe,
like other "bpf_map_xxx_elem()" helpers in the same file.

> +	while (task != &init_task) {
> +		if (task->pid == pid) {
> +			result = 1;
> +			break;
> +		}
> +		task = rcu_dereference(task->real_parent);
> +	}
> +	rcu_read_unlock();
> +
> +	return result;
> +}
> +
> +const struct bpf_func_proto bpf_progenyof_proto = {
> +	.func		= bpf_progenyof,
> +	.gpl_only	= false,
> +	.ret_type	= RET_INTEGER,
> +	.arg1_type	= ARG_ANYTHING,
> +};
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index f1a86a0d881d..8602ae83c799 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -600,6 +600,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		return &bpf_get_prandom_u32_proto;
>  	case BPF_FUNC_probe_read_str:
>  		return &bpf_probe_read_str_proto;
> +	case BPF_FUNC_progenyof:
> +		return &bpf_progenyof_proto;
>  #ifdef CONFIG_CGROUPS
>  	case BPF_FUNC_get_current_cgroup_id:
>  		return &bpf_get_current_cgroup_id_proto;
> -- 
> 2.17.1
> 

Re: [PATCH bpf-next 1/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

On Sat, Mar 02, 2019 at 12:01:14AM +0000, Martin Lau wrote:
> On Fri, Mar 01, 2019 at 09:28:39AM -0800, Javier Honduvilla Coto wrote:
> > On Wed, Feb 27, 2019 at 06:26:41AM +0000, Martin Lau wrote:
> > > On Tue, Feb 26, 2019 at 02:36:49PM -0800, Javier Honduvilla Coto wrote:
> > > > This patch adds the bpf_progenyof helper which receives a PID and returns
> > > What is progenof?
> > >
> > > > 1 if the process currently being executed is in the process hierarchy
> > > > including itself or 0 if not.
> > > >
> > > > This is very useful in tracing programs when we want to filter by a
> > > > given PID and all the children it might spawn. The current workarounds
> > > > most people implement for this purpose have issues:
> > > >
> > > > - Attaching to process spawning syscalls and dynamically add those PIDs
> > > >   to some bpf map that would be used to filter is cumbersome and
> > > > potentially racy.
> > > > - Unrolling some loop to perform what this helper is doing consumes lots
> > > >   of instructions. That and the impossibility to jump backwards makes it
> > > > really hard to be correct in really large process chains.
> > > >
> > > > Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> > > > ---
> > > >  include/linux/bpf.h      |  1 +
> > > >  include/uapi/linux/bpf.h |  3 ++-
> > > >  kernel/bpf/core.c        |  1 +
> > > >  kernel/bpf/helpers.c     | 29 +++++++++++++++++++++++++++++
> > > >  kernel/trace/bpf_trace.c |  2 ++
> > > >  5 files changed, 35 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > > > index de18227b3d95..447395ba202b 100644
> > > > --- a/include/linux/bpf.h
> > > > +++ b/include/linux/bpf.h
> > > > @@ -921,6 +921,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
> > > >  extern const struct bpf_func_proto bpf_spin_lock_proto;
> > > >  extern const struct bpf_func_proto bpf_spin_unlock_proto;
> > > >  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> > > > +extern const struct bpf_func_proto bpf_progenyof_proto;
> > > It seems only used in bpf_trace.c.  Does it have to be here?
> > >
> > > >
> > > >  /* Shared helpers among cBPF and eBPF. */
> > > >  void bpf_user_rnd_init_once(void);
> > > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > > index bcdd2474eee7..804e4218eb28 100644
> > > > --- a/include/uapi/linux/bpf.h
> > > > +++ b/include/uapi/linux/bpf.h
> > > > @@ -2457,7 +2457,8 @@ union bpf_attr {
> > > >  	FN(spin_lock),			\
> > > >  	FN(spin_unlock),		\
> > > >  	FN(sk_fullsock),		\
> > > > -	FN(tcp_sock),
> > > > +	FN(tcp_sock),			\
> > > > +	FN(progenyof),
> > > Please add doc like other helpers do.
> >
> > Oops, good catch, thanks! Will send v2 soon!!
> >
> > >
> > > >
> > > >  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> > > >   * function eBPF program intends to call
> > > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > > > index ef88b167959d..69e209fbd128 100644
> > > > --- a/kernel/bpf/core.c
> > > > +++ b/kernel/bpf/core.c
> > > > @@ -2015,6 +2015,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
> > > >  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
> > > >  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
> > > >  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> > > > +const struct bpf_func_proto bpf_progenyof_proto __weak;
> > > >
> > > >  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
> > > >  {
> > > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > > > index a411fc17d265..3899787e8dbf 100644
> > > > --- a/kernel/bpf/helpers.c
> > > > +++ b/kernel/bpf/helpers.c
> > > > @@ -18,6 +18,7 @@
> > > >  #include <linux/sched.h>
> > > >  #include <linux/uidgid.h>
> > > >  #include <linux/filter.h>
> > > > +#include <linux/init_task.h>
> > > >
> > > >  /* If kernel subsystem is allowing eBPF programs to call this function,
> > > >   * inside its own verifier_ops->get_func_proto() callback it should return
> > > > @@ -364,3 +365,31 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
> > > >  };
> > > >  #endif
> > > >  #endif
> > > > +
> > > > +BPF_CALL_1(bpf_progenyof, int, pid)
> > > > +{
> > > > +	int result = 0;
> > > > +	struct task_struct *task = current;
> > > > +
> > > > +	if (unlikely(!task))
> > > hmm.... Could current be NULL?
> >
> > Wasn't sure about this but added as bpf_get_current_pid_tgid,
> > bpf_get_current_uid_gid, and bpf_get_current_comm check for this. Texted Alexei
> > about this and he told me this is probably not necessary anymore, but I
> > guess it doesn't hurt leaving it?
> >
> > >
> > > > +		return -EINVAL;
> > > > +
> > > > +	rcu_read_lock();
> > > > +	while (task != &init_task) {
> > > I don't know the details of init_task, so qq:
> > > Could the passed in "pid" be the init_task->pid?
> > > If possible, what is the expected "result"?
> > >
> >
> > Yep! init_task doesn't set a pid for what I could see, so I guess it
> > will be PID=0. The test in the last patch check bpf_progenyof(0) :)
> >
> > bpf_progenyof with 0 or 1 will always return 1
> the test in patch 3 commit message has this though:
> "- progenyof(0) == 0"
>
> so the intention for progenyof(0) is to always return 0 or 1?
>
> A random ps output from my vm:
> [root@arch-fb-vm1 bpf]# ps -eaf | head -3
> UID        PID  PPID  C STIME TTY          TIME CMD
> root         1     0  0 11:45 ?        00:00:12 /sbin/init
> root         2     0  0 11:45 ?        00:00:00 [kthreadd]
>
> I was asking because,
> after reading the loop, it seems all tasks tracing back to init_task.
> so my intuitive thinking is progenyof(init_task.pid) should always
> return 1.  If it is otherwise, some comments and doc would be useful
> to explain why treating init_task.pid differently.
>

My bad, that was a typo. bpf_progenyof(1) returns 1, and bpf_progenyof(0)
returns 0. Sorry for the confusion.

This is a good point! I chose to return 0 for this case because of
init_task being an implementation detail and PID 0 not having much
meaning for most users, but you are right that I should document it as
an exception if we keep it as is

That being said I think changing the behaviour to make progenyof(0) return 1
makes more sense from a semantics perspective!

> >
> > > > +		if (task->pid == pid) {
> > > > +			result = 1;
> > > > +			break;
> > > > +		}
> > > > +		task = rcu_dereference(task->real_parent);
> > > > +	}
> > > > +	rcu_read_unlock();
> > > > +
> > > > +	return result;
> > > > +}
> > > > +
> > > > +const struct bpf_func_proto bpf_progenyof_proto = {
> > > > +	.func		= bpf_progenyof,
> > > > +	.gpl_only	= false,
> > > > +	.ret_type	= RET_INTEGER,
> > > > +	.arg1_type	= ARG_ANYTHING,
> > > > +};
> > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > > > index f1a86a0d881d..8602ae83c799 100644
> > > > --- a/kernel/trace/bpf_trace.c
> > > > +++ b/kernel/trace/bpf_trace.c
> > > > @@ -600,6 +600,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > > >  		return &bpf_get_prandom_u32_proto;
> > > >  	case BPF_FUNC_probe_read_str:
> > > >  		return &bpf_probe_read_str_proto;
> > > > +	case BPF_FUNC_progenyof:
> > > > +		return &bpf_progenyof_proto;
> > > >  #ifdef CONFIG_CGROUPS
> > > >  	case BPF_FUNC_get_current_cgroup_id:
> > > >  		return &bpf_get_current_cgroup_id_proto;
> > > > --
> > > > 2.17.1
> > > >

Re: [PATCH v2 bpf-next 1/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

On Sat, Mar 02, 2019 at 12:12:57AM +0000, Martin Lau wrote:
> On Fri, Mar 01, 2019 at 10:06:12AM -0800, Javier Honduvilla Coto wrote:
> > From: Javier Honduvilla Coto <javierhonduco@gmail.com>
> >
> > This patch adds the bpf_progenyof helper which receives a PID and returns
> > 1 if the process currently being executed is in the process hierarchy
> > including itself or 0 if not.
> >
> > This is very useful in tracing programs when we want to filter by a
> > given PID and all the children it might spawn. The current workarounds
> > most people implement for this purpose have issues:
> >
> > - Attaching to process spawning syscalls and dynamically add those PIDs
> >   to some bpf map that would be used to filter is cumbersome and
> > potentially racy.
> > - Unrolling some loop to perform what this helper is doing consumes lots
> >   of instructions. That and the impossibility to jump backwards makes it
> > really hard to be correct in really large process chains.
> >
> > Signed-off-by: Javier Honduvilla Coto <javierhonduco@gmail.com>
> > ---
> >  include/linux/bpf.h      |  1 +
> >  include/uapi/linux/bpf.h | 11 ++++++++++-
> >  kernel/bpf/core.c        |  1 +
> >  kernel/bpf/helpers.c     | 29 +++++++++++++++++++++++++++++
> >  kernel/trace/bpf_trace.c |  2 ++
> >  5 files changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index de18227b3d95..447395ba202b 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -921,6 +921,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
> >  extern const struct bpf_func_proto bpf_spin_lock_proto;
> >  extern const struct bpf_func_proto bpf_spin_unlock_proto;
> >  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> > +extern const struct bpf_func_proto bpf_progenyof_proto;
> >
> >  /* Shared helpers among cBPF and eBPF. */
> >  void bpf_user_rnd_init_once(void);
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index bcdd2474eee7..354ec295864c 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -2359,6 +2359,14 @@ union bpf_attr {
> >   *	Return
> >   *		A **struct bpf_tcp_sock** pointer on success, or NULL in
> >   *		case of failure.
> > + *
> > + * int bpf_progenyof(int pid)
> > + *	Description
> > + *		This helper is useful in programs that want to filter events
> > + *		happening to a pid of any of its descendants.
> > + *	Return
> > + *		1 if the currently executing process' pid is in the process
> > + *		hierarchy of the passed pid. 0 Otherwise.
> >   */
> >  #define __BPF_FUNC_MAPPER(FN)		\
> >  	FN(unspec),			\
> > @@ -2457,7 +2465,8 @@ union bpf_attr {
> >  	FN(spin_lock),			\
> >  	FN(spin_unlock),		\
> >  	FN(sk_fullsock),		\
> > -	FN(tcp_sock),
> > +	FN(tcp_sock),			\
> > +	FN(progenyof),
> >
> >  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> >   * function eBPF program intends to call
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index ef88b167959d..69e209fbd128 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -2015,6 +2015,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
> >  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
> >  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
> >  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> > +const struct bpf_func_proto bpf_progenyof_proto __weak;
> >
> >  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
> >  {
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index a411fc17d265..3899787e8dbf 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/sched.h>
> >  #include <linux/uidgid.h>
> >  #include <linux/filter.h>
> > +#include <linux/init_task.h>
> >
> >  /* If kernel subsystem is allowing eBPF programs to call this function,
> >   * inside its own verifier_ops->get_func_proto() callback it should return
> > @@ -364,3 +365,31 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
> >  };
> >  #endif
> >  #endif
> > +
> > +BPF_CALL_1(bpf_progenyof, int, pid)
> > +{
> > +	int result = 0;
> > +	struct task_struct *task = current;
> > +
> > +	if (unlikely(!task))
> > +		return -EINVAL;
> > +
> > +	rcu_read_lock();
> I don't think it is needed.  It should already be under rcu_read_lock().
> You may add 'WARN_ON_ONCE(!rcu_read_lock_held());' to be safe,
> like other "bpf_map_xxx_elem()" helpers in the same file.
>

Spot on! Just checked that we are rcu locking in include/linux/bpf.h
being called kernel/trace/bpf_trace.c. Thanks!!

> > +	while (task != &init_task) {
> > +		if (task->pid == pid) {
> > +			result = 1;
> > +			break;
> > +		}
> > +		task = rcu_dereference(task->real_parent);
> > +	}
> > +	rcu_read_unlock();
> > +
> > +	return result;
> > +}
> > +
> > +const struct bpf_func_proto bpf_progenyof_proto = {
> > +	.func		= bpf_progenyof,
> > +	.gpl_only	= false,
> > +	.ret_type	= RET_INTEGER,
> > +	.arg1_type	= ARG_ANYTHING,
> > +};
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index f1a86a0d881d..8602ae83c799 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -600,6 +600,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >  		return &bpf_get_prandom_u32_proto;
> >  	case BPF_FUNC_probe_read_str:
> >  		return &bpf_probe_read_str_proto;
> > +	case BPF_FUNC_progenyof:
> > +		return &bpf_progenyof_proto;
> >  #ifdef CONFIG_CGROUPS
> >  	case BPF_FUNC_get_current_cgroup_id:
> >  		return &bpf_get_current_cgroup_id_proto;
> > --
> > 2.17.1
> >

[PATCH v3 bpf-next 0/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

Hi all,

This patch add the bpf_progenyof helper which receives a PID and returns
1 if the process currently being executed is in the process hierarchy,
including itself or 0 if not.

This is very useful in tracing programs when we want to filter by a
given PID and all the children it might have. The current workarounds
most people implement for this purpose have issues:

- Attaching to process spawning syscalls and dynamically add those PIDs
  to  some bpf map that would be used to filter is cumbersome and
potentially racy.
- Unrolling some loop to perform what this helper is doing consumes lots
  of instructions. That and the impossibility to jump backwards makes it
really hard to be correct in really large process chains.

Let me know what do you think!

Thanks,

---
Changed in V3:
        - Removed RCU read (un)locking as BPF programs alredy run in RCU locked
                context
        - progenyof(0) now returns 1, which, semantically makes more sense
        - Added new test case for PID 0 and changed sentinel value for errors
        - Rebase on latest bpf-next/master
        - Used my work email as somehow I accidentally used my personal one in v2

Changed in V2:
        - Adding missing docs in include/uapi/linux/bpf.h

Javier Honduvilla Coto (3):
  bpf: add bpf_progenyof helper
  bpf: sync kernel uapi headers
  bpf: add tests for bpf_progenyof

 include/linux/bpf.h                           |   1 +
 include/uapi/linux/bpf.h                      |  10 +-
 kernel/bpf/core.c                             |   1 +
 kernel/bpf/helpers.c                          |  32 +++
 kernel/trace/bpf_trace.c                      |   2 +
 tools/include/uapi/linux/bpf.h                |   9 +-
 tools/testing/selftests/bpf/.gitignore        |   1 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   1 +
 .../selftests/bpf/progs/test_progenyof_kern.c |  46 +++
 .../selftests/bpf/test_progenyof_user.c       | 268 ++++++++++++++++++
 11 files changed, 370 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_progenyof_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_progenyof_user.c

-- 
2.17.1

[PATCH v3 bpf-next 1/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

This patch adds the bpf_progenyof helper which receives a PID and returns
1 if the process currently being executed is in the process hierarchy
including itself or 0 if not.

This is very useful in tracing programs when we want to filter by a
given PID and all the children it might spawn. The current workarounds
most people implement for this purpose have issues:

- Attaching to process spawning syscalls and dynamically add those PIDs
  to some bpf map that would be used to filter is cumbersome and
potentially racy.
- Unrolling some loop to perform what this helper is doing consumes lots
  of instructions. That and the impossibility to jump backwards makes it
really hard to be correct in really large process chains.

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 include/linux/bpf.h      |  1 +
 include/uapi/linux/bpf.h | 10 +++++++++-
 kernel/bpf/core.c        |  1 +
 kernel/bpf/helpers.c     | 32 ++++++++++++++++++++++++++++++++
 kernel/trace/bpf_trace.c |  2 ++
 5 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a2132e09dc1c..c40a66677b65 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -930,6 +930,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
 extern const struct bpf_func_proto bpf_spin_lock_proto;
 extern const struct bpf_func_proto bpf_spin_unlock_proto;
 extern const struct bpf_func_proto bpf_get_local_storage_proto;
+extern const struct bpf_func_proto bpf_progenyof_proto;
 
 /* Shared helpers among cBPF and eBPF. */
 void bpf_user_rnd_init_once(void);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 3c38ac9a92a7..710c6f6baf5a 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2366,6 +2366,13 @@ union bpf_attr {
  *             current value is ect (ECN capable). Works with IPv6 and IPv4.
  *     Return
  *             1 if set, 0 if not set.
+ * int bpf_progenyof(int pid)
+ *	Description
+ *		This helper is useful in programs that want to filter events
+ *		happening to a pid of any of its descendants.
+ *	Return
+ *		1 if the currently executing process' pid is in the process
+ *		hierarchy of the passed pid. 0 Otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2465,7 +2472,8 @@ union bpf_attr {
 	FN(spin_unlock),		\
 	FN(sk_fullsock),		\
 	FN(tcp_sock),			\
-	FN(skb_ecn_set_ce),
+	FN(skb_ecn_set_ce),		\
+	FN(progenyof),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 3f08c257858e..97e17a8a1530 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2044,6 +2044,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
 const struct bpf_func_proto bpf_get_current_comm_proto __weak;
 const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
 const struct bpf_func_proto bpf_get_local_storage_proto __weak;
+const struct bpf_func_proto bpf_progenyof_proto __weak;
 
 const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
 {
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index a411fc17d265..f093b35d1ba8 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -18,6 +18,7 @@
 #include <linux/sched.h>
 #include <linux/uidgid.h>
 #include <linux/filter.h>
+#include <linux/init_task.h>
 
 /* If kernel subsystem is allowing eBPF programs to call this function,
  * inside its own verifier_ops->get_func_proto() callback it should return
@@ -364,3 +365,34 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
 };
 #endif
 #endif
+
+BPF_CALL_1(bpf_progenyof, int, pid)
+{
+	int result = 0;
+	struct task_struct *task = current;
+
+	WARN_ON(!rcu_read_lock_held());
+
+	if (unlikely(!task))
+		return -EINVAL;
+
+	if (pid == 0)
+		return 1;
+
+	while (task != &init_task) {
+		if (task->pid == pid) {
+			result = 1;
+			break;
+		}
+		task = rcu_dereference(task->real_parent);
+	}
+
+	return result;
+}
+
+const struct bpf_func_proto bpf_progenyof_proto = {
+	.func		= bpf_progenyof,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_ANYTHING,
+};
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index f1a86a0d881d..8602ae83c799 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -600,6 +600,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_prandom_u32_proto;
 	case BPF_FUNC_probe_read_str:
 		return &bpf_probe_read_str_proto;
+	case BPF_FUNC_progenyof:
+		return &bpf_progenyof_proto;
 #ifdef CONFIG_CGROUPS
 	case BPF_FUNC_get_current_cgroup_id:
 		return &bpf_get_current_cgroup_id_proto;
-- 
2.17.1

[PATCH v3 bpf-next 2/3] bpf: sync kernel uapi headers

[Javier Honduvilla Coto]

Sync kernel uapi headers.

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 tools/include/uapi/linux/bpf.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 3c38ac9a92a7..f72741766ba8 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2366,6 +2366,12 @@ union bpf_attr {
  *             current value is ect (ECN capable). Works with IPv6 and IPv4.
  *     Return
  *             1 if set, 0 if not set.
+ *	Description
+ *		This helper is useful in programs that want to filter events
+ *		happening to a pid of any of its descendants.
+ *	Return
+ *		1 if the currently executing process' pid is in the process
+ *		hierarchy of the passed pid. 0 Otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2465,7 +2471,8 @@ union bpf_attr {
 	FN(spin_unlock),		\
 	FN(sk_fullsock),		\
 	FN(tcp_sock),			\
-	FN(skb_ecn_set_ce),
+	FN(skb_ecn_set_ce),		\
+	FN(progenyof),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
-- 
2.17.1

[PATCH v3 bpf-next 3/3] bpf: add tests for bpf_progenyof

[Javier Honduvilla Coto]

Adding the following test cases:

- progenyof(current->pid) == 1
- progenyof(current->real_parent->pid) == 1
- progenyof(1) == 1
- progenyof(0) == 1

- progenyof(-1) == 0
- progenyof(current->children[0]->pid) == 0

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 tools/testing/selftests/bpf/.gitignore        |   1 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   1 +
 .../selftests/bpf/progs/test_progenyof_kern.c |  46 +++
 .../selftests/bpf/test_progenyof_user.c       | 268 ++++++++++++++++++
 5 files changed, 317 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_progenyof_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_progenyof_user.c

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 3b74d23fffab..94590555307c 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -31,3 +31,4 @@ test_section_names
 test_tcpnotify_user
 test_libbpf
 alu32
+test_progenyof_user
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 518cd587cd63..18efaa6e1fb5 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -23,7 +23,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
 	test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \
 	test_sock test_btf test_sockmap test_lirc_mode2_user get_cgroup_id_user \
 	test_socket_cookie test_cgroup_storage test_select_reuseport test_section_names \
-	test_netcnt test_tcpnotify_user test_sock_fields
+	test_netcnt test_tcpnotify_user test_sock_fields test_progenyof_user
 
 BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
 TEST_GEN_FILES = $(BPF_OBJ_FILES)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index c9433a496d54..cca12894b1b9 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -182,6 +182,7 @@ static struct bpf_tcp_sock *(*bpf_tcp_sock)(struct bpf_sock *sk) =
 	(void *) BPF_FUNC_tcp_sock;
 static int (*bpf_skb_ecn_set_ce)(void *ctx) =
 	(void *) BPF_FUNC_skb_ecn_set_ce;
+static int (*bpf_progenyof)(int pid) = (void *) BPF_FUNC_progenyof;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions
diff --git a/tools/testing/selftests/bpf/progs/test_progenyof_kern.c b/tools/testing/selftests/bpf/progs/test_progenyof_kern.c
new file mode 100644
index 000000000000..c4be2c794d83
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_progenyof_kern.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct bpf_map_def SEC("maps") pidmap = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(__u32),
+	.value_size = sizeof(__u32),
+	.max_entries = 2,
+};
+
+struct bpf_map_def SEC("maps") resultmap = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(__u32),
+	.value_size = sizeof(__u32),
+	.max_entries = 1,
+};
+
+SEC("tracepoint/syscalls/sys_enter_open")
+int trace(void *ctx)
+{
+	__u32 pid = bpf_get_current_pid_tgid();
+	__u32 current_key = 0, ancestor_key = 1, *expected_pid, *ancestor_pid;
+	__u32 *val;
+
+	expected_pid = bpf_map_lookup_elem(&pidmap, &current_key);
+	if (!expected_pid || *expected_pid != pid)
+		return 0;
+
+	ancestor_pid = bpf_map_lookup_elem(&pidmap, &ancestor_key);
+	if (!ancestor_pid)
+		return 0;
+
+	if (!bpf_progenyof(*ancestor_pid))
+		return 0;
+
+	val = bpf_map_lookup_elem(&resultmap, &current_key);
+	if (val)
+		*val = *ancestor_pid;
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
diff --git a/tools/testing/selftests/bpf/test_progenyof_user.c b/tools/testing/selftests/bpf/test_progenyof_user.c
new file mode 100644
index 000000000000..a395eb2473b5
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_progenyof_user.c
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <syscall.h>
+#include <unistd.h>
+#include <linux/perf_event.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#define CHECK(condition, tag, format...)                                       \
+	({                                                                     \
+		int __ret = !!(condition);                                     \
+		if (__ret) {                                                   \
+			printf("%s:FAIL:%s ", __func__, tag);                  \
+			printf(format);                                        \
+		} else {                                                       \
+			printf("%s:PASS:%s\n", __func__, tag);                 \
+		}                                                              \
+		__ret;                                                         \
+	})
+
+static int bpf_find_map(const char *test, struct bpf_object *obj,
+			const char *name)
+{
+	struct bpf_map *map;
+
+	map = bpf_object__find_map_by_name(obj, name);
+	if (!map)
+		return -1;
+	return bpf_map__fd(map);
+}
+
+int main(int argc, char **argv)
+{
+	const char *probe_name = "syscalls/sys_enter_open";
+	const char *file = "test_progenyof_kern.o";
+	int err, bytes, efd, prog_fd, pmu_fd;
+	int resultmap_fd, pidmap_fd;
+	struct perf_event_attr attr = {};
+	struct bpf_object *obj;
+	__u32 retrieved_ancestor_pid = 0;
+	__u32 key = 0, pid;
+	int exit_code = EXIT_FAILURE;
+	char buf[256];
+
+	int child_pid, ancestor_pid, root_fd, nonexistant = -42;
+	__u32 ancestor_key = 1;
+	int pipefd[2];
+	char marker[1];
+
+	err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
+	if (CHECK(err, "bpf_prog_load", "err %d errno %d\n", err, errno))
+		goto fail;
+
+	resultmap_fd = bpf_find_map(__func__, obj, "resultmap");
+	if (CHECK(resultmap_fd < 0, "bpf_find_map", "err %d errno %d\n",
+		  resultmap_fd, errno))
+		goto close_prog;
+
+	pidmap_fd = bpf_find_map(__func__, obj, "pidmap");
+	if (CHECK(pidmap_fd < 0, "bpf_find_map", "err %d errno %d\n", pidmap_fd,
+		  errno))
+		goto close_prog;
+
+	pid = getpid();
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+
+	snprintf(buf, sizeof(buf), "/sys/kernel/debug/tracing/events/%s/id",
+		 probe_name);
+	efd = open(buf, O_RDONLY, 0);
+	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+		goto close_prog;
+	bytes = read(efd, buf, sizeof(buf));
+	close(efd);
+	if (CHECK(bytes <= 0 || bytes >= sizeof(buf), "read",
+		  "bytes %d errno %d\n", bytes, errno))
+		goto close_prog;
+
+	attr.config = strtol(buf, NULL, 0);
+	attr.type = PERF_TYPE_TRACEPOINT;
+	attr.sample_type = PERF_SAMPLE_RAW;
+	attr.sample_period = 1;
+	attr.wakeup_events = 1;
+
+	pmu_fd = syscall(__NR_perf_event_open, &attr, getpid(), -1, -1, 0);
+	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n", pmu_fd,
+		  errno))
+		goto close_prog;
+
+	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
+	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n", err,
+		  errno))
+		goto close_pmu;
+
+	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
+	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n", err,
+		  errno))
+		goto close_pmu;
+
+	// Test on ourselve: progenyof(current->pid) is true
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != pid,
+		  "progenyof is true with same pid", "%d == %d\n",
+		  retrieved_ancestor_pid, pid))
+		goto close_pmu;
+
+	// Test that PID 1 is among our progeny
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = 1;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != ancestor_pid,
+		  "progenyof reaches init", "%d == %d\n",
+		  retrieved_ancestor_pid, ancestor_pid))
+		goto close_pmu;
+
+	// Test that PID 0 is among our progeny
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = 0;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != ancestor_pid,
+		  "progenyof does not go over init", "%d == %d\n",
+		  retrieved_ancestor_pid, ancestor_pid))
+		goto close_pmu;
+
+	// Test that we don't go over PID 0
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = -1;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != nonexistant,
+		  "progenyof does not go over init", "%d == %d\n",
+		  retrieved_ancestor_pid, nonexistant))
+		goto close_pmu;
+
+	// Test that we are among the progeny our child
+	pipe(pipefd);
+	child_pid = fork();
+	if (child_pid == -1) {
+		printf("fork failed\n");
+		goto close_pmu;
+	} else if (child_pid == 0) {
+		close(pipefd[1]);
+		read(pipefd[0], &marker, 1);
+
+		root_fd = open("/", O_RDONLY);
+		if (CHECK(efd < 0, "open", "errno %d\n", errno))
+			goto close_prog;
+		close(root_fd);
+
+		close(pipefd[0]);
+		_exit(EXIT_SUCCESS);
+	} else {
+		close(pipefd[0]);
+		bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+		bpf_map_update_elem(pidmap_fd, &key, &child_pid, 0);
+		bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+
+		write(pipefd[1], &marker, 1);
+		wait(NULL);
+		close(pipefd[1]);
+
+		err = bpf_map_lookup_elem(resultmap_fd, &key,
+					  &retrieved_ancestor_pid);
+		if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err,
+			  errno))
+			goto close_pmu;
+		if (CHECK(retrieved_ancestor_pid != pid, "progenyof of parent",
+			  "%d == %d\n", retrieved_ancestor_pid, pid))
+			goto close_pmu;
+	}
+
+	// Test that a child of ours doesn't belong to our progeny
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	pipe(pipefd);
+	child_pid = fork();
+	if (child_pid == -1) {
+		printf("fork failed\n");
+		goto close_pmu;
+	} else if (child_pid == 0) {
+		close(pipefd[1]);
+		read(pipefd[0], marker, 1);
+		close(pipefd[0]);
+		_exit(EXIT_SUCCESS);
+	} else {
+		close(pipefd[0]);
+
+		bpf_map_update_elem(pidmap_fd, &ancestor_key, &child_pid, 0);
+
+		root_fd = open("/", O_RDONLY);
+		if (CHECK(efd < 0, "open", "errno %d\n", errno))
+			goto close_prog;
+		close(root_fd);
+
+		write(pipefd[1], marker, 1);
+		wait(NULL);
+		close(pipefd[1]);
+
+		err = bpf_map_lookup_elem(resultmap_fd, &key,
+					  &retrieved_ancestor_pid);
+		if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err,
+			  errno))
+			goto close_pmu;
+		if (CHECK(retrieved_ancestor_pid != nonexistant, "progenyof of child",
+			  "%d == %d\n", retrieved_ancestor_pid, 0))
+			goto close_pmu;
+	}
+
+	exit_code = EXIT_SUCCESS;
+	printf("%s:PASS\n", argv[0]);
+
+close_pmu:
+	close(pmu_fd);
+close_prog:
+	bpf_object__close(obj);
+fail:
+	return exit_code;
+}
-- 
2.17.1

Re: [PATCH v3 bpf-next 0/3] bpf: add bpf_progenyof helper

[Daniel Borkmann]

On 03/05/2019 11:47 PM, Javier Honduvilla Coto wrote:
> Hi all,
> 
> This patch add the bpf_progenyof helper which receives a PID and returns
> 1 if the process currently being executed is in the process hierarchy,
> including itself or 0 if not.
> 
> This is very useful in tracing programs when we want to filter by a
> given PID and all the children it might have. The current workarounds
> most people implement for this purpose have issues:
> 
> - Attaching to process spawning syscalls and dynamically add those PIDs
>   to  some bpf map that would be used to filter is cumbersome and
> potentially racy.
> - Unrolling some loop to perform what this helper is doing consumes lots
>   of instructions. That and the impossibility to jump backwards makes it
> really hard to be correct in really large process chains.
> 
> Let me know what do you think!
> 
> Thanks,
> 
> ---
> Changed in V3:
>         - Removed RCU read (un)locking as BPF programs alredy run in RCU locked
>                 context
>         - progenyof(0) now returns 1, which, semantically makes more sense
>         - Added new test case for PID 0 and changed sentinel value for errors
>         - Rebase on latest bpf-next/master
>         - Used my work email as somehow I accidentally used my personal one in v2
> 
> Changed in V2:
>         - Adding missing docs in include/uapi/linux/bpf.h
> 
> Javier Honduvilla Coto (3):
>   bpf: add bpf_progenyof helper
>   bpf: sync kernel uapi headers
>   bpf: add tests for bpf_progenyof
> 
>  include/linux/bpf.h                           |   1 +
>  include/uapi/linux/bpf.h                      |  10 +-
>  kernel/bpf/core.c                             |   1 +
>  kernel/bpf/helpers.c                          |  32 +++
>  kernel/trace/bpf_trace.c                      |   2 +
>  tools/include/uapi/linux/bpf.h                |   9 +-
>  tools/testing/selftests/bpf/.gitignore        |   1 +
>  tools/testing/selftests/bpf/Makefile          |   2 +-
>  tools/testing/selftests/bpf/bpf_helpers.h     |   1 +
>  .../selftests/bpf/progs/test_progenyof_kern.c |  46 +++
>  .../selftests/bpf/test_progenyof_user.c       | 268 ++++++++++++++++++
>  11 files changed, 370 insertions(+), 3 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/progs/test_progenyof_kern.c
>  create mode 100644 tools/testing/selftests/bpf/test_progenyof_user.c
> 

bpf-next is closed right now due to merge window, please resubmit once it
opens, thanks!

[PATCH v4 bpf-next 0/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

Hi all,

This patch add the bpf_progenyof helper which receives a PID and returns
1 if the process currently being executed is in the process hierarchy,
including itself or 0 if not.

This is very useful in tracing programs when we want to filter by a
given PID and all the children it might have. The current workarounds
most people implement for this purpose have issues:

- Attaching to process spawning syscalls and dynamically add those PIDs
  to  some bpf map that would be used to filter is cumbersome and
potentially racy.
- Unrolling some loop to perform what this helper is doing consumes lots
  of instructions. That and the impossibility to jump backwards makes it
really hard to be correct in really large process chains.

Let me know what do you think!

Thanks,

---
Changes in V4:
        - Rebased on latest bpf-next after merge window

Changes in V3:
        - Removed RCU read (un)locking as BPF programs alredy run in RCU locked
                context
        - progenyof(0) now returns 1, which, semantically makes more sense
        - Added new test case for PID 0 and changed sentinel value for errors
        - Rebase on latest bpf-next/master
        - Used my work email as somehow I accidentally used my personal one in v2

Changes in V2:
        - Adding missing docs in include/uapi/linux/bpf.h

Javier Honduvilla Coto (3):
  bpf: add bpf_progenyof helper
  bpf: sync kernel uapi headers
  bpf: add tests for bpf_progenyof

 include/linux/bpf.h                           |   1 +
 include/uapi/linux/bpf.h                      |  10 +-
 kernel/bpf/core.c                             |   1 +
 kernel/bpf/helpers.c                          |  32 +++
 kernel/trace/bpf_trace.c                      |   2 +
 tools/include/uapi/linux/bpf.h                |  10 +-
 tools/testing/selftests/bpf/.gitignore        |   1 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   1 +
 .../selftests/bpf/progs/test_progenyof_kern.c |  46 +++
 .../selftests/bpf/test_progenyof_user.c       | 268 ++++++++++++++++++
 11 files changed, 371 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_progenyof_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_progenyof_user.c

-- 
2.17.1

[PATCH v4 bpf-next 1/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

This patch adds the bpf_progenyof helper which receives a PID and returns
1 if the process currently being executed is in the process hierarchy
including itself or 0 if not.

This is very useful in tracing programs when we want to filter by a
given PID and all the children it might spawn. The current workarounds
most people implement for this purpose have issues:

- Attaching to process spawning syscalls and dynamically add those PIDs
  to some bpf map that would be used to filter is cumbersome and
potentially racy.
- Unrolling some loop to perform what this helper is doing consumes lots
  of instructions. That and the impossibility to jump backwards makes it
really hard to be correct in really large process chains.

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 include/linux/bpf.h      |  1 +
 include/uapi/linux/bpf.h | 10 +++++++++-
 kernel/bpf/core.c        |  1 +
 kernel/bpf/helpers.c     | 32 ++++++++++++++++++++++++++++++++
 kernel/trace/bpf_trace.c |  2 ++
 5 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index f62897198844..bd0d2b38e7d5 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -930,6 +930,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
 extern const struct bpf_func_proto bpf_spin_lock_proto;
 extern const struct bpf_func_proto bpf_spin_unlock_proto;
 extern const struct bpf_func_proto bpf_get_local_storage_proto;
+extern const struct bpf_func_proto bpf_progenyof_proto;
 
 /* Shared helpers among cBPF and eBPF. */
 void bpf_user_rnd_init_once(void);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 3c04410137d9..cf54cc739bf4 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2463,6 +2463,13 @@ union bpf_attr {
  * 	Return
  * 		0 if iph and th are a valid SYN cookie ACK, or a negative error
  * 		otherwise.
+ * int bpf_progenyof(int pid)
+ *	Description
+ *		This helper is useful in programs that want to filter events
+ *		happening to a pid of any of its descendants.
+ *	Return
+ *		1 if the currently executing process' pid is in the process
+ *		hierarchy of the passed pid. 0 Otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2565,7 +2572,8 @@ union bpf_attr {
 	FN(skb_ecn_set_ce),		\
 	FN(get_listener_sock),		\
 	FN(skc_lookup_tcp),		\
-	FN(tcp_check_syncookie),
+	FN(tcp_check_syncookie),	\
+	FN(progenyof),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ff09d32a8a1b..437986497468 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2044,6 +2044,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
 const struct bpf_func_proto bpf_get_current_comm_proto __weak;
 const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
 const struct bpf_func_proto bpf_get_local_storage_proto __weak;
+const struct bpf_func_proto bpf_progenyof_proto __weak;
 
 const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
 {
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index a411fc17d265..f093b35d1ba8 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -18,6 +18,7 @@
 #include <linux/sched.h>
 #include <linux/uidgid.h>
 #include <linux/filter.h>
+#include <linux/init_task.h>
 
 /* If kernel subsystem is allowing eBPF programs to call this function,
  * inside its own verifier_ops->get_func_proto() callback it should return
@@ -364,3 +365,34 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
 };
 #endif
 #endif
+
+BPF_CALL_1(bpf_progenyof, int, pid)
+{
+	int result = 0;
+	struct task_struct *task = current;
+
+	WARN_ON(!rcu_read_lock_held());
+
+	if (unlikely(!task))
+		return -EINVAL;
+
+	if (pid == 0)
+		return 1;
+
+	while (task != &init_task) {
+		if (task->pid == pid) {
+			result = 1;
+			break;
+		}
+		task = rcu_dereference(task->real_parent);
+	}
+
+	return result;
+}
+
+const struct bpf_func_proto bpf_progenyof_proto = {
+	.func		= bpf_progenyof,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_ANYTHING,
+};
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d64c00afceb5..e69283d423ff 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -599,6 +599,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_prandom_u32_proto;
 	case BPF_FUNC_probe_read_str:
 		return &bpf_probe_read_str_proto;
+	case BPF_FUNC_progenyof:
+		return &bpf_progenyof_proto;
 #ifdef CONFIG_CGROUPS
 	case BPF_FUNC_get_current_cgroup_id:
 		return &bpf_get_current_cgroup_id_proto;
-- 
2.17.1

[PATCH v4 bpf-next 2/3] bpf: sync kernel uapi headers

[Javier Honduvilla Coto]

Sync kernel uapi headers.

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 tools/include/uapi/linux/bpf.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 3c04410137d9..cf54cc739bf4 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2463,6 +2463,13 @@ union bpf_attr {
  * 	Return
  * 		0 if iph and th are a valid SYN cookie ACK, or a negative error
  * 		otherwise.
+ * int bpf_progenyof(int pid)
+ *	Description
+ *		This helper is useful in programs that want to filter events
+ *		happening to a pid of any of its descendants.
+ *	Return
+ *		1 if the currently executing process' pid is in the process
+ *		hierarchy of the passed pid. 0 Otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2565,7 +2572,8 @@ union bpf_attr {
 	FN(skb_ecn_set_ce),		\
 	FN(get_listener_sock),		\
 	FN(skc_lookup_tcp),		\
-	FN(tcp_check_syncookie),
+	FN(tcp_check_syncookie),	\
+	FN(progenyof),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
-- 
2.17.1

[PATCH v4 bpf-next 3/3] bpf: add tests for bpf_progenyof

[Javier Honduvilla Coto]

Adding the following test cases:

- progenyof(current->pid) == 1
- progenyof(current->real_parent->pid) == 1
- progenyof(1) == 1
- progenyof(0) == 1

- progenyof(-1) == 0
- progenyof(current->children[0]->pid) == 0

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 tools/testing/selftests/bpf/.gitignore        |   1 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   1 +
 .../selftests/bpf/progs/test_progenyof_kern.c |  46 +++
 .../selftests/bpf/test_progenyof_user.c       | 268 ++++++++++++++++++
 5 files changed, 317 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_progenyof_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_progenyof_user.c

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 41e8a689aa77..1c117533ee60 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -32,3 +32,4 @@ test_tcpnotify_user
 test_libbpf
 test_tcp_check_syncookie_user
 alu32
+test_progenyof_user
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index edd59707cb1f..9f3c2a8be23b 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -23,7 +23,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
 	test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \
 	test_sock test_btf test_sockmap test_lirc_mode2_user get_cgroup_id_user \
 	test_socket_cookie test_cgroup_storage test_select_reuseport test_section_names \
-	test_netcnt test_tcpnotify_user test_sock_fields
+	test_netcnt test_tcpnotify_user test_sock_fields test_progenyof_user
 
 BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
 TEST_GEN_FILES = $(BPF_OBJ_FILES)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 97d140961438..b60f1363384a 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -192,6 +192,7 @@ static int (*bpf_skb_ecn_set_ce)(void *ctx) =
 static int (*bpf_tcp_check_syncookie)(struct bpf_sock *sk,
 	    void *ip, int ip_len, void *tcp, int tcp_len) =
 	(void *) BPF_FUNC_tcp_check_syncookie;
+static int (*bpf_progenyof)(int pid) = (void *) BPF_FUNC_progenyof;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions
diff --git a/tools/testing/selftests/bpf/progs/test_progenyof_kern.c b/tools/testing/selftests/bpf/progs/test_progenyof_kern.c
new file mode 100644
index 000000000000..c4be2c794d83
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_progenyof_kern.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct bpf_map_def SEC("maps") pidmap = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(__u32),
+	.value_size = sizeof(__u32),
+	.max_entries = 2,
+};
+
+struct bpf_map_def SEC("maps") resultmap = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(__u32),
+	.value_size = sizeof(__u32),
+	.max_entries = 1,
+};
+
+SEC("tracepoint/syscalls/sys_enter_open")
+int trace(void *ctx)
+{
+	__u32 pid = bpf_get_current_pid_tgid();
+	__u32 current_key = 0, ancestor_key = 1, *expected_pid, *ancestor_pid;
+	__u32 *val;
+
+	expected_pid = bpf_map_lookup_elem(&pidmap, &current_key);
+	if (!expected_pid || *expected_pid != pid)
+		return 0;
+
+	ancestor_pid = bpf_map_lookup_elem(&pidmap, &ancestor_key);
+	if (!ancestor_pid)
+		return 0;
+
+	if (!bpf_progenyof(*ancestor_pid))
+		return 0;
+
+	val = bpf_map_lookup_elem(&resultmap, &current_key);
+	if (val)
+		*val = *ancestor_pid;
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
diff --git a/tools/testing/selftests/bpf/test_progenyof_user.c b/tools/testing/selftests/bpf/test_progenyof_user.c
new file mode 100644
index 000000000000..a395eb2473b5
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_progenyof_user.c
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <syscall.h>
+#include <unistd.h>
+#include <linux/perf_event.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#define CHECK(condition, tag, format...)                                       \
+	({                                                                     \
+		int __ret = !!(condition);                                     \
+		if (__ret) {                                                   \
+			printf("%s:FAIL:%s ", __func__, tag);                  \
+			printf(format);                                        \
+		} else {                                                       \
+			printf("%s:PASS:%s\n", __func__, tag);                 \
+		}                                                              \
+		__ret;                                                         \
+	})
+
+static int bpf_find_map(const char *test, struct bpf_object *obj,
+			const char *name)
+{
+	struct bpf_map *map;
+
+	map = bpf_object__find_map_by_name(obj, name);
+	if (!map)
+		return -1;
+	return bpf_map__fd(map);
+}
+
+int main(int argc, char **argv)
+{
+	const char *probe_name = "syscalls/sys_enter_open";
+	const char *file = "test_progenyof_kern.o";
+	int err, bytes, efd, prog_fd, pmu_fd;
+	int resultmap_fd, pidmap_fd;
+	struct perf_event_attr attr = {};
+	struct bpf_object *obj;
+	__u32 retrieved_ancestor_pid = 0;
+	__u32 key = 0, pid;
+	int exit_code = EXIT_FAILURE;
+	char buf[256];
+
+	int child_pid, ancestor_pid, root_fd, nonexistant = -42;
+	__u32 ancestor_key = 1;
+	int pipefd[2];
+	char marker[1];
+
+	err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
+	if (CHECK(err, "bpf_prog_load", "err %d errno %d\n", err, errno))
+		goto fail;
+
+	resultmap_fd = bpf_find_map(__func__, obj, "resultmap");
+	if (CHECK(resultmap_fd < 0, "bpf_find_map", "err %d errno %d\n",
+		  resultmap_fd, errno))
+		goto close_prog;
+
+	pidmap_fd = bpf_find_map(__func__, obj, "pidmap");
+	if (CHECK(pidmap_fd < 0, "bpf_find_map", "err %d errno %d\n", pidmap_fd,
+		  errno))
+		goto close_prog;
+
+	pid = getpid();
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+
+	snprintf(buf, sizeof(buf), "/sys/kernel/debug/tracing/events/%s/id",
+		 probe_name);
+	efd = open(buf, O_RDONLY, 0);
+	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+		goto close_prog;
+	bytes = read(efd, buf, sizeof(buf));
+	close(efd);
+	if (CHECK(bytes <= 0 || bytes >= sizeof(buf), "read",
+		  "bytes %d errno %d\n", bytes, errno))
+		goto close_prog;
+
+	attr.config = strtol(buf, NULL, 0);
+	attr.type = PERF_TYPE_TRACEPOINT;
+	attr.sample_type = PERF_SAMPLE_RAW;
+	attr.sample_period = 1;
+	attr.wakeup_events = 1;
+
+	pmu_fd = syscall(__NR_perf_event_open, &attr, getpid(), -1, -1, 0);
+	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n", pmu_fd,
+		  errno))
+		goto close_prog;
+
+	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
+	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n", err,
+		  errno))
+		goto close_pmu;
+
+	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
+	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n", err,
+		  errno))
+		goto close_pmu;
+
+	// Test on ourselve: progenyof(current->pid) is true
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != pid,
+		  "progenyof is true with same pid", "%d == %d\n",
+		  retrieved_ancestor_pid, pid))
+		goto close_pmu;
+
+	// Test that PID 1 is among our progeny
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = 1;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != ancestor_pid,
+		  "progenyof reaches init", "%d == %d\n",
+		  retrieved_ancestor_pid, ancestor_pid))
+		goto close_pmu;
+
+	// Test that PID 0 is among our progeny
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = 0;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != ancestor_pid,
+		  "progenyof does not go over init", "%d == %d\n",
+		  retrieved_ancestor_pid, ancestor_pid))
+		goto close_pmu;
+
+	// Test that we don't go over PID 0
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = -1;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != nonexistant,
+		  "progenyof does not go over init", "%d == %d\n",
+		  retrieved_ancestor_pid, nonexistant))
+		goto close_pmu;
+
+	// Test that we are among the progeny our child
+	pipe(pipefd);
+	child_pid = fork();
+	if (child_pid == -1) {
+		printf("fork failed\n");
+		goto close_pmu;
+	} else if (child_pid == 0) {
+		close(pipefd[1]);
+		read(pipefd[0], &marker, 1);
+
+		root_fd = open("/", O_RDONLY);
+		if (CHECK(efd < 0, "open", "errno %d\n", errno))
+			goto close_prog;
+		close(root_fd);
+
+		close(pipefd[0]);
+		_exit(EXIT_SUCCESS);
+	} else {
+		close(pipefd[0]);
+		bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+		bpf_map_update_elem(pidmap_fd, &key, &child_pid, 0);
+		bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+
+		write(pipefd[1], &marker, 1);
+		wait(NULL);
+		close(pipefd[1]);
+
+		err = bpf_map_lookup_elem(resultmap_fd, &key,
+					  &retrieved_ancestor_pid);
+		if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err,
+			  errno))
+			goto close_pmu;
+		if (CHECK(retrieved_ancestor_pid != pid, "progenyof of parent",
+			  "%d == %d\n", retrieved_ancestor_pid, pid))
+			goto close_pmu;
+	}
+
+	// Test that a child of ours doesn't belong to our progeny
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	pipe(pipefd);
+	child_pid = fork();
+	if (child_pid == -1) {
+		printf("fork failed\n");
+		goto close_pmu;
+	} else if (child_pid == 0) {
+		close(pipefd[1]);
+		read(pipefd[0], marker, 1);
+		close(pipefd[0]);
+		_exit(EXIT_SUCCESS);
+	} else {
+		close(pipefd[0]);
+
+		bpf_map_update_elem(pidmap_fd, &ancestor_key, &child_pid, 0);
+
+		root_fd = open("/", O_RDONLY);
+		if (CHECK(efd < 0, "open", "errno %d\n", errno))
+			goto close_prog;
+		close(root_fd);
+
+		write(pipefd[1], marker, 1);
+		wait(NULL);
+		close(pipefd[1]);
+
+		err = bpf_map_lookup_elem(resultmap_fd, &key,
+					  &retrieved_ancestor_pid);
+		if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err,
+			  errno))
+			goto close_pmu;
+		if (CHECK(retrieved_ancestor_pid != nonexistant, "progenyof of child",
+			  "%d == %d\n", retrieved_ancestor_pid, 0))
+			goto close_pmu;
+	}
+
+	exit_code = EXIT_SUCCESS;
+	printf("%s:PASS\n", argv[0]);
+
+close_pmu:
+	close(pmu_fd);
+close_prog:
+	bpf_object__close(obj);
+fail:
+	return exit_code;
+}
-- 
2.17.1

Re: [PATCH v3 bpf-next 0/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

On Thu, Mar 07, 2019 at 10:26:22AM +0100, Daniel Borkmann wrote:
> On 03/05/2019 11:47 PM, Javier Honduvilla Coto wrote:
> > Hi all,
> >
> > This patch add the bpf_progenyof helper which receives a PID and returns
> > 1 if the process currently being executed is in the process hierarchy,
> > including itself or 0 if not.
> >
> > This is very useful in tracing programs when we want to filter by a
> > given PID and all the children it might have. The current workarounds
> > most people implement for this purpose have issues:
> >
> > - Attaching to process spawning syscalls and dynamically add those PIDs
> >   to  some bpf map that would be used to filter is cumbersome and
> > potentially racy.
> > - Unrolling some loop to perform what this helper is doing consumes lots
> >   of instructions. That and the impossibility to jump backwards makes it
> > really hard to be correct in really large process chains.
> >
> > Let me know what do you think!
> >
> > Thanks,
> >
> > ---
> > Changed in V3:
> >         - Removed RCU read (un)locking as BPF programs alredy run in RCU locked
> >                 context
> >         - progenyof(0) now returns 1, which, semantically makes more sense
> >         - Added new test case for PID 0 and changed sentinel value for errors
> >         - Rebase on latest bpf-next/master
> >         - Used my work email as somehow I accidentally used my personal one in v2
> >
> > Changed in V2:
> >         - Adding missing docs in include/uapi/linux/bpf.h
> >
> > Javier Honduvilla Coto (3):
> >   bpf: add bpf_progenyof helper
> >   bpf: sync kernel uapi headers
> >   bpf: add tests for bpf_progenyof
> >
> >  include/linux/bpf.h                           |   1 +
> >  include/uapi/linux/bpf.h                      |  10 +-
> >  kernel/bpf/core.c                             |   1 +
> >  kernel/bpf/helpers.c                          |  32 +++
> >  kernel/trace/bpf_trace.c                      |   2 +
> >  tools/include/uapi/linux/bpf.h                |   9 +-
> >  tools/testing/selftests/bpf/.gitignore        |   1 +
> >  tools/testing/selftests/bpf/Makefile          |   2 +-
> >  tools/testing/selftests/bpf/bpf_helpers.h     |   1 +
> >  .../selftests/bpf/progs/test_progenyof_kern.c |  46 +++
> >  .../selftests/bpf/test_progenyof_user.c       | 268 ++++++++++++++++++
> >  11 files changed, 370 insertions(+), 3 deletions(-)
> >  create mode 100644 tools/testing/selftests/bpf/progs/test_progenyof_kern.c
> >  create mode 100644 tools/testing/selftests/bpf/test_progenyof_user.c
> >
>
> bpf-next is closed right now due to merge window, please resubmit once it
> opens, thanks!

Just did! Thanks :)

Re: [PATCH v4 bpf-next 1/3] bpf: add bpf_progenyof helper

[Daniel Borkmann]

On 03/22/2019 11:38 PM, Javier Honduvilla Coto wrote:
> This patch adds the bpf_progenyof helper which receives a PID and returns
> 1 if the process currently being executed is in the process hierarchy
> including itself or 0 if not.
> 
> This is very useful in tracing programs when we want to filter by a
> given PID and all the children it might spawn. The current workarounds
> most people implement for this purpose have issues:
> 
> - Attaching to process spawning syscalls and dynamically add those PIDs
>   to some bpf map that would be used to filter is cumbersome and
> potentially racy.
> - Unrolling some loop to perform what this helper is doing consumes lots
>   of instructions. That and the impossibility to jump backwards makes it
> really hard to be correct in really large process chains.
> 
> Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> ---
>  include/linux/bpf.h      |  1 +
>  include/uapi/linux/bpf.h | 10 +++++++++-
>  kernel/bpf/core.c        |  1 +
>  kernel/bpf/helpers.c     | 32 ++++++++++++++++++++++++++++++++
>  kernel/trace/bpf_trace.c |  2 ++
>  5 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index f62897198844..bd0d2b38e7d5 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -930,6 +930,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
>  extern const struct bpf_func_proto bpf_spin_lock_proto;
>  extern const struct bpf_func_proto bpf_spin_unlock_proto;
>  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> +extern const struct bpf_func_proto bpf_progenyof_proto;
>  
>  /* Shared helpers among cBPF and eBPF. */
>  void bpf_user_rnd_init_once(void);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 3c04410137d9..cf54cc739bf4 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2463,6 +2463,13 @@ union bpf_attr {
>   * 	Return
>   * 		0 if iph and th are a valid SYN cookie ACK, or a negative error
>   * 		otherwise.
> + * int bpf_progenyof(int pid)
> + *	Description
> + *		This helper is useful in programs that want to filter events
> + *		happening to a pid of any of its descendants.
> + *	Return
> + *		1 if the currently executing process' pid is in the process
> + *		hierarchy of the passed pid. 0 Otherwise.

What about the -EINVAL?

>   */
>  #define __BPF_FUNC_MAPPER(FN)		\
>  	FN(unspec),			\
> @@ -2565,7 +2572,8 @@ union bpf_attr {
>  	FN(skb_ecn_set_ce),		\
>  	FN(get_listener_sock),		\
>  	FN(skc_lookup_tcp),		\
> -	FN(tcp_check_syncookie),
> +	FN(tcp_check_syncookie),	\
> +	FN(progenyof),
>  
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index ff09d32a8a1b..437986497468 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2044,6 +2044,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
>  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
>  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
>  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> +const struct bpf_func_proto bpf_progenyof_proto __weak;
>  
>  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
>  {
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index a411fc17d265..f093b35d1ba8 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -18,6 +18,7 @@
>  #include <linux/sched.h>
>  #include <linux/uidgid.h>
>  #include <linux/filter.h>
> +#include <linux/init_task.h>
>  
>  /* If kernel subsystem is allowing eBPF programs to call this function,
>   * inside its own verifier_ops->get_func_proto() callback it should return
> @@ -364,3 +365,34 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
>  };
>  #endif
>  #endif
> +
> +BPF_CALL_1(bpf_progenyof, int, pid)

Nit: could we add a more descriptive helper name? What's progenyof? Also s/int/pid_t/?

> +{
> +	int result = 0;
> +	struct task_struct *task = current;
> +
> +	WARN_ON(!rcu_read_lock_held());

This is not needed since it's guaranteed in BPF.

> +	if (unlikely(!task))
> +		return -EINVAL;
> +
> +	if (pid == 0)
> +		return 1;
> +
> +	while (task != &init_task) {
> +		if (task->pid == pid) {
> +			result = 1;
> +			break;
> +		}
> +		task = rcu_dereference(task->real_parent);
> +	}
> +
> +	return result;
> +}
> +
> +const struct bpf_func_proto bpf_progenyof_proto = {
> +	.func		= bpf_progenyof,
> +	.gpl_only	= false,
> +	.ret_type	= RET_INTEGER,
> +	.arg1_type	= ARG_ANYTHING,
> +};
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index d64c00afceb5..e69283d423ff 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -599,6 +599,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		return &bpf_get_prandom_u32_proto;
>  	case BPF_FUNC_probe_read_str:
>  		return &bpf_probe_read_str_proto;
> +	case BPF_FUNC_progenyof:
> +		return &bpf_progenyof_proto;
>  #ifdef CONFIG_CGROUPS
>  	case BPF_FUNC_get_current_cgroup_id:
>  		return &bpf_get_current_cgroup_id_proto;
> 

Re: [PATCH v4 bpf-next 1/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

On Mon, Mar 25, 2019 at 03:17:03PM +0100, Daniel Borkmann wrote:
> On 03/22/2019 11:38 PM, Javier Honduvilla Coto wrote:
> > This patch adds the bpf_progenyof helper which receives a PID and returns
> > 1 if the process currently being executed is in the process hierarchy
> > including itself or 0 if not.
> >
> > This is very useful in tracing programs when we want to filter by a
> > given PID and all the children it might spawn. The current workarounds
> > most people implement for this purpose have issues:
> >
> > - Attaching to process spawning syscalls and dynamically add those PIDs
> >   to some bpf map that would be used to filter is cumbersome and
> > potentially racy.
> > - Unrolling some loop to perform what this helper is doing consumes lots
> >   of instructions. That and the impossibility to jump backwards makes it
> > really hard to be correct in really large process chains.
> >
> > Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> > ---
> >  include/linux/bpf.h      |  1 +
> >  include/uapi/linux/bpf.h | 10 +++++++++-
> >  kernel/bpf/core.c        |  1 +
> >  kernel/bpf/helpers.c     | 32 ++++++++++++++++++++++++++++++++
> >  kernel/trace/bpf_trace.c |  2 ++
> >  5 files changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index f62897198844..bd0d2b38e7d5 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -930,6 +930,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
> >  extern const struct bpf_func_proto bpf_spin_lock_proto;
> >  extern const struct bpf_func_proto bpf_spin_unlock_proto;
> >  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> > +extern const struct bpf_func_proto bpf_progenyof_proto;
> >
> >  /* Shared helpers among cBPF and eBPF. */
> >  void bpf_user_rnd_init_once(void);
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 3c04410137d9..cf54cc739bf4 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -2463,6 +2463,13 @@ union bpf_attr {
> >   * 	Return
> >   * 		0 if iph and th are a valid SYN cookie ACK, or a negative error
> >   * 		otherwise.
> > + * int bpf_progenyof(int pid)
> > + *	Description
> > + *		This helper is useful in programs that want to filter events
> > + *		happening to a pid of any of its descendants.
> > + *	Return
> > + *		1 if the currently executing process' pid is in the process
> > + *		hierarchy of the passed pid. 0 Otherwise.
>
> What about the -EINVAL?

I think we can remove this, as Alexei told me this was not needed
anymore (copied it from other helpers that have it).

Should we remove that check in other patch for the other helpers
that have it?

>
> >   */
> >  #define __BPF_FUNC_MAPPER(FN)		\
> >  	FN(unspec),			\
> > @@ -2565,7 +2572,8 @@ union bpf_attr {
> >  	FN(skb_ecn_set_ce),		\
> >  	FN(get_listener_sock),		\
> >  	FN(skc_lookup_tcp),		\
> > -	FN(tcp_check_syncookie),
> > +	FN(tcp_check_syncookie),	\
> > +	FN(progenyof),
> >
> >  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> >   * function eBPF program intends to call
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index ff09d32a8a1b..437986497468 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -2044,6 +2044,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
> >  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
> >  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
> >  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> > +const struct bpf_func_proto bpf_progenyof_proto __weak;
> >
> >  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
> >  {
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index a411fc17d265..f093b35d1ba8 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/sched.h>
> >  #include <linux/uidgid.h>
> >  #include <linux/filter.h>
> > +#include <linux/init_task.h>
> >
> >  /* If kernel subsystem is allowing eBPF programs to call this function,
> >   * inside its own verifier_ops->get_func_proto() callback it should return
> > @@ -364,3 +365,34 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
> >  };
> >  #endif
> >  #endif
> > +
> > +BPF_CALL_1(bpf_progenyof, int, pid)
>
> Nit: could we add a more descriptive helper name? What's progenyof? Also s/int/pid_t/?
>

It's true that "progeny" is not a very commonly used word :D. A coworker
suggested "descendantof", what do you think? It's a bit difficult to
convey "is true on the passed pid + on all the process under the
hierarchy chain of that pid", so I will try rewording the docs so the
semantics are very clear!

Will change to pid_t :) Thanks!

> > +{
> > +	int result = 0;
> > +	struct task_struct *task = current;
> > +
> > +	WARN_ON(!rcu_read_lock_held());
>
> This is not needed since it's guaranteed in BPF.
>
> > +	if (unlikely(!task))
> > +		return -EINVAL;
> > +
> > +	if (pid == 0)
> > +		return 1;
> > +
> > +	while (task != &init_task) {
> > +		if (task->pid == pid) {
> > +			result = 1;
> > +			break;
> > +		}
> > +		task = rcu_dereference(task->real_parent);
> > +	}
> > +
> > +	return result;
> > +}
> > +
> > +const struct bpf_func_proto bpf_progenyof_proto = {
> > +	.func		= bpf_progenyof,
> > +	.gpl_only	= false,
> > +	.ret_type	= RET_INTEGER,
> > +	.arg1_type	= ARG_ANYTHING,
> > +};
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index d64c00afceb5..e69283d423ff 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -599,6 +599,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >  		return &bpf_get_prandom_u32_proto;
> >  	case BPF_FUNC_probe_read_str:
> >  		return &bpf_probe_read_str_proto;
> > +	case BPF_FUNC_progenyof:
> > +		return &bpf_progenyof_proto;
> >  #ifdef CONFIG_CGROUPS
> >  	case BPF_FUNC_get_current_cgroup_id:
> >  		return &bpf_get_current_cgroup_id_proto;
> >
>

Re: [PATCH v4 bpf-next 1/3] bpf: add bpf_progenyof helper

[Javier Honduvilla Coto]

On Mon, Mar 25, 2019 at 03:17:03PM +0100, Daniel Borkmann wrote:
> On 03/22/2019 11:38 PM, Javier Honduvilla Coto wrote:
> > This patch adds the bpf_progenyof helper which receives a PID and returns
> > 1 if the process currently being executed is in the process hierarchy
> > including itself or 0 if not.
> >
> > This is very useful in tracing programs when we want to filter by a
> > given PID and all the children it might spawn. The current workarounds
> > most people implement for this purpose have issues:
> >
> > - Attaching to process spawning syscalls and dynamically add those PIDs
> >   to some bpf map that would be used to filter is cumbersome and
> > potentially racy.
> > - Unrolling some loop to perform what this helper is doing consumes lots
> >   of instructions. That and the impossibility to jump backwards makes it
> > really hard to be correct in really large process chains.
> >
> > Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> > ---
> >  include/linux/bpf.h      |  1 +
> >  include/uapi/linux/bpf.h | 10 +++++++++-
> >  kernel/bpf/core.c        |  1 +
> >  kernel/bpf/helpers.c     | 32 ++++++++++++++++++++++++++++++++
> >  kernel/trace/bpf_trace.c |  2 ++
> >  5 files changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index f62897198844..bd0d2b38e7d5 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -930,6 +930,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
> >  extern const struct bpf_func_proto bpf_spin_lock_proto;
> >  extern const struct bpf_func_proto bpf_spin_unlock_proto;
> >  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> > +extern const struct bpf_func_proto bpf_progenyof_proto;
> >
> >  /* Shared helpers among cBPF and eBPF. */
> >  void bpf_user_rnd_init_once(void);
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 3c04410137d9..cf54cc739bf4 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -2463,6 +2463,13 @@ union bpf_attr {
> >   * 	Return
> >   * 		0 if iph and th are a valid SYN cookie ACK, or a negative error
> >   * 		otherwise.
> > + * int bpf_progenyof(int pid)
> > + *	Description
> > + *		This helper is useful in programs that want to filter events
> > + *		happening to a pid of any of its descendants.
> > + *	Return
> > + *		1 if the currently executing process' pid is in the process
> > + *		hierarchy of the passed pid. 0 Otherwise.
>
> What about the -EINVAL?
>
> >   */
> >  #define __BPF_FUNC_MAPPER(FN)		\
> >  	FN(unspec),			\
> > @@ -2565,7 +2572,8 @@ union bpf_attr {
> >  	FN(skb_ecn_set_ce),		\
> >  	FN(get_listener_sock),		\
> >  	FN(skc_lookup_tcp),		\
> > -	FN(tcp_check_syncookie),
> > +	FN(tcp_check_syncookie),	\
> > +	FN(progenyof),
> >
> >  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> >   * function eBPF program intends to call
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index ff09d32a8a1b..437986497468 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -2044,6 +2044,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
> >  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
> >  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
> >  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> > +const struct bpf_func_proto bpf_progenyof_proto __weak;
> >
> >  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
> >  {
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index a411fc17d265..f093b35d1ba8 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/sched.h>
> >  #include <linux/uidgid.h>
> >  #include <linux/filter.h>
> > +#include <linux/init_task.h>
> >
> >  /* If kernel subsystem is allowing eBPF programs to call this function,
> >   * inside its own verifier_ops->get_func_proto() callback it should return
> > @@ -364,3 +365,34 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
> >  };
> >  #endif
> >  #endif
> > +
> > +BPF_CALL_1(bpf_progenyof, int, pid)
>
> Nit: could we add a more descriptive helper name? What's progenyof? Also s/int/pid_t/?
>
> > +{
> > +	int result = 0;
> > +	struct task_struct *task = current;
> > +
> > +	WARN_ON(!rcu_read_lock_held());
>
> This is not needed since it's guaranteed in BPF.

Sure, thanks!

>
> > +	if (unlikely(!task))
> > +		return -EINVAL;
> > +
> > +	if (pid == 0)
> > +		return 1;
> > +
> > +	while (task != &init_task) {
> > +		if (task->pid == pid) {
> > +			result = 1;
> > +			break;
> > +		}
> > +		task = rcu_dereference(task->real_parent);
> > +	}
> > +
> > +	return result;
> > +}
> > +
> > +const struct bpf_func_proto bpf_progenyof_proto = {
> > +	.func		= bpf_progenyof,
> > +	.gpl_only	= false,
> > +	.ret_type	= RET_INTEGER,
> > +	.arg1_type	= ARG_ANYTHING,
> > +};
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index d64c00afceb5..e69283d423ff 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -599,6 +599,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >  		return &bpf_get_prandom_u32_proto;
> >  	case BPF_FUNC_probe_read_str:
> >  		return &bpf_probe_read_str_proto;
> > +	case BPF_FUNC_progenyof:
> > +		return &bpf_progenyof_proto;
> >  #ifdef CONFIG_CGROUPS
> >  	case BPF_FUNC_get_current_cgroup_id:
> >  		return &bpf_get_current_cgroup_id_proto;
> >
>

Re: [PATCH v4 bpf-next 1/3] bpf: add bpf_progenyof helper

[Brendan Gregg]

On Wed, Mar 27, 2019 at 8:59 AM Javier Honduvilla Coto
<javierhonduco@fb.com> wrote:
>
> On Mon, Mar 25, 2019 at 03:17:03PM +0100, Daniel Borkmann wrote:
> > On 03/22/2019 11:38 PM, Javier Honduvilla Coto wrote:
> > > This patch adds the bpf_progenyof helper which receives a PID and returns
> > > 1 if the process currently being executed is in the process hierarchy
> > > including itself or 0 if not.
> > >
> > > This is very useful in tracing programs when we want to filter by a
> > > given PID and all the children it might spawn. The current workarounds
> > > most people implement for this purpose have issues:
> > >
> > > - Attaching to process spawning syscalls and dynamically add those PIDs
> > >   to some bpf map that would be used to filter is cumbersome and
> > > potentially racy.
> > > - Unrolling some loop to perform what this helper is doing consumes lots
> > >   of instructions. That and the impossibility to jump backwards makes it
> > > really hard to be correct in really large process chains.
> > >
> > > Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> > > ---
> > >  include/linux/bpf.h      |  1 +
> > >  include/uapi/linux/bpf.h | 10 +++++++++-
> > >  kernel/bpf/core.c        |  1 +
> > >  kernel/bpf/helpers.c     | 32 ++++++++++++++++++++++++++++++++
> > >  kernel/trace/bpf_trace.c |  2 ++
> > >  5 files changed, 45 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > > index f62897198844..bd0d2b38e7d5 100644
> > > --- a/include/linux/bpf.h
> > > +++ b/include/linux/bpf.h
> > > @@ -930,6 +930,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
> > >  extern const struct bpf_func_proto bpf_spin_lock_proto;
> > >  extern const struct bpf_func_proto bpf_spin_unlock_proto;
> > >  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> > > +extern const struct bpf_func_proto bpf_progenyof_proto;
> > >
> > >  /* Shared helpers among cBPF and eBPF. */
> > >  void bpf_user_rnd_init_once(void);
> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index 3c04410137d9..cf54cc739bf4 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -2463,6 +2463,13 @@ union bpf_attr {
> > >   *         Return
> > >   *                 0 if iph and th are a valid SYN cookie ACK, or a negative error
> > >   *                 otherwise.
> > > + * int bpf_progenyof(int pid)
> > > + * Description
> > > + *         This helper is useful in programs that want to filter events
> > > + *         happening to a pid of any of its descendants.
> > > + * Return
> > > + *         1 if the currently executing process' pid is in the process
> > > + *         hierarchy of the passed pid. 0 Otherwise.
> >
> > What about the -EINVAL?
>
> I think we can remove this, as Alexei told me this was not needed
> anymore (copied it from other helpers that have it).
>
> Should we remove that check in other patch for the other helpers
> that have it?
>
> >
> > >   */
> > >  #define __BPF_FUNC_MAPPER(FN)              \
> > >     FN(unspec),                     \
> > > @@ -2565,7 +2572,8 @@ union bpf_attr {
> > >     FN(skb_ecn_set_ce),             \
> > >     FN(get_listener_sock),          \
> > >     FN(skc_lookup_tcp),             \
> > > -   FN(tcp_check_syncookie),
> > > +   FN(tcp_check_syncookie),        \
> > > +   FN(progenyof),
> > >
> > >  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> > >   * function eBPF program intends to call
> > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > > index ff09d32a8a1b..437986497468 100644
> > > --- a/kernel/bpf/core.c
> > > +++ b/kernel/bpf/core.c
> > > @@ -2044,6 +2044,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
> > >  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
> > >  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
> > >  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> > > +const struct bpf_func_proto bpf_progenyof_proto __weak;
> > >
> > >  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
> > >  {
> > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > > index a411fc17d265..f093b35d1ba8 100644
> > > --- a/kernel/bpf/helpers.c
> > > +++ b/kernel/bpf/helpers.c
> > > @@ -18,6 +18,7 @@
> > >  #include <linux/sched.h>
> > >  #include <linux/uidgid.h>
> > >  #include <linux/filter.h>
> > > +#include <linux/init_task.h>
> > >
> > >  /* If kernel subsystem is allowing eBPF programs to call this function,
> > >   * inside its own verifier_ops->get_func_proto() callback it should return
> > > @@ -364,3 +365,34 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
> > >  };
> > >  #endif
> > >  #endif
> > > +
> > > +BPF_CALL_1(bpf_progenyof, int, pid)
> >
> > Nit: could we add a more descriptive helper name? What's progenyof? Also s/int/pid_t/?
> >
>
> It's true that "progeny" is not a very commonly used word :D. A coworker
> suggested "descendantof", what do you think? It's a bit difficult to
> convey "is true on the passed pid + on all the process under the
> hierarchy chain of that pid", so I will try rewording the docs so the
> semantics are very clear!

What about childof? I see the word child used more than anything:

$ man ps pgrep pidstat pstree top | grep child | wc -l
29
$ man ps pgrep pidstat pstree top | grep progeny | wc -l
0
$ man ps pgrep pidstat pstree top | grep ancestor | wc -l
2

Brendan
-- 
Brendan Gregg, Senior Performance Architect, Netflix

[PATCH v5 bpf-next 0/3] bpf: add bpf_descendant_of helper

[Javier Honduvilla Coto]

Hi all,

This patch adds the bpf_descendant_of helper which accepts a PID and
returns 1 if the PID of the process currently being executed is a
descendant of it or if it's itself. Returns 0 otherwise.

This is very useful in tracing programs when we want to filter by a
given PID and all the children it might spawn. The current workarounds
most people implement for this purpose have issues:

- Attaching to process spawning syscalls and dynamically add those PIDs
  to some bpf map that would be used to filter is cumbersome and
potentially racy.
- Unrolling some loop to perform what this helper is doing consumes lots
  of instructions. That and the impossibility to jump backwards makes it
really hard to be correct in really large process chains.

Let me know what do you think!

Thanks,

---
Changes in V5:
        - Addressed code review feedback
        - Renamed from progenyof => descendant_of as suggested by Jon Haslam
and Brendan Gregg

Changes in V4:
        - Rebased on latest bpf-next after merge window

Changes in V3:
        - Removed RCU read (un)locking as BPF programs alredy run in RCU locked
                context
        - progenyof(0) now returns 1, which, semantically makes more sense
        - Added new test case for PID 0 and changed sentinel value for errors
        - Rebase on latest bpf-next/master
        - Used my work email as somehow I accidentally used my personal one in v2

Changes in V2:
        - Adding missing docs in include/uapi/linux/bpf.h


Javier Honduvilla Coto (3):
  bpf: add bpf_descendant_of helper
  bpf: sync kernel uapi headers
  bpf: add tests for bpf_descendant_of

 include/linux/bpf.h                           |   1 +
 include/uapi/linux/bpf.h                      |  10 +-
 kernel/bpf/core.c                             |   1 +
 kernel/bpf/helpers.c                          |  27 ++
 kernel/trace/bpf_trace.c                      |   2 +
 tools/include/uapi/linux/bpf.h                |  10 +-
 tools/testing/selftests/bpf/.gitignore        |   1 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   3 +
 .../bpf/progs/test_descendant_of_kern.c       |  46 +++
 .../selftests/bpf/test_descendant_of_user.c   | 268 ++++++++++++++++++
 11 files changed, 368 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_descendant_of_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_descendant_of_user.c

-- 
2.17.1

[PATCH v5 bpf-next 1/3] bpf: add bpf_descendant_of helper

[Javier Honduvilla Coto]

This patch adds the bpf_descendant_of helper which accepts a PID and
returns 1 if the PID of the process currently being executed is a
descendant of it or if it's itself. Returns 0 otherwise.

This is very useful in tracing programs when we want to filter by a
given PID and all the children it might spawn. The current workarounds
most people implement for this purpose have issues:

- Attaching to process spawning syscalls and dynamically add those PIDs
  to some bpf map that would be used to filter is cumbersome and
potentially racy.
- Unrolling some loop to perform what this helper is doing consumes lots
  of instructions. That and the impossibility to jump backwards makes it
really hard to be correct in really large process chains.

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 include/linux/bpf.h      |  1 +
 include/uapi/linux/bpf.h | 10 +++++++++-
 kernel/bpf/core.c        |  1 +
 kernel/bpf/helpers.c     | 27 +++++++++++++++++++++++++++
 kernel/trace/bpf_trace.c |  2 ++
 5 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 65f7094c40b4..0539999f07f3 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -967,6 +967,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
 extern const struct bpf_func_proto bpf_spin_lock_proto;
 extern const struct bpf_func_proto bpf_spin_unlock_proto;
 extern const struct bpf_func_proto bpf_get_local_storage_proto;
+extern const struct bpf_func_proto bpf_descendant_of_proto;
 
 /* Shared helpers among cBPF and eBPF. */
 void bpf_user_rnd_init_once(void);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index af1cbd951f26..f707b286c21d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2493,6 +2493,13 @@ union bpf_attr {
  * 	Return
  * 		0 if iph and th are a valid SYN cookie ACK, or a negative error
  * 		otherwise.
+ * int bpf_descendant_of(pid_t pid)
+ *	Description
+ *		This helper is useful in programs that want to filter events
+ *		happening to a pid or to any of its descendants.
+ *	Return
+ *		1 if the passed pid is an ancestor of the currently executing
+ *		process' pid or equal to it.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2595,7 +2602,8 @@ union bpf_attr {
 	FN(skb_ecn_set_ce),		\
 	FN(get_listener_sock),		\
 	FN(skc_lookup_tcp),		\
-	FN(tcp_check_syncookie),
+	FN(tcp_check_syncookie),	\
+	FN(descendant_of),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ace8c22c8b0e..df93d7157657 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2046,6 +2046,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
 const struct bpf_func_proto bpf_get_current_comm_proto __weak;
 const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
 const struct bpf_func_proto bpf_get_local_storage_proto __weak;
+const struct bpf_func_proto bpf_descendant_of_proto __weak;
 
 const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
 {
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index a411fc17d265..d04186c69042 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -18,6 +18,7 @@
 #include <linux/sched.h>
 #include <linux/uidgid.h>
 #include <linux/filter.h>
+#include <linux/init_task.h>
 
 /* If kernel subsystem is allowing eBPF programs to call this function,
  * inside its own verifier_ops->get_func_proto() callback it should return
@@ -364,3 +365,29 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
 };
 #endif
 #endif
+
+BPF_CALL_1(bpf_descendant_of, pid_t, pid)
+{
+	int result = 0;
+	struct task_struct *task = current;
+
+	if (pid == 0)
+		return 1;
+
+	while (task != &init_task) {
+		if (task->pid == pid) {
+			result = 1;
+			break;
+		}
+		task = rcu_dereference(task->real_parent);
+	}
+
+	return result;
+}
+
+const struct bpf_func_proto bpf_descendant_of_proto = {
+	.func		= bpf_descendant_of,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_ANYTHING,
+};
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d64c00afceb5..0968e38a2aae 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -599,6 +599,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_prandom_u32_proto;
 	case BPF_FUNC_probe_read_str:
 		return &bpf_probe_read_str_proto;
+	case BPF_FUNC_descendant_of:
+		return &bpf_descendant_of_proto;
 #ifdef CONFIG_CGROUPS
 	case BPF_FUNC_get_current_cgroup_id:
 		return &bpf_get_current_cgroup_id_proto;
-- 
2.17.1

[PATCH v5 bpf-next 2/3] bpf: sync kernel uapi headers

[Javier Honduvilla Coto]

Sync kernel uapi headers.

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 tools/include/uapi/linux/bpf.h | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index af1cbd951f26..f707b286c21d 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2493,6 +2493,13 @@ union bpf_attr {
  * 	Return
  * 		0 if iph and th are a valid SYN cookie ACK, or a negative error
  * 		otherwise.
+ * int bpf_descendant_of(pid_t pid)
+ *	Description
+ *		This helper is useful in programs that want to filter events
+ *		happening to a pid or to any of its descendants.
+ *	Return
+ *		1 if the passed pid is an ancestor of the currently executing
+ *		process' pid or equal to it.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2595,7 +2602,8 @@ union bpf_attr {
 	FN(skb_ecn_set_ce),		\
 	FN(get_listener_sock),		\
 	FN(skc_lookup_tcp),		\
-	FN(tcp_check_syncookie),
+	FN(tcp_check_syncookie),	\
+	FN(descendant_of),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
-- 
2.17.1

[PATCH v5 bpf-next 3/3] bpf: add tests for bpf_descendant_of

[Javier Honduvilla Coto]

Adding the following test cases:

- bpf_descendant_of(current->pid) == 1
- bpf_descendant_of(current->real_parent->pid) == 1
- bpf_descendant_of(1) == 1
- bpf_descendant_of(0) == 1

- bpf_descendant_of(-1) == 0
- bpf_descendant_of(current->children[0]->pid) == 0

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 tools/testing/selftests/bpf/.gitignore        |   1 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   3 +
 .../bpf/progs/test_descendant_of_kern.c       |  46 +++
 .../selftests/bpf/test_descendant_of_user.c   | 268 ++++++++++++++++++
 5 files changed, 319 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_descendant_of_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_descendant_of_user.c

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 41e8a689aa77..ab8adac4bcb6 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -32,3 +32,4 @@ test_tcpnotify_user
 test_libbpf
 test_tcp_check_syncookie_user
 alu32
+test_descendant_of_user
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 078283d073b0..dbe65853e8ca 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -23,7 +23,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
 	test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \
 	test_sock test_btf test_sockmap test_lirc_mode2_user get_cgroup_id_user \
 	test_socket_cookie test_cgroup_storage test_select_reuseport test_section_names \
-	test_netcnt test_tcpnotify_user test_sock_fields
+	test_netcnt test_tcpnotify_user test_sock_fields test_descendant_of_user
 
 BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
 TEST_GEN_FILES = $(BPF_OBJ_FILES)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index e85d62cb53d0..c7ec64515373 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -1,4 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+#include <sys/types.h>
+
 #ifndef __BPF_HELPERS_H
 #define __BPF_HELPERS_H
 
@@ -192,6 +194,7 @@ static int (*bpf_skb_ecn_set_ce)(void *ctx) =
 static int (*bpf_tcp_check_syncookie)(struct bpf_sock *sk,
 	    void *ip, int ip_len, void *tcp, int tcp_len) =
 	(void *) BPF_FUNC_tcp_check_syncookie;
+static int (*bpf_descendant_of)(pid_t pid) = (void *) BPF_FUNC_descendant_of;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions
diff --git a/tools/testing/selftests/bpf/progs/test_descendant_of_kern.c b/tools/testing/selftests/bpf/progs/test_descendant_of_kern.c
new file mode 100644
index 000000000000..e7d3f02355a7
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_descendant_of_kern.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct bpf_map_def SEC("maps") pidmap = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(__u32),
+	.value_size = sizeof(__u32),
+	.max_entries = 2,
+};
+
+struct bpf_map_def SEC("maps") resultmap = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(__u32),
+	.value_size = sizeof(__u32),
+	.max_entries = 1,
+};
+
+SEC("tracepoint/syscalls/sys_enter_open")
+int trace(void *ctx)
+{
+	__u32 pid = bpf_get_current_pid_tgid();
+	__u32 current_key = 0, ancestor_key = 1, *expected_pid, *ancestor_pid;
+	__u32 *val;
+
+	expected_pid = bpf_map_lookup_elem(&pidmap, &current_key);
+	if (!expected_pid || *expected_pid != pid)
+		return 0;
+
+	ancestor_pid = bpf_map_lookup_elem(&pidmap, &ancestor_key);
+	if (!ancestor_pid)
+		return 0;
+
+	if (!bpf_descendant_of(*ancestor_pid))
+		return 0;
+
+	val = bpf_map_lookup_elem(&resultmap, &current_key);
+	if (val)
+		*val = *ancestor_pid;
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
diff --git a/tools/testing/selftests/bpf/test_descendant_of_user.c b/tools/testing/selftests/bpf/test_descendant_of_user.c
new file mode 100644
index 000000000000..cfcd6f9fb99a
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_descendant_of_user.c
@@ -0,0 +1,268 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <syscall.h>
+#include <unistd.h>
+#include <linux/perf_event.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#define CHECK(condition, tag, format...)                                       \
+	({                                                                     \
+		int __ret = !!(condition);                                     \
+		if (__ret) {                                                   \
+			printf("%s:FAIL:%s ", __func__, tag);                  \
+			printf(format);                                        \
+		} else {                                                       \
+			printf("%s:PASS:%s\n", __func__, tag);                 \
+		}                                                              \
+		__ret;                                                         \
+	})
+
+static int bpf_find_map(const char *test, struct bpf_object *obj,
+			const char *name)
+{
+	struct bpf_map *map;
+
+	map = bpf_object__find_map_by_name(obj, name);
+	if (!map)
+		return -1;
+	return bpf_map__fd(map);
+}
+
+int main(int argc, char **argv)
+{
+	const char *probe_name = "syscalls/sys_enter_open";
+	const char *file = "test_descendant_of_kern.o";
+	int err, bytes, efd, prog_fd, pmu_fd;
+	int resultmap_fd, pidmap_fd;
+	struct perf_event_attr attr = {};
+	struct bpf_object *obj;
+	__u32 retrieved_ancestor_pid = 0;
+	__u32 key = 0, pid;
+	int exit_code = EXIT_FAILURE;
+	char buf[256];
+
+	int child_pid, ancestor_pid, root_fd, nonexistant = -42;
+	__u32 ancestor_key = 1;
+	int pipefd[2];
+	char marker[1];
+
+	err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
+	if (CHECK(err, "bpf_prog_load", "err %d errno %d\n", err, errno))
+		goto fail;
+
+	resultmap_fd = bpf_find_map(__func__, obj, "resultmap");
+	if (CHECK(resultmap_fd < 0, "bpf_find_map", "err %d errno %d\n",
+		  resultmap_fd, errno))
+		goto close_prog;
+
+	pidmap_fd = bpf_find_map(__func__, obj, "pidmap");
+	if (CHECK(pidmap_fd < 0, "bpf_find_map", "err %d errno %d\n", pidmap_fd,
+		  errno))
+		goto close_prog;
+
+	pid = getpid();
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+
+	snprintf(buf, sizeof(buf), "/sys/kernel/debug/tracing/events/%s/id",
+		 probe_name);
+	efd = open(buf, O_RDONLY, 0);
+	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+		goto close_prog;
+	bytes = read(efd, buf, sizeof(buf));
+	close(efd);
+	if (CHECK(bytes <= 0 || bytes >= sizeof(buf), "read",
+		  "bytes %d errno %d\n", bytes, errno))
+		goto close_prog;
+
+	attr.config = strtol(buf, NULL, 0);
+	attr.type = PERF_TYPE_TRACEPOINT;
+	attr.sample_type = PERF_SAMPLE_RAW;
+	attr.sample_period = 1;
+	attr.wakeup_events = 1;
+
+	pmu_fd = syscall(__NR_perf_event_open, &attr, getpid(), -1, -1, 0);
+	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n", pmu_fd,
+		  errno))
+		goto close_prog;
+
+	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
+	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n", err,
+		  errno))
+		goto close_pmu;
+
+	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
+	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n", err,
+		  errno))
+		goto close_pmu;
+
+	// Test on ourselve: descendant_of(current->pid) is true
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != pid,
+		  "descendant_of is true with same pid", "%d == %d\n",
+		  retrieved_ancestor_pid, pid))
+		goto close_pmu;
+
+	// Test that PID 1 an ancestor
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = 1;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != ancestor_pid,
+		  "descendant_of reaches init", "%d == %d\n",
+		  retrieved_ancestor_pid, ancestor_pid))
+		goto close_pmu;
+
+	// Test that PID 0 is an ancestor
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = 0;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != ancestor_pid,
+		  "descendant_of does not go over init", "%d == %d\n",
+		  retrieved_ancestor_pid, ancestor_pid))
+		goto close_pmu;
+
+	// Test that we don't go over PID 0
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = -1;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &retrieved_ancestor_pid);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(retrieved_ancestor_pid != nonexistant,
+		  "descendant_of does not go over init", "%d == %d\n",
+		  retrieved_ancestor_pid, nonexistant))
+		goto close_pmu;
+
+	// Test that we are an ancestor of our child
+	pipe(pipefd);
+	child_pid = fork();
+	if (child_pid == -1) {
+		printf("fork failed\n");
+		goto close_pmu;
+	} else if (child_pid == 0) {
+		close(pipefd[1]);
+		read(pipefd[0], &marker, 1);
+
+		root_fd = open("/", O_RDONLY);
+		if (CHECK(efd < 0, "open", "errno %d\n", errno))
+			goto close_prog;
+		close(root_fd);
+
+		close(pipefd[0]);
+		_exit(EXIT_SUCCESS);
+	} else {
+		close(pipefd[0]);
+		bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+		bpf_map_update_elem(pidmap_fd, &key, &child_pid, 0);
+		bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+
+		write(pipefd[1], &marker, 1);
+		wait(NULL);
+		close(pipefd[1]);
+
+		err = bpf_map_lookup_elem(resultmap_fd, &key,
+					  &retrieved_ancestor_pid);
+		if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err,
+			  errno))
+			goto close_pmu;
+		if (CHECK(retrieved_ancestor_pid != pid, "descendant_of of parent",
+			  "%d == %d\n", retrieved_ancestor_pid, pid))
+			goto close_pmu;
+	}
+
+	// Test that a child of ours doesn't belong to our ancestors
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	pipe(pipefd);
+	child_pid = fork();
+	if (child_pid == -1) {
+		printf("fork failed\n");
+		goto close_pmu;
+	} else if (child_pid == 0) {
+		close(pipefd[1]);
+		read(pipefd[0], marker, 1);
+		close(pipefd[0]);
+		_exit(EXIT_SUCCESS);
+	} else {
+		close(pipefd[0]);
+
+		bpf_map_update_elem(pidmap_fd, &ancestor_key, &child_pid, 0);
+
+		root_fd = open("/", O_RDONLY);
+		if (CHECK(efd < 0, "open", "errno %d\n", errno))
+			goto close_prog;
+		close(root_fd);
+
+		write(pipefd[1], marker, 1);
+		wait(NULL);
+		close(pipefd[1]);
+
+		err = bpf_map_lookup_elem(resultmap_fd, &key,
+					  &retrieved_ancestor_pid);
+		if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err,
+			  errno))
+			goto close_pmu;
+		if (CHECK(retrieved_ancestor_pid != nonexistant, "descendant_of of child",
+			  "%d == %d\n", retrieved_ancestor_pid, 0))
+			goto close_pmu;
+	}
+
+	exit_code = EXIT_SUCCESS;
+	printf("%s:PASS\n", argv[0]);
+
+close_pmu:
+	close(pmu_fd);
+close_prog:
+	bpf_object__close(obj);
+fail:
+	return exit_code;
+}
-- 
2.17.1

Re: [PATCH v5 bpf-next 0/3] bpf: add bpf_descendant_of helper

[Song Liu]

On Wed, Apr 10, 2019 at 1:38 PM Javier Honduvilla Coto
<javierhonduco@fb.com> wrote:
>
> Hi all,
>
> This patch adds the bpf_descendant_of helper which accepts a PID and
> returns 1 if the PID of the process currently being executed is a
> descendant of it or if it's itself. Returns 0 otherwise.
>
> This is very useful in tracing programs when we want to filter by a
> given PID and all the children it might spawn. The current workarounds
> most people implement for this purpose have issues:
>
> - Attaching to process spawning syscalls and dynamically add those PIDs
>   to some bpf map that would be used to filter is cumbersome and
> potentially racy.
> - Unrolling some loop to perform what this helper is doing consumes lots
>   of instructions. That and the impossibility to jump backwards makes it
> really hard to be correct in really large process chains.
>
> Let me know what do you think!
>
> Thanks,

For the set:

Acked-by: Song Liu <songliubraving@fb.com>

>
> ---
> Changes in V5:
>         - Addressed code review feedback
>         - Renamed from progenyof => descendant_of as suggested by Jon Haslam
> and Brendan Gregg
>
> Changes in V4:
>         - Rebased on latest bpf-next after merge window
>
> Changes in V3:
>         - Removed RCU read (un)locking as BPF programs alredy run in RCU locked
>                 context
>         - progenyof(0) now returns 1, which, semantically makes more sense
>         - Added new test case for PID 0 and changed sentinel value for errors
>         - Rebase on latest bpf-next/master
>         - Used my work email as somehow I accidentally used my personal one in v2
>
> Changes in V2:
>         - Adding missing docs in include/uapi/linux/bpf.h
>
>
> Javier Honduvilla Coto (3):
>   bpf: add bpf_descendant_of helper
>   bpf: sync kernel uapi headers
>   bpf: add tests for bpf_descendant_of
>
>  include/linux/bpf.h                           |   1 +
>  include/uapi/linux/bpf.h                      |  10 +-
>  kernel/bpf/core.c                             |   1 +
>  kernel/bpf/helpers.c                          |  27 ++
>  kernel/trace/bpf_trace.c                      |   2 +
>  tools/include/uapi/linux/bpf.h                |  10 +-
>  tools/testing/selftests/bpf/.gitignore        |   1 +
>  tools/testing/selftests/bpf/Makefile          |   2 +-
>  tools/testing/selftests/bpf/bpf_helpers.h     |   3 +
>  .../bpf/progs/test_descendant_of_kern.c       |  46 +++
>  .../selftests/bpf/test_descendant_of_user.c   | 268 ++++++++++++++++++
>  11 files changed, 368 insertions(+), 3 deletions(-)
>  create mode 100644 tools/testing/selftests/bpf/progs/test_descendant_of_kern.c
>  create mode 100644 tools/testing/selftests/bpf/test_descendant_of_user.c
>
> --
> 2.17.1
>

Re: [PATCH v5 bpf-next 1/3] bpf: add bpf_descendant_of helper

[Daniel Borkmann]

On 04/10/2019 10:36 PM, Javier Honduvilla Coto wrote:
> This patch adds the bpf_descendant_of helper which accepts a PID and
> returns 1 if the PID of the process currently being executed is a
> descendant of it or if it's itself. Returns 0 otherwise.
> 
> This is very useful in tracing programs when we want to filter by a
> given PID and all the children it might spawn. The current workarounds
> most people implement for this purpose have issues:
> 
> - Attaching to process spawning syscalls and dynamically add those PIDs
>   to some bpf map that would be used to filter is cumbersome and
> potentially racy.
> - Unrolling some loop to perform what this helper is doing consumes lots
>   of instructions. That and the impossibility to jump backwards makes it
> really hard to be correct in really large process chains.
> 
> Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> ---
>  include/linux/bpf.h      |  1 +
>  include/uapi/linux/bpf.h | 10 +++++++++-
>  kernel/bpf/core.c        |  1 +
>  kernel/bpf/helpers.c     | 27 +++++++++++++++++++++++++++
>  kernel/trace/bpf_trace.c |  2 ++
>  5 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 65f7094c40b4..0539999f07f3 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -967,6 +967,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
>  extern const struct bpf_func_proto bpf_spin_lock_proto;
>  extern const struct bpf_func_proto bpf_spin_unlock_proto;
>  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> +extern const struct bpf_func_proto bpf_descendant_of_proto;
>  
>  /* Shared helpers among cBPF and eBPF. */
>  void bpf_user_rnd_init_once(void);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index af1cbd951f26..f707b286c21d 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2493,6 +2493,13 @@ union bpf_attr {
>   * 	Return
>   * 		0 if iph and th are a valid SYN cookie ACK, or a negative error
>   * 		otherwise.
> + * int bpf_descendant_of(pid_t pid)

Small nit: Looks good to go, but please add a newline before the new helper
description like all the rest in there.

> + *	Description
> + *		This helper is useful in programs that want to filter events
> + *		happening to a pid or to any of its descendants.

One more thing that would be helpful is to add a short description here that
this helper can be used in combination with bpf_get_current_pid_tgid(), and
that pid here is representation from init pid namespace if I grok it correctly.

> + *	Return
> + *		1 if the passed pid is an ancestor of the currently executing
> + *		process' pid or equal to it.
>   */
>  #define __BPF_FUNC_MAPPER(FN)		\
>  	FN(unspec),			\
> @@ -2595,7 +2602,8 @@ union bpf_attr {
>  	FN(skb_ecn_set_ce),		\
>  	FN(get_listener_sock),		\
>  	FN(skc_lookup_tcp),		\
> -	FN(tcp_check_syncookie),
> +	FN(tcp_check_syncookie),	\
> +	FN(descendant_of),
>  
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index ace8c22c8b0e..df93d7157657 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2046,6 +2046,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
>  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
>  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
>  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> +const struct bpf_func_proto bpf_descendant_of_proto __weak;
>  
>  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
>  {
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index a411fc17d265..d04186c69042 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -18,6 +18,7 @@
>  #include <linux/sched.h>
>  #include <linux/uidgid.h>
>  #include <linux/filter.h>
> +#include <linux/init_task.h>
>  
>  /* If kernel subsystem is allowing eBPF programs to call this function,
>   * inside its own verifier_ops->get_func_proto() callback it should return
> @@ -364,3 +365,29 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
>  };
>  #endif
>  #endif
> +
> +BPF_CALL_1(bpf_descendant_of, pid_t, pid)
> +{
> +	int result = 0;
> +	struct task_struct *task = current;
> +
> +	if (pid == 0)
> +		return 1;
> +
> +	while (task != &init_task) {
> +		if (task->pid == pid) {
> +			result = 1;
> +			break;
> +		}
> +		task = rcu_dereference(task->real_parent);
> +	}
> +
> +	return result;
> +}
> +
> +const struct bpf_func_proto bpf_descendant_of_proto = {
> +	.func		= bpf_descendant_of,
> +	.gpl_only	= false,
> +	.ret_type	= RET_INTEGER,
> +	.arg1_type	= ARG_ANYTHING,
> +};
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index d64c00afceb5..0968e38a2aae 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -599,6 +599,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		return &bpf_get_prandom_u32_proto;
>  	case BPF_FUNC_probe_read_str:
>  		return &bpf_probe_read_str_proto;
> +	case BPF_FUNC_descendant_of:
> +		return &bpf_descendant_of_proto;
>  #ifdef CONFIG_CGROUPS
>  	case BPF_FUNC_get_current_cgroup_id:
>  		return &bpf_get_current_cgroup_id_proto;
> 

Re: [PATCH v5 bpf-next 1/3] bpf: add bpf_descendant_of helper

[Javier Honduvilla Coto]

On Thu, Apr 11, 2019 at 11:55:58PM +0200, Daniel Borkmann wrote:
> On 04/10/2019 10:36 PM, Javier Honduvilla Coto wrote:
> > This patch adds the bpf_descendant_of helper which accepts a PID and
> > returns 1 if the PID of the process currently being executed is a
> > descendant of it or if it's itself. Returns 0 otherwise.
> >
> > This is very useful in tracing programs when we want to filter by a
> > given PID and all the children it might spawn. The current workarounds
> > most people implement for this purpose have issues:
> >
> > - Attaching to process spawning syscalls and dynamically add those PIDs
> >   to some bpf map that would be used to filter is cumbersome and
> > potentially racy.
> > - Unrolling some loop to perform what this helper is doing consumes lots
> >   of instructions. That and the impossibility to jump backwards makes it
> > really hard to be correct in really large process chains.
> >
> > Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> > ---
> >  include/linux/bpf.h      |  1 +
> >  include/uapi/linux/bpf.h | 10 +++++++++-
> >  kernel/bpf/core.c        |  1 +
> >  kernel/bpf/helpers.c     | 27 +++++++++++++++++++++++++++
> >  kernel/trace/bpf_trace.c |  2 ++
> >  5 files changed, 40 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index 65f7094c40b4..0539999f07f3 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -967,6 +967,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
> >  extern const struct bpf_func_proto bpf_spin_lock_proto;
> >  extern const struct bpf_func_proto bpf_spin_unlock_proto;
> >  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> > +extern const struct bpf_func_proto bpf_descendant_of_proto;
> >
> >  /* Shared helpers among cBPF and eBPF. */
> >  void bpf_user_rnd_init_once(void);
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index af1cbd951f26..f707b286c21d 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -2493,6 +2493,13 @@ union bpf_attr {
> >   * 	Return
> >   * 		0 if iph and th are a valid SYN cookie ACK, or a negative error
> >   * 		otherwise.
> > + * int bpf_descendant_of(pid_t pid)
>
> Small nit: Looks good to go, but please add a newline before the new helper
> description like all the rest in there.

Thanks!

>
> > + *	Description
> > + *		This helper is useful in programs that want to filter events
> > + *		happening to a pid or to any of its descendants.
>
> One more thing that would be helpful is to add a short description here that
> this helper can be used in combination with bpf_get_current_pid_tgid(), and
> that pid here is representation from init pid namespace if I grok it correctly.

What use case do you have in mind for bpf_get_current_pid_tgid() +
bpf_descendant_of()? Most of the cases the former won't be necessary as
the latter is alredy fetching the pid of the process in the current
context, but maybe I'm missing something! :)

Not sure about the last part, sorry, are you referring that we should
maybe mention that the descendant check is performed within a pid namespace
and does not cross pid namespaces?

>
> > + *	Return
> > + *		1 if the passed pid is an ancestor of the currently executing
> > + *		process' pid or equal to it.
> >   */
> >  #define __BPF_FUNC_MAPPER(FN)		\
> >  	FN(unspec),			\
> > @@ -2595,7 +2602,8 @@ union bpf_attr {
> >  	FN(skb_ecn_set_ce),		\
> >  	FN(get_listener_sock),		\
> >  	FN(skc_lookup_tcp),		\
> > -	FN(tcp_check_syncookie),
> > +	FN(tcp_check_syncookie),	\
> > +	FN(descendant_of),
> >
> >  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> >   * function eBPF program intends to call
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index ace8c22c8b0e..df93d7157657 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -2046,6 +2046,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
> >  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
> >  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
> >  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> > +const struct bpf_func_proto bpf_descendant_of_proto __weak;
> >
> >  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
> >  {
> > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> > index a411fc17d265..d04186c69042 100644
> > --- a/kernel/bpf/helpers.c
> > +++ b/kernel/bpf/helpers.c
> > @@ -18,6 +18,7 @@
> >  #include <linux/sched.h>
> >  #include <linux/uidgid.h>
> >  #include <linux/filter.h>
> > +#include <linux/init_task.h>
> >
> >  /* If kernel subsystem is allowing eBPF programs to call this function,
> >   * inside its own verifier_ops->get_func_proto() callback it should return
> > @@ -364,3 +365,29 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
> >  };
> >  #endif
> >  #endif
> > +
> > +BPF_CALL_1(bpf_descendant_of, pid_t, pid)
> > +{
> > +	int result = 0;
> > +	struct task_struct *task = current;
> > +
> > +	if (pid == 0)
> > +		return 1;
> > +
> > +	while (task != &init_task) {
> > +		if (task->pid == pid) {
> > +			result = 1;
> > +			break;
> > +		}
> > +		task = rcu_dereference(task->real_parent);
> > +	}
> > +
> > +	return result;
> > +}
> > +
> > +const struct bpf_func_proto bpf_descendant_of_proto = {
> > +	.func		= bpf_descendant_of,
> > +	.gpl_only	= false,
> > +	.ret_type	= RET_INTEGER,
> > +	.arg1_type	= ARG_ANYTHING,
> > +};
> > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > index d64c00afceb5..0968e38a2aae 100644
> > --- a/kernel/trace/bpf_trace.c
> > +++ b/kernel/trace/bpf_trace.c
> > @@ -599,6 +599,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> >  		return &bpf_get_prandom_u32_proto;
> >  	case BPF_FUNC_probe_read_str:
> >  		return &bpf_probe_read_str_proto;
> > +	case BPF_FUNC_descendant_of:
> > +		return &bpf_descendant_of_proto;
> >  #ifdef CONFIG_CGROUPS
> >  	case BPF_FUNC_get_current_cgroup_id:
> >  		return &bpf_get_current_cgroup_id_proto;
> >
>

[PATCH v6 bpf-next 0/3] bpf: add bpf_descendant_of helper

[Javier Honduvilla Coto]

Hi all,

This patch adds the bpf_descendant_of helper which accepts a PID and
returns 1 if the PID of the process currently being executed is a
descendant of it or if it's itself. Returns 0 otherwise. The passed
PID should be the one as seen from the "global" pid namespace as the
processes' PIDs in the hierarchy are resolved using the context of said
initial namespace.

This is very useful in tracing programs when we want to filter by a
given PID and all the children it might spawn. The current workarounds
most people implement for this purpose have issues:

- Attaching to process spawning syscalls and dynamically add those PIDs
to some bpf map that would be used to filter is cumbersome and
potentially racy.
- Unrolling some loop to perform what this helper is doing consumes lots
of instructions. That and the impossibility to jump backwards makes it
really hard to be correct in really large process chains.


Let me know what do you think!

Thanks,

---
Changes in V6:
        - Small style fix
        - Clarify in the docs that we are resolving PIDs using the global,
initial PID namespace, and the provided *pid* argument should be global, too
        - Changed the way we assert on the helper return value

Changes in V5:
        - Addressed code review feedback
        - Renamed from progenyof => descendant_of as suggested by Jon Haslam
and Brendan Gregg

Changes in V4:
        - Rebased on latest bpf-next after merge window

Changes in V3:
        - Removed RCU read (un)locking as BPF programs alredy run in RCU locked
                context
        - progenyof(0) now returns 1, which, semantically makes more sense
        - Added new test case for PID 0 and changed sentinel value for errors
        - Rebase on latest bpf-next/master
        - Used my work email as somehow I accidentally used my personal one in v2

Changes in V2:
        - Adding missing docs in include/uapi/linux/bpf.h
-- 
2.17.1

[PATCH v6 bpf-next 1/3] bpf: add bpf_descendant_of helper

[Javier Honduvilla Coto]

This patch adds the bpf_descendant_of helper which accepts a PID and
returns 1 if the PID of the process currently being executed is a
descendant of it or if it's itself. Returns 0 otherwise. The passed
PID should be the one as seen from the "global" pid namespace as the
processes' PIDs in the hierarchy are resolved using the context of said
initial namespace.

This is very useful in tracing programs when we want to filter by a
given PID and all the children it might spawn. The current workarounds
most people implement for this purpose have issues:

- Attaching to process spawning syscalls and dynamically add those PIDs
  to some bpf map that would be used to filter is cumbersome and
potentially racy.
- Unrolling some loop to perform what this helper is doing consumes lots
  of instructions. That and the impossibility to jump backwards makes it
really hard to be correct in really large process chains.

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 include/linux/bpf.h      |  1 +
 include/uapi/linux/bpf.h | 20 +++++++++++++++++++-
 kernel/bpf/core.c        |  1 +
 kernel/bpf/helpers.c     | 27 +++++++++++++++++++++++++++
 kernel/trace/bpf_trace.c |  2 ++
 5 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 18f4cc2c6acd..4e861138887d 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1053,6 +1053,7 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto;
 extern const struct bpf_func_proto bpf_strtol_proto;
 extern const struct bpf_func_proto bpf_strtoul_proto;
 extern const struct bpf_func_proto bpf_tcp_sock_proto;
+extern const struct bpf_func_proto bpf_descendant_of_proto;
 
 /* Shared helpers among cBPF and eBPF. */
 void bpf_user_rnd_init_once(void);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 5695ab53e354..7e8c2bd654f5 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -2713,6 +2713,23 @@ union bpf_attr {
  *		**-EPERM** if no permission to send the *sig*.
  *
  *		**-EAGAIN** if bpf program can try again.
+ *
+ * int bpf_descendant_of(pid_t pid)
+ *     Description
+ *             Determine if the process identified by *pid* is an ancestor
+ *             (or equal) of the user process executed in this tracing
+ *             context. This is useful when filtering events happening
+ *             to a process and all of its descendants.
+ *
+ *             Note that *pid* must be the pid from the global namespace
+ *             as the pids of the process chain will be resolved using the
+ *             initial pid namespace viewer context.
+ *     Return
+ *             * 1 if the process identified by *pid* is an ancestor, or equal,
+ *             of the currently executing process within the global pid
+ *             namespace
+ *
+ *             * 0 otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2824,7 +2841,8 @@ union bpf_attr {
 	FN(strtoul),			\
 	FN(sk_storage_get),		\
 	FN(sk_storage_delete),		\
-	FN(send_signal),
+	FN(send_signal),		\
+	FN(descendant_of),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 16079550db6d..8f7f0ec8cded 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2039,6 +2039,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
 const struct bpf_func_proto bpf_get_current_comm_proto __weak;
 const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
 const struct bpf_func_proto bpf_get_local_storage_proto __weak;
+const struct bpf_func_proto bpf_descendant_of_proto __weak;
 
 const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
 {
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 5e28718928ca..2214194e5f49 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -11,6 +11,7 @@
 #include <linux/uidgid.h>
 #include <linux/filter.h>
 #include <linux/ctype.h>
+#include <linux/init_task.h>
 
 #include "../../lib/kstrtox.h"
 
@@ -487,3 +488,29 @@ const struct bpf_func_proto bpf_strtoul_proto = {
 	.arg4_type	= ARG_PTR_TO_LONG,
 };
 #endif
+
+BPF_CALL_1(bpf_descendant_of, pid_t, pid)
+{
+	int result = 0;
+	struct task_struct *task = current;
+
+	if (pid == 0)
+		return 1;
+
+	while (task != &init_task) {
+		if (task->pid == pid) {
+			result = 1;
+			break;
+		}
+		task = rcu_dereference(task->real_parent);
+	}
+
+	return result;
+}
+
+const struct bpf_func_proto bpf_descendant_of_proto = {
+	.func		= bpf_descendant_of,
+	.gpl_only	= false,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_ANYTHING,
+};
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index ca1255d14576..797d7b4a8e9a 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -703,6 +703,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_prandom_u32_proto;
 	case BPF_FUNC_probe_read_str:
 		return &bpf_probe_read_str_proto;
+	case BPF_FUNC_descendant_of:
+		return &bpf_descendant_of_proto;
 #ifdef CONFIG_CGROUPS
 	case BPF_FUNC_get_current_cgroup_id:
 		return &bpf_get_current_cgroup_id_proto;
-- 
2.17.1

[PATCH v6 bpf-next 2/3] bpf: sync kernel uapi headers

[Javier Honduvilla Coto]

Sync kernel uapi headers.

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 tools/include/uapi/linux/bpf.h | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 402208581b2d..505ee91898c2 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -2710,6 +2710,23 @@ union bpf_attr {
  *		**-EPERM** if no permission to send the *sig*.
  *
  *		**-EAGAIN** if bpf program can try again.
+ *
+ * int bpf_descendant_of(pid_t pid)
+ *     Description
+ *             Determine if the process identified by *pid* is an ancestor
+ *             (or equal) of the user process executed in this tracing
+ *             context. This is useful when filtering events happening
+ *             to a process and all of its descendants.
+ *
+ *             Note that *pid* must be the pid from the global namespace
+ *             as the pids of the process chain will be resolved using the
+ *             initial pid namespace viewer context.
+ *     Return
+ *             * 1 if the process identified by *pid* is an ancestor, or equal,
+ *             of the currently executing process within the global pid
+ *             namespace
+ *
+ *             * 0 otherwise.
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -2821,7 +2838,8 @@ union bpf_attr {
 	FN(strtoul),			\
 	FN(sk_storage_get),		\
 	FN(sk_storage_delete),		\
-	FN(send_signal),
+	FN(send_signal),		\
+	FN(descendant_of),
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
-- 
2.17.1

[PATCH v6 bpf-next 3/3] bpf: add tests for bpf_descendant_of

[Javier Honduvilla Coto]

Adding the following test cases:

- bpf_descendant_of(current->pid) == 1
- bpf_descendant_of(current->real_parent->pid) == 1
- bpf_descendant_of(1) == 1
- bpf_descendant_of(0) == 1

- bpf_descendant_of(-1) == 0
- bpf_descendant_of(current->children[0]->pid) == 0

Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
---
 tools/testing/selftests/bpf/.gitignore        |   1 +
 tools/testing/selftests/bpf/Makefile          |   2 +-
 tools/testing/selftests/bpf/bpf_helpers.h     |   3 +
 .../bpf/progs/test_descendant_of_kern.c       |  43 +++
 .../selftests/bpf/test_descendant_of_user.c   | 266 ++++++++++++++++++
 5 files changed, 314 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_descendant_of_kern.c
 create mode 100644 tools/testing/selftests/bpf/test_descendant_of_user.c

diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
index 90f70d2c7c22..4b63d7105ba2 100644
--- a/tools/testing/selftests/bpf/.gitignore
+++ b/tools/testing/selftests/bpf/.gitignore
@@ -43,3 +43,4 @@ test_sockopt
 test_sockopt_sk
 test_sockopt_multi
 test_tcp_rtt
+test_descendant_of_user
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 2620406a53ec..b3dc1e26c41c 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -27,7 +27,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
 	test_cgroup_storage test_select_reuseport test_section_names \
 	test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
 	test_btf_dump test_cgroup_attach xdping test_sockopt test_sockopt_sk \
-	test_sockopt_multi test_tcp_rtt
+	test_sockopt_multi test_tcp_rtt test_descendant_of_user
 
 BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
 TEST_GEN_FILES = $(BPF_OBJ_FILES)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 5a3d92c8bec8..7525783ffbc9 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -1,4 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0 */
+#include <sys/types.h>
+
 #ifndef __BPF_HELPERS_H
 #define __BPF_HELPERS_H
 
@@ -228,6 +230,7 @@ static void *(*bpf_sk_storage_get)(void *map, struct bpf_sock *sk,
 static int (*bpf_sk_storage_delete)(void *map, struct bpf_sock *sk) =
 	(void *)BPF_FUNC_sk_storage_delete;
 static int (*bpf_send_signal)(unsigned sig) = (void *)BPF_FUNC_send_signal;
+static int (*bpf_descendant_of)(pid_t pid) = (void *) BPF_FUNC_descendant_of;
 
 /* llvm builtin functions that eBPF C program may use to
  * emit BPF_LD_ABS and BPF_LD_IND instructions
diff --git a/tools/testing/selftests/bpf/progs/test_descendant_of_kern.c b/tools/testing/selftests/bpf/progs/test_descendant_of_kern.c
new file mode 100644
index 000000000000..802e01595527
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_descendant_of_kern.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct bpf_map_def SEC("maps") pidmap = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(__u32),
+	.value_size = sizeof(__u32),
+	.max_entries = 2,
+};
+
+struct bpf_map_def SEC("maps") resultmap = {
+	.type = BPF_MAP_TYPE_ARRAY,
+	.key_size = sizeof(__u32),
+	.value_size = sizeof(__u32),
+	.max_entries = 1,
+};
+
+SEC("tracepoint/syscalls/sys_enter_open")
+int trace(void *ctx)
+{
+	__u32 pid = bpf_get_current_pid_tgid();
+	__u32 current_key = 0, ancestor_key = 1, *expected_pid, *ancestor_pid;
+	__u32 *val;
+
+	expected_pid = bpf_map_lookup_elem(&pidmap, &current_key);
+	if (!expected_pid || *expected_pid != pid)
+		return 0;
+
+	ancestor_pid = bpf_map_lookup_elem(&pidmap, &ancestor_key);
+	if (!ancestor_pid)
+		return 0;
+
+	val = bpf_map_lookup_elem(&resultmap, &current_key);
+	if (val)
+		*val = bpf_descendant_of(*ancestor_pid);
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1;
diff --git a/tools/testing/selftests/bpf/test_descendant_of_user.c b/tools/testing/selftests/bpf/test_descendant_of_user.c
new file mode 100644
index 000000000000..f616c8c976a4
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_descendant_of_user.c
@@ -0,0 +1,266 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <syscall.h>
+#include <unistd.h>
+#include <linux/perf_event.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+
+#include <bpf/bpf.h>
+#include <bpf/libbpf.h>
+
+#define CHECK(condition, tag, format...)                                       \
+	({                                                                     \
+		int __ret = !!(condition);                                     \
+		if (__ret) {                                                   \
+			printf("%s:FAIL:%s ", __func__, tag);                  \
+			printf(format);                                        \
+		} else {                                                       \
+			printf("%s:PASS:%s\n", __func__, tag);                 \
+		}                                                              \
+		__ret;                                                         \
+	})
+
+static int bpf_find_map(const char *test, struct bpf_object *obj,
+			const char *name)
+{
+	struct bpf_map *map;
+
+	map = bpf_object__find_map_by_name(obj, name);
+	if (!map)
+		return -1;
+	return bpf_map__fd(map);
+}
+
+int main(int argc, char **argv)
+{
+	const char *probe_name = "syscalls/sys_enter_open";
+	const char *file = "test_descendant_of_kern.o";
+	int err, bytes, efd, prog_fd, pmu_fd;
+	int resultmap_fd, pidmap_fd;
+	struct perf_event_attr attr = {};
+	struct bpf_object *obj;
+	__u32 descendant_of_result = 0;
+	__u32 key = 0, pid;
+	int exit_code = EXIT_FAILURE;
+	char buf[256];
+
+	int child_pid, ancestor_pid, root_fd, nonexistant = -42;
+	__u32 ancestor_key = 1;
+	int pipefd[2];
+	char marker[1];
+
+	err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
+	if (CHECK(err, "bpf_prog_load", "err %d errno %d\n", err, errno))
+		goto fail;
+
+	resultmap_fd = bpf_find_map(__func__, obj, "resultmap");
+	if (CHECK(resultmap_fd < 0, "bpf_find_map", "err %d errno %d\n",
+		  resultmap_fd, errno))
+		goto close_prog;
+
+	pidmap_fd = bpf_find_map(__func__, obj, "pidmap");
+	if (CHECK(pidmap_fd < 0, "bpf_find_map", "err %d errno %d\n", pidmap_fd,
+		  errno))
+		goto close_prog;
+
+	pid = getpid();
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+
+	snprintf(buf, sizeof(buf), "/sys/kernel/debug/tracing/events/%s/id",
+		 probe_name);
+	efd = open(buf, O_RDONLY, 0);
+	if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
+		goto close_prog;
+	bytes = read(efd, buf, sizeof(buf));
+	close(efd);
+	if (CHECK(bytes <= 0 || bytes >= sizeof(buf), "read",
+		  "bytes %d errno %d\n", bytes, errno))
+		goto close_prog;
+
+	attr.config = strtol(buf, NULL, 0);
+	attr.type = PERF_TYPE_TRACEPOINT;
+	attr.sample_type = PERF_SAMPLE_RAW;
+	attr.sample_period = 1;
+	attr.wakeup_events = 1;
+
+	pmu_fd = syscall(__NR_perf_event_open, &attr, getpid(), -1, -1, 0);
+	if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n", pmu_fd,
+		  errno))
+		goto close_prog;
+
+	err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
+	if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n", err,
+		  errno))
+		goto close_pmu;
+
+	err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
+	if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n", err,
+		  errno))
+		goto close_pmu;
+
+	// Test that descendant_of(current->pid) is true
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &descendant_of_result);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(descendant_of_result != 1,
+		  "descendant_of is true with same pid", "%d == %d\n",
+		  descendant_of_result, 1))
+		goto close_pmu;
+
+	// Test that PID 1 an ancestor
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = 1;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &descendant_of_result);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(descendant_of_result != 1, "descendant_of reaches init",
+		  "%d == %d\n", descendant_of_result, 1))
+		goto close_pmu;
+
+	// Test that PID 0 is an ancestor
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = 0;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &descendant_of_result);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(descendant_of_result != 1, "PID 0 is our ancestor",
+		  "%d == %d\n", descendant_of_result, 1))
+		goto close_pmu;
+
+	// Test that we don't go over PID 0
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	ancestor_pid = -1;
+	bpf_map_update_elem(pidmap_fd, &ancestor_key, &ancestor_pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	root_fd = open("/", O_RDONLY);
+	if (CHECK(efd < 0, "open", "errno %d\n", errno))
+		goto close_prog;
+	close(root_fd);
+
+	err = bpf_map_lookup_elem(resultmap_fd, &key, &descendant_of_result);
+	if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
+		goto close_pmu;
+	if (CHECK(descendant_of_result != 0,
+		  "descendant_of does not go over PID 0", "%d == %d\n",
+		  descendant_of_result, 0))
+		goto close_pmu;
+
+	// Test that we are an ancestor of our child
+	pipe(pipefd);
+	child_pid = fork();
+	if (child_pid == -1) {
+		printf("fork failed\n");
+		goto close_pmu;
+	} else if (child_pid == 0) {
+		close(pipefd[1]);
+		read(pipefd[0], &marker, 1);
+
+		root_fd = open("/", O_RDONLY);
+		if (CHECK(efd < 0, "open", "errno %d\n", errno))
+			goto close_prog;
+		close(root_fd);
+
+		close(pipefd[0]);
+		_exit(EXIT_SUCCESS);
+	} else {
+		close(pipefd[0]);
+		bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+		bpf_map_update_elem(pidmap_fd, &key, &child_pid, 0);
+		bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
+
+		write(pipefd[1], &marker, 1);
+		wait(NULL);
+		close(pipefd[1]);
+
+		err = bpf_map_lookup_elem(resultmap_fd, &key,
+					  &descendant_of_result);
+		if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err,
+			  errno))
+			goto close_pmu;
+		if (CHECK(descendant_of_result != 1, "descendant_of of parent",
+			  "%d == %d\n", descendant_of_result, 1))
+			goto close_pmu;
+	}
+
+	// Test that a child of ours doesn't belong to our ancestors
+	bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
+	bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
+
+	pipe(pipefd);
+	child_pid = fork();
+	if (child_pid == -1) {
+		printf("fork failed\n");
+		goto close_pmu;
+	} else if (child_pid == 0) {
+		close(pipefd[1]);
+		read(pipefd[0], marker, 1);
+		close(pipefd[0]);
+		_exit(EXIT_SUCCESS);
+	} else {
+		close(pipefd[0]);
+
+		bpf_map_update_elem(pidmap_fd, &ancestor_key, &child_pid, 0);
+
+		root_fd = open("/", O_RDONLY);
+		if (CHECK(efd < 0, "open", "errno %d\n", errno))
+			goto close_prog;
+		close(root_fd);
+
+		write(pipefd[1], marker, 1);
+		wait(NULL);
+		close(pipefd[1]);
+
+		err = bpf_map_lookup_elem(resultmap_fd, &key,
+					  &descendant_of_result);
+		if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err,
+			  errno))
+			goto close_pmu;
+		if (CHECK(descendant_of_result != 0, "descendant_of of child",
+			  "%d == %d\n", descendant_of_result, 0))
+			goto close_pmu;
+	}
+
+	exit_code = EXIT_SUCCESS;
+	printf("%s:PASS\n", argv[0]);
+
+close_pmu:
+	close(pmu_fd);
+close_prog:
+	bpf_object__close(obj);
+fail:
+	return exit_code;
+}
-- 
2.17.1

Re: [PATCH v6 bpf-next 3/3] bpf: add tests for bpf_descendant_of

[Andrii Nakryiko]

On Wed, Jul 10, 2019 at 11:31 AM Javier Honduvilla Coto
<javierhonduco@fb.com> wrote:
>
> Adding the following test cases:

FYI, bpf-next is closed, so this won't be able to go in for about 2 weeks.

>
> - bpf_descendant_of(current->pid) == 1
> - bpf_descendant_of(current->real_parent->pid) == 1
> - bpf_descendant_of(1) == 1
> - bpf_descendant_of(0) == 1
>
> - bpf_descendant_of(-1) == 0
> - bpf_descendant_of(current->children[0]->pid) == 0
>
> Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> ---
>  tools/testing/selftests/bpf/.gitignore        |   1 +
>  tools/testing/selftests/bpf/Makefile          |   2 +-
>  tools/testing/selftests/bpf/bpf_helpers.h     |   3 +
>  .../bpf/progs/test_descendant_of_kern.c       |  43 +++
>  .../selftests/bpf/test_descendant_of_user.c   | 266 ++++++++++++++++++
>  5 files changed, 314 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/bpf/progs/test_descendant_of_kern.c
>  create mode 100644 tools/testing/selftests/bpf/test_descendant_of_user.c
>
> diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> index 90f70d2c7c22..4b63d7105ba2 100644
> --- a/tools/testing/selftests/bpf/.gitignore
> +++ b/tools/testing/selftests/bpf/.gitignore
> @@ -43,3 +43,4 @@ test_sockopt
>  test_sockopt_sk
>  test_sockopt_multi
>  test_tcp_rtt
> +test_descendant_of_user
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 2620406a53ec..b3dc1e26c41c 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -27,7 +27,7 @@ TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test
>         test_cgroup_storage test_select_reuseport test_section_names \
>         test_netcnt test_tcpnotify_user test_sock_fields test_sysctl test_hashmap \
>         test_btf_dump test_cgroup_attach xdping test_sockopt test_sockopt_sk \
> -       test_sockopt_multi test_tcp_rtt
> +       test_sockopt_multi test_tcp_rtt test_descendant_of_user

Could you please instead add this test as part of test_progs? See for
instance prog_tests/attach_probe.c for recently added test.

>
>  BPF_OBJ_FILES = $(patsubst %.c,%.o, $(notdir $(wildcard progs/*.c)))
>  TEST_GEN_FILES = $(BPF_OBJ_FILES)
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index 5a3d92c8bec8..7525783ffbc9 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -1,4 +1,6 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
> +#include <sys/types.h>
> +
>  #ifndef __BPF_HELPERS_H
>  #define __BPF_HELPERS_H
>
> @@ -228,6 +230,7 @@ static void *(*bpf_sk_storage_get)(void *map, struct bpf_sock *sk,
>  static int (*bpf_sk_storage_delete)(void *map, struct bpf_sock *sk) =
>         (void *)BPF_FUNC_sk_storage_delete;
>  static int (*bpf_send_signal)(unsigned sig) = (void *)BPF_FUNC_send_signal;
> +static int (*bpf_descendant_of)(pid_t pid) = (void *) BPF_FUNC_descendant_of;

Can you split bpf_helpers.h update into a separate commit?

>
>  /* llvm builtin functions that eBPF C program may use to
>   * emit BPF_LD_ABS and BPF_LD_IND instructions
> diff --git a/tools/testing/selftests/bpf/progs/test_descendant_of_kern.c b/tools/testing/selftests/bpf/progs/test_descendant_of_kern.c
> new file mode 100644
> index 000000000000..802e01595527
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_descendant_of_kern.c
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/bpf.h>
> +#include "bpf_helpers.h"
> +
> +struct bpf_map_def SEC("maps") pidmap = {
> +       .type = BPF_MAP_TYPE_ARRAY,
> +       .key_size = sizeof(__u32),
> +       .value_size = sizeof(__u32),
> +       .max_entries = 2,
> +};
> +
> +struct bpf_map_def SEC("maps") resultmap = {
> +       .type = BPF_MAP_TYPE_ARRAY,
> +       .key_size = sizeof(__u32),
> +       .value_size = sizeof(__u32),
> +       .max_entries = 1,
> +};

Please update this to use new BTF-defined maps (see lots of recently
converted tests for example).

> +
> +SEC("tracepoint/syscalls/sys_enter_open")
> +int trace(void *ctx)
> +{
> +       __u32 pid = bpf_get_current_pid_tgid();
> +       __u32 current_key = 0, ancestor_key = 1, *expected_pid, *ancestor_pid;
> +       __u32 *val;
> +
> +       expected_pid = bpf_map_lookup_elem(&pidmap, &current_key);
> +       if (!expected_pid || *expected_pid != pid)
> +               return 0;
> +
> +       ancestor_pid = bpf_map_lookup_elem(&pidmap, &ancestor_key);
> +       if (!ancestor_pid)
> +               return 0;
> +
> +       val = bpf_map_lookup_elem(&resultmap, &current_key);
> +       if (val)
> +               *val = bpf_descendant_of(*ancestor_pid);
> +
> +       return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> +__u32 _version SEC("version") = 1;
> diff --git a/tools/testing/selftests/bpf/test_descendant_of_user.c b/tools/testing/selftests/bpf/test_descendant_of_user.c
> new file mode 100644
> index 000000000000..f616c8c976a4
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_descendant_of_user.c
> @@ -0,0 +1,266 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <syscall.h>
> +#include <unistd.h>
> +#include <linux/perf_event.h>
> +#include <sys/ioctl.h>
> +#include <sys/types.h>
> +#include <sys/wait.h>
> +
> +#include <bpf/bpf.h>
> +#include <bpf/libbpf.h>
> +
> +#define CHECK(condition, tag, format...)                                       \
> +       ({                                                                     \
> +               int __ret = !!(condition);                                     \
> +               if (__ret) {                                                   \
> +                       printf("%s:FAIL:%s ", __func__, tag);                  \
> +                       printf(format);                                        \
> +               } else {                                                       \
> +                       printf("%s:PASS:%s\n", __func__, tag);                 \
> +               }                                                              \
> +               __ret;                                                         \
> +       })

You won't need this if done as part of test_progs.

> +
> +static int bpf_find_map(const char *test, struct bpf_object *obj,
> +                       const char *name)
> +{
> +       struct bpf_map *map;
> +
> +       map = bpf_object__find_map_by_name(obj, name);
> +       if (!map)
> +               return -1;
> +       return bpf_map__fd(map);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +       const char *probe_name = "syscalls/sys_enter_open";
> +       const char *file = "test_descendant_of_kern.o";
> +       int err, bytes, efd, prog_fd, pmu_fd;
> +       int resultmap_fd, pidmap_fd;
> +       struct perf_event_attr attr = {};
> +       struct bpf_object *obj;
> +       __u32 descendant_of_result = 0;
> +       __u32 key = 0, pid;
> +       int exit_code = EXIT_FAILURE;
> +       char buf[256];
> +
> +       int child_pid, ancestor_pid, root_fd, nonexistant = -42;
> +       __u32 ancestor_key = 1;
> +       int pipefd[2];
> +       char marker[1];
> +
> +       err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
> +       if (CHECK(err, "bpf_prog_load", "err %d errno %d\n", err, errno))
> +               goto fail;
> +
> +       resultmap_fd = bpf_find_map(__func__, obj, "resultmap");
> +       if (CHECK(resultmap_fd < 0, "bpf_find_map", "err %d errno %d\n",
> +                 resultmap_fd, errno))
> +               goto close_prog;
> +
> +       pidmap_fd = bpf_find_map(__func__, obj, "pidmap");
> +       if (CHECK(pidmap_fd < 0, "bpf_find_map", "err %d errno %d\n", pidmap_fd,
> +                 errno))
> +               goto close_prog;
> +
> +       pid = getpid();
> +       bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
> +       bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
> +
> +       snprintf(buf, sizeof(buf), "/sys/kernel/debug/tracing/events/%s/id",
> +                probe_name);
> +       efd = open(buf, O_RDONLY, 0);
> +       if (CHECK(efd < 0, "open", "err %d errno %d\n", efd, errno))
> +               goto close_prog;
> +       bytes = read(efd, buf, sizeof(buf));
> +       close(efd);
> +       if (CHECK(bytes <= 0 || bytes >= sizeof(buf), "read",
> +                 "bytes %d errno %d\n", bytes, errno))
> +               goto close_prog;
> +
> +       attr.config = strtol(buf, NULL, 0);
> +       attr.type = PERF_TYPE_TRACEPOINT;
> +       attr.sample_type = PERF_SAMPLE_RAW;
> +       attr.sample_period = 1;
> +       attr.wakeup_events = 1;
> +
> +       pmu_fd = syscall(__NR_perf_event_open, &attr, getpid(), -1, -1, 0);
> +       if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n", pmu_fd,
> +                 errno))
> +               goto close_prog;
> +
> +       err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
> +       if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n", err,
> +                 errno))
> +               goto close_pmu;
> +
> +       err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
> +       if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n", err,
> +                 errno))
> +               goto close_pmu;

Can you please switch all this to new libbpf tracing APIs? see
prog_tests/attach_probe.c for examples.

> +
> +       // Test that descendant_of(current->pid) is true
> +       bpf_map_update_elem(pidmap_fd, &key, &pid, 0);
> +       bpf_map_update_elem(pidmap_fd, &ancestor_key, &pid, 0);
> +       bpf_map_update_elem(resultmap_fd, &key, &nonexistant, 0);
> +
> +       root_fd = open("/", O_RDONLY);
> +       if (CHECK(efd < 0, "open", "errno %d\n", errno))
> +               goto close_prog;
> +       close(root_fd);

I'd suggest to just use raw_tracepoint 'sys_enter', which would be
easy to trigger with just `usleep(1)`. Makes for quite simpler code.
See, e.g., tools/testing/selftests/bpf/prog_tests/get_stack_raw_tp.c

> +
> +       err = bpf_map_lookup_elem(resultmap_fd, &key, &descendant_of_result);
> +       if (CHECK(err, "bpf_map_lookup_elem", "err %d errno %d\n", err, errno))
> +               goto close_pmu;
> +       if (CHECK(descendant_of_result != 1,
> +                 "descendant_of is true with same pid", "%d == %d\n",
> +                 descendant_of_result, 1))
> +               goto close_pmu;
> +

<snip>

Re: [PATCH v6 bpf-next 0/3] bpf: add bpf_descendant_of helper

[Daniel Borkmann]

On 07/10/2019 08:00 PM, Javier Honduvilla Coto wrote:
> Hi all,
> 
> This patch adds the bpf_descendant_of helper which accepts a PID and
> returns 1 if the PID of the process currently being executed is a
> descendant of it or if it's itself. Returns 0 otherwise. The passed
> PID should be the one as seen from the "global" pid namespace as the
> processes' PIDs in the hierarchy are resolved using the context of said
> initial namespace.
> 
> This is very useful in tracing programs when we want to filter by a
> given PID and all the children it might spawn. The current workarounds
> most people implement for this purpose have issues:
> 
> - Attaching to process spawning syscalls and dynamically add those PIDs
> to some bpf map that would be used to filter is cumbersome and
> potentially racy.
> - Unrolling some loop to perform what this helper is doing consumes lots
> of instructions. That and the impossibility to jump backwards makes it
> really hard to be correct in really large process chains.
> 
> 
> Let me know what do you think!
> 
> Thanks,
> 
> ---
> Changes in V6:
>         - Small style fix
>         - Clarify in the docs that we are resolving PIDs using the global,
> initial PID namespace, and the provided *pid* argument should be global, too
>         - Changed the way we assert on the helper return value
> 
> Changes in V5:
>         - Addressed code review feedback
>         - Renamed from progenyof => descendant_of as suggested by Jon Haslam
> and Brendan Gregg
> 
> Changes in V4:
>         - Rebased on latest bpf-next after merge window
> 
> Changes in V3:
>         - Removed RCU read (un)locking as BPF programs alredy run in RCU locked
>                 context
>         - progenyof(0) now returns 1, which, semantically makes more sense
>         - Added new test case for PID 0 and changed sentinel value for errors
>         - Rebase on latest bpf-next/master
>         - Used my work email as somehow I accidentally used my personal one in v2
> 
> Changes in V2:
>         - Adding missing docs in include/uapi/linux/bpf.h
> 

bpf-next is currently closed due to merge window, please resubmit once it reopens.

Thanks,
Daniel