* [PATCH v4 bpf-next 0/3] Open-coded task_vma iter
@ 2023-10-02 19:53 Dave Marchevsky
2023-10-02 19:53 ` [PATCH v4 bpf-next 1/3] bpf: Don't explicitly emit BTF for struct btf_iter_num Dave Marchevsky
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Dave Marchevsky @ 2023-10-02 19:53 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Kernel Team, Dave Marchevsky
At Meta we have a profiling daemon which periodically collects
information on many hosts. This collection usually involves grabbing
stacks (user and kernel) using perf_event BPF progs and later symbolicating
them. For user stacks we try to use BPF_F_USER_BUILD_ID and rely on
remote symbolication, but BPF_F_USER_BUILD_ID doesn't always succeed. In
those cases we must fall back to digging around in /proc/PID/maps to map
virtual address to (binary, offset). The /proc/PID/maps digging does not
occur synchronously with stack collection, so the process might already
be gone, in which case it won't have /proc/PID/maps and we will fail to
symbolicate.
This 'exited process problem' doesn't occur very often as
most of the prod services we care to profile are long-lived daemons, but
there are enough usecases to warrant a workaround: a BPF program which
can be optionally loaded at data collection time and essentially walks
/proc/PID/maps. Currently this is done by walking the vma list:
struct vm_area_struct* mmap = BPF_CORE_READ(mm, mmap);
mmap_next = BPF_CORE_READ(rmap, vm_next); /* in a loop */
Since commit 763ecb035029 ("mm: remove the vma linked list") there's no
longer a vma linked list to walk. Walking the vma maple tree is not as
simple as hopping struct vm_area_struct->vm_next. Luckily,
commit f39af05949a4 ("mm: add VMA iterator"), another commit in that series,
added struct vma_iterator and for_each_vma macro for easy vma iteration. If
similar functionality was exposed to BPF programs, it would be perfect for our
usecase.
This series adds such functionality, specifically a BPF equivalent of
for_each_vma using the open-coded iterator style.
Notes:
* This approach was chosen after discussion on a previous series [0] which
attempted to solve the same problem by adding a BPF_F_VMA_NEXT flag to
bpf_find_vma.
* Unlike the task_vma bpf_iter, the open-coded iterator kfuncs here do not
drop the vma read lock between iterations. See Alexei's response in [0].
* The [vsyscall] page isn't really part of task->mm's vmas, but
/proc/PID/maps returns information about it anyways. The vma iter added
here does not do the same. See comment on selftest in patch 3.
* bpf_iter_task_vma allocates a _data struct which contains - among other
things - struct vma_iterator, using BPF allocator and keeps a pointer to
the bpf_iter_task_vma_data. This is done in order to prevent changes to
struct ma_state - which is wrapped by struct vma_iterator - from
necessitating changes to uapi struct bpf_iter_task_vma.
Changelog:
v3 -> v4: https://lore.kernel.org/bpf/20230822050558.2937659-1-davemarchevsky@fb.com/
Patch 1 ("bpf: Don't explicitly emit BTF for struct btf_iter_num")
* Add Andrii ack
Patch 2 ("bpf: Introduce task_vma open-coded iterator kfuncs")
* Mark bpf_iter_task_vma_new args KF_RCU and remove now-unnecessary !task
check (Yonghong)
* Although KF_RCU is a function-level flag, in reality it only applies to
the task_struct *task parameter, as the other two params are a scalar int
and a specially-handled KF_ARG_PTR_TO_ITER
* Remove struct bpf_iter_task_vma definition from uapi headers, define in
kernel/bpf/task_iter.c instead (Andrii)
Patch 3 ("selftests/bpf: Add tests for open-coded task_vma iter")
* Use a local var when looping over vmas to track map idx. Update vmas_seen
global after done iterating. Don't start iterating or update vmas_seen if
vmas_seen global is nonzero. (Andrii)
* Move getpgid() call to correct spot - above skel detach. (Andrii)
v2 -> v3: https://lore.kernel.org/bpf/20230821173415.1970776-1-davemarchevsky@fb.com/
Patch 1 ("bpf: Don't explicitly emit BTF for struct btf_iter_num")
* Add Yonghong ack
Patch 2 ("bpf: Introduce task_vma open-coded iterator kfuncs")
* UAPI bpf header and tools/ version should match
* Add bpf_iter_task_vma_kern_data which bpf_iter_task_vma_kern points to,
bpf_mem_alloc/free it instead of just vma_iterator. (Alexei)
* Inner data ptr == NULL implies initialization failed
v1 -> v2: https://lore.kernel.org/bpf/20230810183513.684836-1-davemarchevsky@fb.com/
* Patch 1
* Now removes the unnecessary BTF_TYPE_EMIT instead of changing the
type (Yonghong)
* Patch 2
* Don't do unnecessary BTF_TYPE_EMIT (Yonghong)
* Bump task refcount to prevent ->mm reuse (Yonghong)
* Keep a pointer to vma_iterator in bpf_iter_task_vma, alloc/free
via BPF mem allocator (Yonghong, Stanislav)
* Patch 3
Patch summary:
* Patch 1 is a tiny fix I ran into while implementing the vma iter in this
series. It can be applied independently.
* Patch 2 is the meat of the implementation
* Patch 3 adds tests for the new functionality
* Existing iter tests exercise failure cases (e.g. prog that doesn't call
_destroy()). I didn't replicate them in this series, but am happy to add
them in v2 if folks feel that it would be worthwhile.
[0]: https://lore.kernel.org/bpf/20230801145414.418145-1-davemarchevsky@fb.com/
Dave Marchevsky (3):
bpf: Don't explicitly emit BTF for struct btf_iter_num
bpf: Introduce task_vma open-coded iterator kfuncs
selftests/bpf: Add tests for open-coded task_vma iter
kernel/bpf/bpf_iter.c | 2 -
kernel/bpf/helpers.c | 3 +
kernel/bpf/task_iter.c | 85 +++++++++++++++++++
tools/lib/bpf/bpf_helpers.h | 8 ++
.../selftests/bpf/prog_tests/bpf_iter.c | 26 +++---
.../testing/selftests/bpf/prog_tests/iters.c | 71 ++++++++++++++++
...f_iter_task_vma.c => bpf_iter_task_vmas.c} | 0
.../selftests/bpf/progs/iters_task_vma.c | 63 ++++++++++++++
8 files changed, 243 insertions(+), 15 deletions(-)
rename tools/testing/selftests/bpf/progs/{bpf_iter_task_vma.c => bpf_iter_task_vmas.c} (100%)
create mode 100644 tools/testing/selftests/bpf/progs/iters_task_vma.c
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v4 bpf-next 1/3] bpf: Don't explicitly emit BTF for struct btf_iter_num
2023-10-02 19:53 [PATCH v4 bpf-next 0/3] Open-coded task_vma iter Dave Marchevsky
@ 2023-10-02 19:53 ` Dave Marchevsky
2023-10-02 19:53 ` [PATCH v4 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs Dave Marchevsky
2023-10-02 19:53 ` [PATCH v4 bpf-next 3/3] selftests/bpf: Add tests for open-coded task_vma iter Dave Marchevsky
2 siblings, 0 replies; 8+ messages in thread
From: Dave Marchevsky @ 2023-10-02 19:53 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Kernel Team, Dave Marchevsky, Yonghong Song
Commit 6018e1f407cc ("bpf: implement numbers iterator") added the
BTF_TYPE_EMIT line that this patch is modifying. The struct btf_iter_num
doesn't exist, so only a forward declaration is emitted in BTF:
FWD 'btf_iter_num' fwd_kind=struct
That commit was probably hoping to ensure that struct bpf_iter_num is
emitted in vmlinux BTF. A previous version of this patch changed the
line to emit the correct type, but Yonghong confirmed that it would
definitely be emitted regardless in [0], so this patch simply removes
the line.
This isn't marked "Fixes" because the extraneous btf_iter_num FWD wasn't
causing any issues that I noticed, aside from mild confusion when I
looked through the code.
[0]: https://lore.kernel.org/bpf/25d08207-43e6-36a8-5e0f-47a913d4cda5@linux.dev/
Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
Acked-by: Andrii Nakryiko <andrii@kernel.org>
---
kernel/bpf/bpf_iter.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/kernel/bpf/bpf_iter.c b/kernel/bpf/bpf_iter.c
index 96856f130cbf..833faa04461b 100644
--- a/kernel/bpf/bpf_iter.c
+++ b/kernel/bpf/bpf_iter.c
@@ -793,8 +793,6 @@ __bpf_kfunc int bpf_iter_num_new(struct bpf_iter_num *it, int start, int end)
BUILD_BUG_ON(sizeof(struct bpf_iter_num_kern) != sizeof(struct bpf_iter_num));
BUILD_BUG_ON(__alignof__(struct bpf_iter_num_kern) != __alignof__(struct bpf_iter_num));
- BTF_TYPE_EMIT(struct btf_iter_num);
-
/* start == end is legit, it's an empty range and we'll just get NULL
* on first (and any subsequent) bpf_iter_num_next() call
*/
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs
2023-10-02 19:53 [PATCH v4 bpf-next 0/3] Open-coded task_vma iter Dave Marchevsky
2023-10-02 19:53 ` [PATCH v4 bpf-next 1/3] bpf: Don't explicitly emit BTF for struct btf_iter_num Dave Marchevsky
@ 2023-10-02 19:53 ` Dave Marchevsky
2023-10-02 21:29 ` kernel test robot
` (2 more replies)
2023-10-02 19:53 ` [PATCH v4 bpf-next 3/3] selftests/bpf: Add tests for open-coded task_vma iter Dave Marchevsky
2 siblings, 3 replies; 8+ messages in thread
From: Dave Marchevsky @ 2023-10-02 19:53 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Kernel Team, Dave Marchevsky,
Nathan Slingerland
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.
A pointer to an inner data struct, struct bpf_iter_task_vma_data, is the
only field in struct bpf_iter_task_vma. This is because the inner data
struct contains a struct vma_iterator (not ptr), whose size is likely to
change under us. If bpf_iter_task_vma_kern contained vma_iterator directly
such a change would require change in opaque bpf_iter_task_vma struct's
size. So better to allocate vma_iterator using BPF allocator, and since
that alloc must already succeed, might as well allocate all iter fields,
thereby freezing struct bpf_iter_task_vma size.
Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
Cc: Nathan Slingerland <slinger@meta.com>
---
kernel/bpf/helpers.c | 3 +
kernel/bpf/task_iter.c | 85 +++++++++++++++++++
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
5 files changed, 109 insertions(+), 13 deletions(-)
rename tools/testing/selftests/bpf/progs/{bpf_iter_task_vma.c => bpf_iter_task_vmas.c} (100%)
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index dd1c69ee3375..6b2373db65bd 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2549,6 +2549,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 | KF_RCU)
+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 7473068ed313..5c8e559be5e5 100644
--- a/kernel/bpf/task_iter.c
+++ b/kernel/bpf/task_iter.c
@@ -7,7 +7,9 @@
#include <linux/fs.h>
#include <linux/fdtable.h>
#include <linux/filter.h>
+#include <linux/bpf_mem_alloc.h>
#include <linux/btf_ids.h>
+#include <linux/mm_types.h>
#include "mmap_unlock_work.h"
static const char * const iter_task_type_names[] = {
@@ -803,6 +805,89 @@ const struct bpf_func_proto bpf_find_vma_proto = {
.arg5_type = ARG_ANYTHING,
};
+struct bpf_iter_task_vma_kern_data {
+ struct task_struct *task;
+ struct mm_struct *mm;
+ struct mmap_unlock_irq_work *work;
+ struct vma_iterator vmi;
+};
+
+struct bpf_iter_task_vma {
+ /* opaque iterator state; having __u64 here allows to preserve correct
+ * alignment requirements in vmlinux.h, generated from BTF
+ */
+ __u64 __opaque[1];
+} __attribute__((aligned(8)));
+
+/* Non-opaque version of bpf_iter_task_vma */
+struct bpf_iter_task_vma_kern {
+ struct bpf_iter_task_vma_kern_data *data;
+} __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 *kit = (void *)it;
+ bool irq_work_busy = false;
+ int err;
+
+ 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));
+
+ /* is_iter_reg_valid_uninit guarantees that kit hasn't been initialized
+ * before, so non-NULL kit->data doesn't point to previously
+ * bpf_mem_alloc'd bpf_iter_task_vma_kern_data
+ */
+ kit->data = bpf_mem_alloc(&bpf_global_ma, sizeof(struct bpf_iter_task_vma_kern_data));
+ if (!kit->data)
+ return -ENOMEM;
+
+ kit->data->task = get_task_struct(task);
+ kit->data->mm = task->mm;
+ if (!kit->data->mm) {
+ err = -ENOENT;
+ goto err_cleanup_iter;
+ }
+
+ /* kit->data->work == NULL is valid after bpf_mmap_unlock_get_irq_work */
+ irq_work_busy = bpf_mmap_unlock_get_irq_work(&kit->data->work);
+ if (irq_work_busy || !mmap_read_trylock(kit->data->mm)) {
+ err = -EBUSY;
+ goto err_cleanup_iter;
+ }
+
+ vma_iter_init(&kit->data->vmi, kit->data->mm, addr);
+ return 0;
+
+err_cleanup_iter:
+ if (kit->data->task)
+ put_task_struct(kit->data->task);
+ bpf_mem_free(&bpf_global_ma, kit->data);
+ /* NULL kit->data signals failed bpf_iter_task_vma initialization */
+ kit->data = NULL;
+ return err;
+}
+
+__bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it)
+{
+ struct bpf_iter_task_vma_kern *kit = (void *)it;
+
+ if (!kit->data) /* bpf_iter_task_vma_new failed */
+ return NULL;
+ return vma_next(&kit->data->vmi);
+}
+
+__bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it)
+{
+ struct bpf_iter_task_vma_kern *kit = (void *)it;
+
+ if (kit->data) {
+ bpf_mmap_unlock_mm(kit->data->work, kit->data->mm);
+ put_task_struct(kit->data->task);
+ bpf_mem_free(&bpf_global_ma, kit->data);
+ }
+}
+
DEFINE_PER_CPU(struct mmap_unlock_irq_work, mmap_unlock_work);
static void do_mmap_read_unlock(struct irq_work *entry)
diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
index 77ceea575dc7..c6abb7fd8d73 100644
--- a/tools/lib/bpf/bpf_helpers.h
+++ b/tools/lib/bpf/bpf_helpers.h
@@ -303,6 +303,14 @@ extern int bpf_iter_num_new(struct bpf_iter_num *it, int start, int end) __weak
extern int *bpf_iter_num_next(struct bpf_iter_num *it) __weak __ksym;
extern void bpf_iter_num_destroy(struct bpf_iter_num *it) __weak __ksym;
+struct bpf_iter_task_vma;
+
+extern int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it,
+ struct task_struct *task,
+ unsigned long addr) __weak __ksym;
+extern struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it) __weak __ksym;
+extern void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) __weak __ksym;
+
#ifndef bpf_for_each
/* bpf_for_each(iter_type, cur_elem, args...) provides generic construct for
* using BPF open-coded iterators without having to write mundane explicit
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
index 1f02168103dd..41aba139b20b 100644
--- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
+++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
@@ -10,7 +10,7 @@
#include "bpf_iter_task.skel.h"
#include "bpf_iter_task_stack.skel.h"
#include "bpf_iter_task_file.skel.h"
-#include "bpf_iter_task_vma.skel.h"
+#include "bpf_iter_task_vmas.skel.h"
#include "bpf_iter_task_btf.skel.h"
#include "bpf_iter_tcp4.skel.h"
#include "bpf_iter_tcp6.skel.h"
@@ -1399,19 +1399,19 @@ static void str_strip_first_line(char *str)
static void test_task_vma_common(struct bpf_iter_attach_opts *opts)
{
int err, iter_fd = -1, proc_maps_fd = -1;
- struct bpf_iter_task_vma *skel;
+ struct bpf_iter_task_vmas *skel;
int len, read_size = 4;
char maps_path[64];
- skel = bpf_iter_task_vma__open();
- if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vma__open"))
+ skel = bpf_iter_task_vmas__open();
+ if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vmas__open"))
return;
skel->bss->pid = getpid();
skel->bss->one_task = opts ? 1 : 0;
- err = bpf_iter_task_vma__load(skel);
- if (!ASSERT_OK(err, "bpf_iter_task_vma__load"))
+ err = bpf_iter_task_vmas__load(skel);
+ if (!ASSERT_OK(err, "bpf_iter_task_vmas__load"))
goto out;
skel->links.proc_maps = bpf_program__attach_iter(
@@ -1462,25 +1462,25 @@ static void test_task_vma_common(struct bpf_iter_attach_opts *opts)
out:
close(proc_maps_fd);
close(iter_fd);
- bpf_iter_task_vma__destroy(skel);
+ bpf_iter_task_vmas__destroy(skel);
}
static void test_task_vma_dead_task(void)
{
- struct bpf_iter_task_vma *skel;
+ struct bpf_iter_task_vmas *skel;
int wstatus, child_pid = -1;
time_t start_tm, cur_tm;
int err, iter_fd = -1;
int wait_sec = 3;
- skel = bpf_iter_task_vma__open();
- if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vma__open"))
+ skel = bpf_iter_task_vmas__open();
+ if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vmas__open"))
return;
skel->bss->pid = getpid();
- err = bpf_iter_task_vma__load(skel);
- if (!ASSERT_OK(err, "bpf_iter_task_vma__load"))
+ err = bpf_iter_task_vmas__load(skel);
+ if (!ASSERT_OK(err, "bpf_iter_task_vmas__load"))
goto out;
skel->links.proc_maps = bpf_program__attach_iter(
@@ -1533,7 +1533,7 @@ static void test_task_vma_dead_task(void)
out:
waitpid(child_pid, &wstatus, 0);
close(iter_fd);
- bpf_iter_task_vma__destroy(skel);
+ bpf_iter_task_vmas__destroy(skel);
}
void test_bpf_sockmap_map_iter_fd(void)
diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_task_vma.c b/tools/testing/selftests/bpf/progs/bpf_iter_task_vmas.c
similarity index 100%
rename from tools/testing/selftests/bpf/progs/bpf_iter_task_vma.c
rename to tools/testing/selftests/bpf/progs/bpf_iter_task_vmas.c
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4 bpf-next 3/3] selftests/bpf: Add tests for open-coded task_vma iter
2023-10-02 19:53 [PATCH v4 bpf-next 0/3] Open-coded task_vma iter Dave Marchevsky
2023-10-02 19:53 ` [PATCH v4 bpf-next 1/3] bpf: Don't explicitly emit BTF for struct btf_iter_num Dave Marchevsky
2023-10-02 19:53 ` [PATCH v4 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs Dave Marchevsky
@ 2023-10-02 19:53 ` Dave Marchevsky
2023-10-03 22:46 ` Andrii Nakryiko
2 siblings, 1 reply; 8+ messages in thread
From: Dave Marchevsky @ 2023-10-02 19:53 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Kernel Team, Dave Marchevsky
The open-coded task_vma iter added earlier in this series allows for
natural iteration over a task's vmas using existing open-coded iter
infrastructure, specifically bpf_for_each.
This patch adds a test demonstrating this pattern and validating
correctness. The vma->vm_start and vma->vm_end addresses of the first
1000 vmas are recorded and compared to /proc/PID/maps output. As
expected, both see the same vmas and addresses - with the exception of
the [vsyscall] vma - which is explained in a comment in the prog_tests
program.
Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
---
.../testing/selftests/bpf/prog_tests/iters.c | 71 +++++++++++++++++++
.../selftests/bpf/progs/iters_task_vma.c | 63 ++++++++++++++++
2 files changed, 134 insertions(+)
create mode 100644 tools/testing/selftests/bpf/progs/iters_task_vma.c
diff --git a/tools/testing/selftests/bpf/prog_tests/iters.c b/tools/testing/selftests/bpf/prog_tests/iters.c
index 10804ae5ae97..7a18fc21f364 100644
--- a/tools/testing/selftests/bpf/prog_tests/iters.c
+++ b/tools/testing/selftests/bpf/prog_tests/iters.c
@@ -8,6 +8,7 @@
#include "iters_looping.skel.h"
#include "iters_num.skel.h"
#include "iters_testmod_seq.skel.h"
+#include "iters_task_vma.skel.h"
static void subtest_num_iters(void)
{
@@ -90,6 +91,74 @@ static void subtest_testmod_seq_iters(void)
iters_testmod_seq__destroy(skel);
}
+static void subtest_task_vma_iters(void)
+{
+ unsigned long start, end, bpf_iter_start, bpf_iter_end;
+ struct iters_task_vma *skel;
+ char rest_of_line[1000];
+ unsigned int seen;
+ int err;
+ FILE *f;
+
+ skel = iters_task_vma__open();
+ if (!ASSERT_OK_PTR(skel, "skel_open"))
+ return;
+
+ bpf_program__set_autoload(skel->progs.iter_task_vma_for_each, true);
+
+ err = iters_task_vma__load(skel);
+ if (!ASSERT_OK(err, "skel_load"))
+ goto cleanup;
+
+ skel->bss->target_pid = getpid();
+
+ err = iters_task_vma__attach(skel);
+ if (!ASSERT_OK(err, "skel_attach"))
+ goto cleanup;
+
+ getpgid(skel->bss->target_pid);
+ iters_task_vma__detach(skel);
+
+ if (!ASSERT_GT(skel->bss->vmas_seen, 0, "vmas_seen_gt_zero"))
+ goto cleanup;
+
+ f = fopen("/proc/self/maps", "r");
+ if (!ASSERT_OK_PTR(f, "proc_maps_fopen"))
+ goto cleanup;
+
+ seen = 0;
+ while (fscanf(f, "%lx-%lx %[^\n]\n", &start, &end, rest_of_line) == 3) {
+ /* [vsyscall] vma isn't _really_ part of task->mm vmas.
+ * /proc/PID/maps returns it when out of vmas - see get_gate_vma
+ * calls in fs/proc/task_mmu.c
+ */
+ if (strstr(rest_of_line, "[vsyscall]"))
+ continue;
+
+ err = bpf_map_lookup_elem(bpf_map__fd(skel->maps.vm_start),
+ &seen, &bpf_iter_start);
+ if (!ASSERT_OK(err, "vm_start map_lookup_elem"))
+ goto cleanup;
+
+ err = bpf_map_lookup_elem(bpf_map__fd(skel->maps.vm_end),
+ &seen, &bpf_iter_end);
+ if (!ASSERT_OK(err, "vm_end map_lookup_elem"))
+ goto cleanup;
+
+ ASSERT_EQ(bpf_iter_start, start, "vma->vm_start match");
+ ASSERT_EQ(bpf_iter_end, end, "vma->vm_end match");
+ seen++;
+ }
+
+ fclose(f);
+
+ if (!ASSERT_EQ(skel->bss->vmas_seen, seen, "vmas_seen_eq"))
+ goto cleanup;
+
+cleanup:
+ iters_task_vma__destroy(skel);
+}
+
void test_iters(void)
{
RUN_TESTS(iters_state_safety);
@@ -103,4 +172,6 @@ void test_iters(void)
subtest_num_iters();
if (test__start_subtest("testmod_seq"))
subtest_testmod_seq_iters();
+ if (test__start_subtest("task_vma"))
+ subtest_task_vma_iters();
}
diff --git a/tools/testing/selftests/bpf/progs/iters_task_vma.c b/tools/testing/selftests/bpf/progs/iters_task_vma.c
new file mode 100644
index 000000000000..1ac7ca0633be
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/iters_task_vma.c
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+
+#include <limits.h>
+#include <linux/errno.h>
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+pid_t target_pid = 0;
+unsigned int vmas_seen = 0;
+
+struct {
+ __uint(type, BPF_MAP_TYPE_ARRAY);
+ __uint(max_entries, 1000);
+ __type(key, int);
+ __type(value, unsigned long);
+} vm_start SEC(".maps");
+
+struct {
+ __uint(type, BPF_MAP_TYPE_ARRAY);
+ __uint(max_entries, 1000);
+ __type(key, int);
+ __type(value, unsigned long);
+} vm_end SEC(".maps");
+
+SEC("?raw_tp/sys_enter")
+int iter_task_vma_for_each(const void *ctx)
+{
+ struct task_struct *task = bpf_get_current_task_btf();
+ struct vm_area_struct *vma;
+ unsigned long *start, *end;
+ unsigned int seen = 0;
+
+ if (task->pid != target_pid)
+ return 0;
+
+ if (vmas_seen)
+ return 0;
+
+ bpf_for_each(task_vma, vma, task, 0) {
+ if (seen >= 1000)
+ break;
+
+ start = bpf_map_lookup_elem(&vm_start, &seen);
+ if (!start)
+ break;
+ *start = vma->vm_start;
+
+ end = bpf_map_lookup_elem(&vm_end, &seen);
+ if (!end)
+ break;
+ *end = vma->vm_end;
+
+ seen++;
+ }
+
+ if (!vmas_seen)
+ vmas_seen = seen;
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
--
2.34.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs
2023-10-02 19:53 ` [PATCH v4 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs Dave Marchevsky
@ 2023-10-02 21:29 ` kernel test robot
2023-10-03 16:28 ` kernel test robot
2023-10-03 22:39 ` Andrii Nakryiko
2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2023-10-02 21:29 UTC (permalink / raw)
To: Dave Marchevsky, bpf
Cc: oe-kbuild-all, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Kernel Team, Dave Marchevsky,
Nathan Slingerland
Hi Dave,
kernel test robot noticed the following build warnings:
[auto build test WARNING on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Dave-Marchevsky/bpf-Don-t-explicitly-emit-BTF-for-struct-btf_iter_num/20231003-035600
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20231002195341.2940874-3-davemarchevsky%40fb.com
patch subject: [PATCH v4 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231003/202310030525.R8YDHYsO-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231003/202310030525.R8YDHYsO-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310030525.R8YDHYsO-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> kernel/bpf/task_iter.c:827:17: warning: no previous prototype for 'bpf_iter_task_vma_new' [-Wmissing-prototypes]
827 | __bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it,
| ^~~~~~~~~~~~~~~~~~~~~
>> kernel/bpf/task_iter.c:871:36: warning: no previous prototype for 'bpf_iter_task_vma_next' [-Wmissing-prototypes]
871 | __bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it)
| ^~~~~~~~~~~~~~~~~~~~~~
>> kernel/bpf/task_iter.c:880:18: warning: no previous prototype for 'bpf_iter_task_vma_destroy' [-Wmissing-prototypes]
880 | __bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
vim +/bpf_iter_task_vma_new +827 kernel/bpf/task_iter.c
826
> 827 __bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it,
828 struct task_struct *task, u64 addr)
829 {
830 struct bpf_iter_task_vma_kern *kit = (void *)it;
831 bool irq_work_busy = false;
832 int err;
833
834 BUILD_BUG_ON(sizeof(struct bpf_iter_task_vma_kern) != sizeof(struct bpf_iter_task_vma));
835 BUILD_BUG_ON(__alignof__(struct bpf_iter_task_vma_kern) != __alignof__(struct bpf_iter_task_vma));
836
837 /* is_iter_reg_valid_uninit guarantees that kit hasn't been initialized
838 * before, so non-NULL kit->data doesn't point to previously
839 * bpf_mem_alloc'd bpf_iter_task_vma_kern_data
840 */
841 kit->data = bpf_mem_alloc(&bpf_global_ma, sizeof(struct bpf_iter_task_vma_kern_data));
842 if (!kit->data)
843 return -ENOMEM;
844
845 kit->data->task = get_task_struct(task);
846 kit->data->mm = task->mm;
847 if (!kit->data->mm) {
848 err = -ENOENT;
849 goto err_cleanup_iter;
850 }
851
852 /* kit->data->work == NULL is valid after bpf_mmap_unlock_get_irq_work */
853 irq_work_busy = bpf_mmap_unlock_get_irq_work(&kit->data->work);
854 if (irq_work_busy || !mmap_read_trylock(kit->data->mm)) {
855 err = -EBUSY;
856 goto err_cleanup_iter;
857 }
858
859 vma_iter_init(&kit->data->vmi, kit->data->mm, addr);
860 return 0;
861
862 err_cleanup_iter:
863 if (kit->data->task)
864 put_task_struct(kit->data->task);
865 bpf_mem_free(&bpf_global_ma, kit->data);
866 /* NULL kit->data signals failed bpf_iter_task_vma initialization */
867 kit->data = NULL;
868 return err;
869 }
870
> 871 __bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it)
872 {
873 struct bpf_iter_task_vma_kern *kit = (void *)it;
874
875 if (!kit->data) /* bpf_iter_task_vma_new failed */
876 return NULL;
877 return vma_next(&kit->data->vmi);
878 }
879
> 880 __bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it)
881 {
882 struct bpf_iter_task_vma_kern *kit = (void *)it;
883
884 if (kit->data) {
885 bpf_mmap_unlock_mm(kit->data->work, kit->data->mm);
886 put_task_struct(kit->data->task);
887 bpf_mem_free(&bpf_global_ma, kit->data);
888 }
889 }
890
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs
2023-10-02 19:53 ` [PATCH v4 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs Dave Marchevsky
2023-10-02 21:29 ` kernel test robot
@ 2023-10-03 16:28 ` kernel test robot
2023-10-03 22:39 ` Andrii Nakryiko
2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2023-10-03 16:28 UTC (permalink / raw)
To: Dave Marchevsky, bpf
Cc: oe-kbuild-all, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Martin KaFai Lau, Kernel Team, Dave Marchevsky,
Nathan Slingerland
Hi Dave,
kernel test robot noticed the following build warnings:
[auto build test WARNING on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Dave-Marchevsky/bpf-Don-t-explicitly-emit-BTF-for-struct-btf_iter_num/20231003-035600
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/20231002195341.2940874-3-davemarchevsky%40fb.com
patch subject: [PATCH v4 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs
config: i386-buildonly-randconfig-001-20231003 (https://download.01.org/0day-ci/archive/20231004/202310040045.CufS8H4U-lkp@intel.com/config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231004/202310040045.CufS8H4U-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202310040045.CufS8H4U-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> kernel/bpf/task_iter.c:827:17: warning: no previous declaration for 'bpf_iter_task_vma_new' [-Wmissing-declarations]
__bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it,
^~~~~~~~~~~~~~~~~~~~~
>> kernel/bpf/task_iter.c:871:36: warning: no previous declaration for 'bpf_iter_task_vma_next' [-Wmissing-declarations]
__bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it)
^~~~~~~~~~~~~~~~~~~~~~
>> kernel/bpf/task_iter.c:880:18: warning: no previous declaration for 'bpf_iter_task_vma_destroy' [-Wmissing-declarations]
__bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it)
^~~~~~~~~~~~~~~~~~~~~~~~~
vim +/bpf_iter_task_vma_new +827 kernel/bpf/task_iter.c
826
> 827 __bpf_kfunc int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it,
828 struct task_struct *task, u64 addr)
829 {
830 struct bpf_iter_task_vma_kern *kit = (void *)it;
831 bool irq_work_busy = false;
832 int err;
833
834 BUILD_BUG_ON(sizeof(struct bpf_iter_task_vma_kern) != sizeof(struct bpf_iter_task_vma));
835 BUILD_BUG_ON(__alignof__(struct bpf_iter_task_vma_kern) != __alignof__(struct bpf_iter_task_vma));
836
837 /* is_iter_reg_valid_uninit guarantees that kit hasn't been initialized
838 * before, so non-NULL kit->data doesn't point to previously
839 * bpf_mem_alloc'd bpf_iter_task_vma_kern_data
840 */
841 kit->data = bpf_mem_alloc(&bpf_global_ma, sizeof(struct bpf_iter_task_vma_kern_data));
842 if (!kit->data)
843 return -ENOMEM;
844
845 kit->data->task = get_task_struct(task);
846 kit->data->mm = task->mm;
847 if (!kit->data->mm) {
848 err = -ENOENT;
849 goto err_cleanup_iter;
850 }
851
852 /* kit->data->work == NULL is valid after bpf_mmap_unlock_get_irq_work */
853 irq_work_busy = bpf_mmap_unlock_get_irq_work(&kit->data->work);
854 if (irq_work_busy || !mmap_read_trylock(kit->data->mm)) {
855 err = -EBUSY;
856 goto err_cleanup_iter;
857 }
858
859 vma_iter_init(&kit->data->vmi, kit->data->mm, addr);
860 return 0;
861
862 err_cleanup_iter:
863 if (kit->data->task)
864 put_task_struct(kit->data->task);
865 bpf_mem_free(&bpf_global_ma, kit->data);
866 /* NULL kit->data signals failed bpf_iter_task_vma initialization */
867 kit->data = NULL;
868 return err;
869 }
870
> 871 __bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it)
872 {
873 struct bpf_iter_task_vma_kern *kit = (void *)it;
874
875 if (!kit->data) /* bpf_iter_task_vma_new failed */
876 return NULL;
877 return vma_next(&kit->data->vmi);
878 }
879
> 880 __bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it)
881 {
882 struct bpf_iter_task_vma_kern *kit = (void *)it;
883
884 if (kit->data) {
885 bpf_mmap_unlock_mm(kit->data->work, kit->data->mm);
886 put_task_struct(kit->data->task);
887 bpf_mem_free(&bpf_global_ma, kit->data);
888 }
889 }
890
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs
2023-10-02 19:53 ` [PATCH v4 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs Dave Marchevsky
2023-10-02 21:29 ` kernel test robot
2023-10-03 16:28 ` kernel test robot
@ 2023-10-03 22:39 ` Andrii Nakryiko
2 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2023-10-03 22:39 UTC (permalink / raw)
To: Dave Marchevsky
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Kernel Team, Nathan Slingerland
On Mon, Oct 2, 2023 at 12:53 PM Dave Marchevsky <davemarchevsky@fb.com> 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.
>
> A pointer to an inner data struct, struct bpf_iter_task_vma_data, is the
> only field in struct bpf_iter_task_vma. This is because the inner data
> struct contains a struct vma_iterator (not ptr), whose size is likely to
> change under us. If bpf_iter_task_vma_kern contained vma_iterator directly
> such a change would require change in opaque bpf_iter_task_vma struct's
> size. So better to allocate vma_iterator using BPF allocator, and since
> that alloc must already succeed, might as well allocate all iter fields,
> thereby freezing struct bpf_iter_task_vma size.
>
> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
> Cc: Nathan Slingerland <slinger@meta.com>
> ---
> kernel/bpf/helpers.c | 3 +
> kernel/bpf/task_iter.c | 85 +++++++++++++++++++
> 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
> 5 files changed, 109 insertions(+), 13 deletions(-)
> rename tools/testing/selftests/bpf/progs/{bpf_iter_task_vma.c => bpf_iter_task_vmas.c} (100%)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index dd1c69ee3375..6b2373db65bd 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2549,6 +2549,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 | KF_RCU)
> +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 7473068ed313..5c8e559be5e5 100644
> --- a/kernel/bpf/task_iter.c
> +++ b/kernel/bpf/task_iter.c
> @@ -7,7 +7,9 @@
> #include <linux/fs.h>
> #include <linux/fdtable.h>
> #include <linux/filter.h>
> +#include <linux/bpf_mem_alloc.h>
> #include <linux/btf_ids.h>
> +#include <linux/mm_types.h>
> #include "mmap_unlock_work.h"
>
> static const char * const iter_task_type_names[] = {
> @@ -803,6 +805,89 @@ const struct bpf_func_proto bpf_find_vma_proto = {
> .arg5_type = ARG_ANYTHING,
> };
>
> +struct bpf_iter_task_vma_kern_data {
> + struct task_struct *task;
> + struct mm_struct *mm;
> + struct mmap_unlock_irq_work *work;
> + struct vma_iterator vmi;
> +};
> +
> +struct bpf_iter_task_vma {
> + /* opaque iterator state; having __u64 here allows to preserve correct
> + * alignment requirements in vmlinux.h, generated from BTF
> + */
> + __u64 __opaque[1];
> +} __attribute__((aligned(8)));
> +
> +/* Non-opaque version of bpf_iter_task_vma */
> +struct bpf_iter_task_vma_kern {
> + struct bpf_iter_task_vma_kern_data *data;
> +} __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 *kit = (void *)it;
> + bool irq_work_busy = false;
> + int err;
> +
> + 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));
> +
> + /* is_iter_reg_valid_uninit guarantees that kit hasn't been initialized
> + * before, so non-NULL kit->data doesn't point to previously
> + * bpf_mem_alloc'd bpf_iter_task_vma_kern_data
> + */
> + kit->data = bpf_mem_alloc(&bpf_global_ma, sizeof(struct bpf_iter_task_vma_kern_data));
> + if (!kit->data)
> + return -ENOMEM;
> +
> + kit->data->task = get_task_struct(task);
> + kit->data->mm = task->mm;
> + if (!kit->data->mm) {
> + err = -ENOENT;
> + goto err_cleanup_iter;
> + }
> +
> + /* kit->data->work == NULL is valid after bpf_mmap_unlock_get_irq_work */
> + irq_work_busy = bpf_mmap_unlock_get_irq_work(&kit->data->work);
> + if (irq_work_busy || !mmap_read_trylock(kit->data->mm)) {
> + err = -EBUSY;
> + goto err_cleanup_iter;
> + }
> +
> + vma_iter_init(&kit->data->vmi, kit->data->mm, addr);
> + return 0;
> +
> +err_cleanup_iter:
> + if (kit->data->task)
> + put_task_struct(kit->data->task);
> + bpf_mem_free(&bpf_global_ma, kit->data);
> + /* NULL kit->data signals failed bpf_iter_task_vma initialization */
> + kit->data = NULL;
> + return err;
> +}
> +
> +__bpf_kfunc struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it)
> +{
> + struct bpf_iter_task_vma_kern *kit = (void *)it;
> +
> + if (!kit->data) /* bpf_iter_task_vma_new failed */
> + return NULL;
> + return vma_next(&kit->data->vmi);
> +}
> +
> +__bpf_kfunc void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it)
> +{
> + struct bpf_iter_task_vma_kern *kit = (void *)it;
> +
> + if (kit->data) {
> + bpf_mmap_unlock_mm(kit->data->work, kit->data->mm);
> + put_task_struct(kit->data->task);
> + bpf_mem_free(&bpf_global_ma, kit->data);
> + }
> +}
> +
> DEFINE_PER_CPU(struct mmap_unlock_irq_work, mmap_unlock_work);
>
> static void do_mmap_read_unlock(struct irq_work *entry)
> diff --git a/tools/lib/bpf/bpf_helpers.h b/tools/lib/bpf/bpf_helpers.h
> index 77ceea575dc7..c6abb7fd8d73 100644
> --- a/tools/lib/bpf/bpf_helpers.h
> +++ b/tools/lib/bpf/bpf_helpers.h
> @@ -303,6 +303,14 @@ extern int bpf_iter_num_new(struct bpf_iter_num *it, int start, int end) __weak
> extern int *bpf_iter_num_next(struct bpf_iter_num *it) __weak __ksym;
> extern void bpf_iter_num_destroy(struct bpf_iter_num *it) __weak __ksym;
>
> +struct bpf_iter_task_vma;
> +
> +extern int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it,
> + struct task_struct *task,
> + unsigned long addr) __weak __ksym;
> +extern struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it) __weak __ksym;
> +extern void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) __weak __ksym;
hm... still in bpf_helpers.h
Also, let's split this change out from kernel changes, there is no
need to couple these changes with kernel patches.
> +
> #ifndef bpf_for_each
> /* bpf_for_each(iter_type, cur_elem, args...) provides generic construct for
> * using BPF open-coded iterators without having to write mundane explicit
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> index 1f02168103dd..41aba139b20b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> @@ -10,7 +10,7 @@
> #include "bpf_iter_task.skel.h"
> #include "bpf_iter_task_stack.skel.h"
> #include "bpf_iter_task_file.skel.h"
> -#include "bpf_iter_task_vma.skel.h"
> +#include "bpf_iter_task_vmas.skel.h"
> #include "bpf_iter_task_btf.skel.h"
> #include "bpf_iter_tcp4.skel.h"
> #include "bpf_iter_tcp6.skel.h"
> @@ -1399,19 +1399,19 @@ static void str_strip_first_line(char *str)
> static void test_task_vma_common(struct bpf_iter_attach_opts *opts)
> {
> int err, iter_fd = -1, proc_maps_fd = -1;
> - struct bpf_iter_task_vma *skel;
> + struct bpf_iter_task_vmas *skel;
> int len, read_size = 4;
> char maps_path[64];
>
> - skel = bpf_iter_task_vma__open();
> - if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vma__open"))
> + skel = bpf_iter_task_vmas__open();
> + if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vmas__open"))
> return;
>
> skel->bss->pid = getpid();
> skel->bss->one_task = opts ? 1 : 0;
>
> - err = bpf_iter_task_vma__load(skel);
> - if (!ASSERT_OK(err, "bpf_iter_task_vma__load"))
> + err = bpf_iter_task_vmas__load(skel);
> + if (!ASSERT_OK(err, "bpf_iter_task_vmas__load"))
> goto out;
>
> skel->links.proc_maps = bpf_program__attach_iter(
> @@ -1462,25 +1462,25 @@ static void test_task_vma_common(struct bpf_iter_attach_opts *opts)
> out:
> close(proc_maps_fd);
> close(iter_fd);
> - bpf_iter_task_vma__destroy(skel);
> + bpf_iter_task_vmas__destroy(skel);
> }
>
> static void test_task_vma_dead_task(void)
> {
> - struct bpf_iter_task_vma *skel;
> + struct bpf_iter_task_vmas *skel;
> int wstatus, child_pid = -1;
> time_t start_tm, cur_tm;
> int err, iter_fd = -1;
> int wait_sec = 3;
>
> - skel = bpf_iter_task_vma__open();
> - if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vma__open"))
> + skel = bpf_iter_task_vmas__open();
> + if (!ASSERT_OK_PTR(skel, "bpf_iter_task_vmas__open"))
> return;
>
> skel->bss->pid = getpid();
>
> - err = bpf_iter_task_vma__load(skel);
> - if (!ASSERT_OK(err, "bpf_iter_task_vma__load"))
> + err = bpf_iter_task_vmas__load(skel);
> + if (!ASSERT_OK(err, "bpf_iter_task_vmas__load"))
> goto out;
>
> skel->links.proc_maps = bpf_program__attach_iter(
> @@ -1533,7 +1533,7 @@ static void test_task_vma_dead_task(void)
> out:
> waitpid(child_pid, &wstatus, 0);
> close(iter_fd);
> - bpf_iter_task_vma__destroy(skel);
> + bpf_iter_task_vmas__destroy(skel);
> }
>
> void test_bpf_sockmap_map_iter_fd(void)
> diff --git a/tools/testing/selftests/bpf/progs/bpf_iter_task_vma.c b/tools/testing/selftests/bpf/progs/bpf_iter_task_vmas.c
> similarity index 100%
> rename from tools/testing/selftests/bpf/progs/bpf_iter_task_vma.c
> rename to tools/testing/selftests/bpf/progs/bpf_iter_task_vmas.c
let's do this in a separate pre-patch?
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4 bpf-next 3/3] selftests/bpf: Add tests for open-coded task_vma iter
2023-10-02 19:53 ` [PATCH v4 bpf-next 3/3] selftests/bpf: Add tests for open-coded task_vma iter Dave Marchevsky
@ 2023-10-03 22:46 ` Andrii Nakryiko
0 siblings, 0 replies; 8+ messages in thread
From: Andrii Nakryiko @ 2023-10-03 22:46 UTC (permalink / raw)
To: Dave Marchevsky
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Kernel Team
On Mon, Oct 2, 2023 at 12:53 PM Dave Marchevsky <davemarchevsky@fb.com> wrote:
>
> The open-coded task_vma iter added earlier in this series allows for
> natural iteration over a task's vmas using existing open-coded iter
> infrastructure, specifically bpf_for_each.
>
> This patch adds a test demonstrating this pattern and validating
> correctness. The vma->vm_start and vma->vm_end addresses of the first
> 1000 vmas are recorded and compared to /proc/PID/maps output. As
> expected, both see the same vmas and addresses - with the exception of
> the [vsyscall] vma - which is explained in a comment in the prog_tests
> program.
>
> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com>
> ---
> .../testing/selftests/bpf/prog_tests/iters.c | 71 +++++++++++++++++++
> .../selftests/bpf/progs/iters_task_vma.c | 63 ++++++++++++++++
> 2 files changed, 134 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/iters_task_vma.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/iters.c b/tools/testing/selftests/bpf/prog_tests/iters.c
> index 10804ae5ae97..7a18fc21f364 100644
> --- a/tools/testing/selftests/bpf/prog_tests/iters.c
> +++ b/tools/testing/selftests/bpf/prog_tests/iters.c
> @@ -8,6 +8,7 @@
> #include "iters_looping.skel.h"
> #include "iters_num.skel.h"
> #include "iters_testmod_seq.skel.h"
> +#include "iters_task_vma.skel.h"
>
> static void subtest_num_iters(void)
> {
> @@ -90,6 +91,74 @@ static void subtest_testmod_seq_iters(void)
> iters_testmod_seq__destroy(skel);
> }
>
> +static void subtest_task_vma_iters(void)
> +{
> + unsigned long start, end, bpf_iter_start, bpf_iter_end;
> + struct iters_task_vma *skel;
> + char rest_of_line[1000];
> + unsigned int seen;
> + int err;
> + FILE *f;
> +
> + skel = iters_task_vma__open();
> + if (!ASSERT_OK_PTR(skel, "skel_open"))
> + return;
> +
> + bpf_program__set_autoload(skel->progs.iter_task_vma_for_each, true);
> +
no need, just drop ? from SEC(). Then you can do shorter
iters_task_vma__open_and_load() in one step.
> + err = iters_task_vma__load(skel);
> + if (!ASSERT_OK(err, "skel_load"))
> + goto cleanup;
> +
> + skel->bss->target_pid = getpid();
> +
> + err = iters_task_vma__attach(skel);
> + if (!ASSERT_OK(err, "skel_attach"))
> + goto cleanup;
> +
> + getpgid(skel->bss->target_pid);
> + iters_task_vma__detach(skel);
> +
> + if (!ASSERT_GT(skel->bss->vmas_seen, 0, "vmas_seen_gt_zero"))
> + goto cleanup;
> +
> + f = fopen("/proc/self/maps", "r");
> + if (!ASSERT_OK_PTR(f, "proc_maps_fopen"))
> + goto cleanup;
> +
> + seen = 0;
> + while (fscanf(f, "%lx-%lx %[^\n]\n", &start, &end, rest_of_line) == 3) {
> + /* [vsyscall] vma isn't _really_ part of task->mm vmas.
> + * /proc/PID/maps returns it when out of vmas - see get_gate_vma
> + * calls in fs/proc/task_mmu.c
> + */
> + if (strstr(rest_of_line, "[vsyscall]"))
> + continue;
> +
> + err = bpf_map_lookup_elem(bpf_map__fd(skel->maps.vm_start),
> + &seen, &bpf_iter_start);
> + if (!ASSERT_OK(err, "vm_start map_lookup_elem"))
> + goto cleanup;
> +
> + err = bpf_map_lookup_elem(bpf_map__fd(skel->maps.vm_end),
> + &seen, &bpf_iter_end);
> + if (!ASSERT_OK(err, "vm_end map_lookup_elem"))
> + goto cleanup;
> +
> + ASSERT_EQ(bpf_iter_start, start, "vma->vm_start match");
> + ASSERT_EQ(bpf_iter_end, end, "vma->vm_end match");
> + seen++;
> + }
> +
> + fclose(f);
move this into cleanup region (with NULL check, of course), some code
paths won't close the file
> +
> + if (!ASSERT_EQ(skel->bss->vmas_seen, seen, "vmas_seen_eq"))
> + goto cleanup;
> +
> +cleanup:
> + iters_task_vma__destroy(skel);
> +}
> +
> void test_iters(void)
> {
> RUN_TESTS(iters_state_safety);
> @@ -103,4 +172,6 @@ void test_iters(void)
> subtest_num_iters();
> if (test__start_subtest("testmod_seq"))
> subtest_testmod_seq_iters();
> + if (test__start_subtest("task_vma"))
> + subtest_task_vma_iters();
> }
> diff --git a/tools/testing/selftests/bpf/progs/iters_task_vma.c b/tools/testing/selftests/bpf/progs/iters_task_vma.c
> new file mode 100644
> index 000000000000..1ac7ca0633be
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/iters_task_vma.c
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
> +
> +#include <limits.h>
> +#include <linux/errno.h>
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +pid_t target_pid = 0;
> +unsigned int vmas_seen = 0;
> +
> +struct {
> + __uint(type, BPF_MAP_TYPE_ARRAY);
> + __uint(max_entries, 1000);
> + __type(key, int);
> + __type(value, unsigned long);
> +} vm_start SEC(".maps");
> +
> +struct {
> + __uint(type, BPF_MAP_TYPE_ARRAY);
> + __uint(max_entries, 1000);
> + __type(key, int);
> + __type(value, unsigned long);
> +} vm_end SEC(".maps");
Very minor point, but just in case you didn't think about it.
This is old school way, I'd just do:
struct { __u64 vm_start, vm_end; } vm_ranges[1000];
It would simplify user space part a bit as well
> +
> +SEC("?raw_tp/sys_enter")
> +int iter_task_vma_for_each(const void *ctx)
> +{
> + struct task_struct *task = bpf_get_current_task_btf();
> + struct vm_area_struct *vma;
> + unsigned long *start, *end;
> + unsigned int seen = 0;
> +
> + if (task->pid != target_pid)
> + return 0;
> +
> + if (vmas_seen)
> + return 0;
> +
> + bpf_for_each(task_vma, vma, task, 0) {
> + if (seen >= 1000)
> + break;
> +
> + start = bpf_map_lookup_elem(&vm_start, &seen);
> + if (!start)
> + break;
> + *start = vma->vm_start;
> +
> + end = bpf_map_lookup_elem(&vm_end, &seen);
> + if (!end)
> + break;
> + *end = vma->vm_end;
> +
> + seen++;
> + }
> +
> + if (!vmas_seen)
> + vmas_seen = seen;
> + return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-10-03 22:46 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-02 19:53 [PATCH v4 bpf-next 0/3] Open-coded task_vma iter Dave Marchevsky
2023-10-02 19:53 ` [PATCH v4 bpf-next 1/3] bpf: Don't explicitly emit BTF for struct btf_iter_num Dave Marchevsky
2023-10-02 19:53 ` [PATCH v4 bpf-next 2/3] bpf: Introduce task_vma open-coded iterator kfuncs Dave Marchevsky
2023-10-02 21:29 ` kernel test robot
2023-10-03 16:28 ` kernel test robot
2023-10-03 22:39 ` Andrii Nakryiko
2023-10-02 19:53 ` [PATCH v4 bpf-next 3/3] selftests/bpf: Add tests for open-coded task_vma iter Dave Marchevsky
2023-10-03 22:46 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox