On 11/4/21 10:17 AM, Song Liu wrote:

quoted

On Nov 4, 2021, at 10:07 AM, Yonghong Song [off-list ref] wrote:



On 11/4/21 12:00 AM, Song Liu wrote:

quoted

Add tests for bpf_find_vma in perf_event program and kprobe program. The
perf_event program is triggered from NMI context, so the second call of
bpf_find_vma() will return -EBUSY (irq_work busy). The kprobe program,
on the other hand, does not have this constraint.
Also add test for illegal writes to task or vma from the callback
function. The verifier should reject both cases.
Signed-off-by: Song Liu <redacted>

[...]

quoted

quoted

+static void test_find_vma_pe(struct find_vma *skel)
+{
+	struct bpf_link *link = NULL;
+	volatile int j = 0;
+	int pfd = -1, i;
+
+	pfd = open_pe();
+	if (pfd < 0) {
+		if (pfd == -ENOENT || pfd == -EOPNOTSUPP) {
+			printf("%s:SKIP:no PERF_COUNT_HW_CPU_CYCLES\n", __func__);
+			test__skip();
+		}
+		if (!ASSERT_GE(pfd, 0, "perf_event_open"))
+			goto cleanup;
+	}
+
+	link = bpf_program__attach_perf_event(skel->progs.handle_pe, pfd);
+	if (!ASSERT_OK_PTR(link, "attach_perf_event"))
+		goto cleanup;
+
+	for (i = 0; i < 1000000; ++i)
+		++j;

Does this really work? Compiler could do
  j += 1000000;

I think compiler won't do it with volatile j?

Ya. volatile j should be fine.

quoted

quoted

+
+	test_and_reset_skel(skel, -EBUSY /* in nmi, irq_work is busy */);
+cleanup:
+	bpf_link__destroy(link);
+	close(pfd);
+	/* caller will clean up skel */

Above comment is not needed. It should be clear from the code.

[...]

quoted

quoted

+int handle_getpid(void)
+{
+	struct task_struct *task = bpf_get_current_task_btf();
+	struct callback_ctx data = {0};
+
+	if (task->pid != target_pid)
+		return 0;
+
+	find_addr_ret = bpf_find_vma(task, addr, check_vma, &data, 0);
+
+	/* this should return -ENOENT */
+	find_zero_ret = bpf_find_vma(task, 0, check_vma, &data, 0);
+	return 0;
+}
+
+SEC("perf_event")
+int handle_pe(void)
+{
+	struct task_struct *task = bpf_get_current_task_btf();
+	struct callback_ctx data = {0};
+
+	if (task->pid != target_pid)
+		return 0;

This is tricky. How do we guarantee task->pid == target_pid hit?
This probably mostly okay in serial running mode. But it may
become more challenging if test_progs is running in parallel mode?

This is on a per task perf_event, so it shouldn't hit other tasks.

I see. we have the following parameters for perf_event open.

        pid == 0 and cpu == -1
               This measures the calling process/thread on any CPU.

So yes, we are fine then.

quoted

quoted

+
+	find_addr_ret = bpf_find_vma(task, addr, check_vma, &data, 0);
+
+	/* In NMI, this should return -EBUSY, as the previous call is using
+	 * the irq_work.
+	 */
+	find_zero_ret = bpf_find_vma(task, 0, check_vma, &data, 0);
+	return 0;
+}

[...]