Re: [PATCH bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/10/23 11:35 AM, Dave Marchevsky wrote:
This patch adds kfuncs bpf_iter_task_vma_{new,next,destroy} which allow
creation and manipulation of struct bpf_iter_task_vma in open-coded
iterator style. BPF programs can use these kfuncs directly or through
bpf_for_each macro for natural-looking iteration of all task vmas.

The implementation borrows heavily from bpf_find_vma helper's locking -
differing only in that it holds the mmap_read lock for all iterations
while the helper only executes its provided callback on a maximum of 1
vma. Aside from locking, struct vma_iterator and vma_next do all the
heavy lifting.

The newly-added struct bpf_iter_task_vma has a name collision with a
selftest for the seq_file task_vma iter's bpf skel, so the selftests/bpf/progs
file is renamed in order to avoid the collision.

Signed-off-by: Dave Marchevsky <davemarchevsky@xxxxxx>
Cc: Nathan Slingerland <slinger@xxxxxxxx>
---
  include/uapi/linux/bpf.h                      |  5 ++
  kernel/bpf/helpers.c                          |  3 +
  kernel/bpf/task_iter.c                        | 56 +++++++++++++++++++
  tools/include/uapi/linux/bpf.h                |  5 ++
  tools/lib/bpf/bpf_helpers.h                   |  8 +++
  .../selftests/bpf/prog_tests/bpf_iter.c       | 26 ++++-----
  ...f_iter_task_vma.c => bpf_iter_task_vmas.c} |  0
  7 files changed, 90 insertions(+), 13 deletions(-)
  rename tools/testing/selftests/bpf/progs/{bpf_iter_task_vma.c => bpf_iter_task_vmas.c} (100%)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index d21deb46f49f..c4a65968f9f5 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -7291,4 +7291,9 @@ struct bpf_iter_num {
  	__u64 __opaque[1];
  } __attribute__((aligned(8)));
+struct bpf_iter_task_vma {
+	__u64 __opaque[9]; /* See bpf_iter_num comment above */
+	char __opaque_c[3];
+} __attribute__((aligned(8)));
+
  #endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index eb91cae0612a..7a06dea749f1 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2482,6 +2482,9 @@ BTF_ID_FLAGS(func, bpf_dynptr_slice_rdwr, KF_RET_NULL)
  BTF_ID_FLAGS(func, bpf_iter_num_new, KF_ITER_NEW)
  BTF_ID_FLAGS(func, bpf_iter_num_next, KF_ITER_NEXT | KF_RET_NULL)
  BTF_ID_FLAGS(func, bpf_iter_num_destroy, KF_ITER_DESTROY)
+BTF_ID_FLAGS(func, bpf_iter_task_vma_new, KF_ITER_NEW)
+BTF_ID_FLAGS(func, bpf_iter_task_vma_next, KF_ITER_NEXT | KF_RET_NULL)
+BTF_ID_FLAGS(func, bpf_iter_task_vma_destroy, KF_ITER_DESTROY)
  BTF_ID_FLAGS(func, bpf_dynptr_adjust)
  BTF_ID_FLAGS(func, bpf_dynptr_is_null)
  BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly)
diff --git a/kernel/bpf/task_iter.c b/kernel/bpf/task_iter.c
index c4ab9d6cdbe9..76be9998a65a 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -8,6 +8,7 @@
  #include <linux/fdtable.h>
  #include <linux/filter.h>
  #include <linux/btf_ids.h>
+#include <linux/mm_types.h>
  #include "mmap_unlock_work.h"
static const char * const iter_task_type_names[] = {
@@ -823,6 +824,61 @@ const struct bpf_func_proto bpf_find_vma_proto = {
  	.arg5_type	= ARG_ANYTHING,
  };
+struct bpf_iter_task_vma_kern {
+	struct mm_struct *mm;
+	struct mmap_unlock_irq_work *work;
+	struct vma_iterator vmi;
+} __attribute__((aligned(8)));
+
+__bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it,
+				      struct task_struct *task, u64 addr)
+{
+	struct bpf_iter_task_vma_kern *i = (void *)it;
+	bool irq_work_busy = false;
+
+	BUILD_BUG_ON(sizeof(struct bpf_iter_task_vma_kern) != sizeof(struct bpf_iter_task_vma));
+	BUILD_BUG_ON(__alignof__(struct bpf_iter_task_vma_kern) != __alignof__(struct bpf_iter_task_vma));
+
+	BTF_TYPE_EMIT(struct bpf_iter_task_vma);
+
+	/* NULL i->mm signals failed bpf_iter_task_vma initialization.
+	 * i->work == NULL is valid.
+	 */
+	i->mm = NULL;
+	if (!task)
+		return -ENOENT;
+
+	i->mm = task->mm;
+	if (!i->mm)
+		return -ENOENT;

We might have an issue here as well if task is in __put_task_struct() stage. It is possible that we did i->mm from task->mm and then
task is freed and 'mm' is reused by somebody self.

To prevent such cases, I suggest we try to take a reference
of 'task' first. If we can get a reference then task is valid
and task->mm will not be freed and we will be fine.

+
+	irq_work_busy = bpf_mmap_unlock_get_irq_work(&i->work);
+	if (irq_work_busy || !mmap_read_trylock(i->mm)) {
+		i->mm = NULL;
+		return -EBUSY;
+	}
+
+	vma_iter_init(&i->vmi, i->mm, addr);
+	return 0;
+}
+
[...]




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux