On 23-Dez 22:49, Andrii Nakryiko wrote:

On Fri, Dec 20, 2019 at 7:42 AM KP Singh [off-list ref] wrote:

quoted

From: KP Singh <redacted>

* Load a BPF program that audits mprotect calls
* Attach the program to the "file_mprotect" LSM hook
* Verify if the program is actually loading by reading
  securityfs
* Initialize the perf events buffer and poll for audit events
* Do an mprotect on some memory allocated on the heap
* Verify if the audit event was received

Signed-off-by: KP Singh <redacted>
---
 MAINTAINERS                                   |   2 +
 .../bpf/prog_tests/lsm_mprotect_audit.c       | 129 ++++++++++++++++++
 .../selftests/bpf/progs/lsm_mprotect_audit.c  |  58 ++++++++
 3 files changed, 189 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/lsm_mprotect_audit.c
 create mode 100644 tools/testing/selftests/bpf/progs/lsm_mprotect_audit.c

[...]

quoted

+/*
+ * Define some of the structs used in the BPF program.
+ * Only the field names and their sizes need to be the
+ * same as the kernel type, the order is irrelevant.
+ */
+struct mm_struct {
+       unsigned long start_brk, brk, start_stack;
+};
+
+struct vm_area_struct {
+       unsigned long start_brk, brk, start_stack;
+       unsigned long vm_start, vm_end;
+       struct mm_struct *vm_mm;
+       unsigned long vm_flags;
+};
+
+BPF_TRACE_3("lsm/file_mprotect", mprotect_audit,
+           struct vm_area_struct *, vma,
+           unsigned long, reqprot, unsigned long, prot)
+{
+       struct mprotect_audit_log audit_log = {};
+       int is_heap = 0;
+
+       __builtin_preserve_access_index(({

you don't need __builtin_preserve_access_index, if you mark
vm_area_struct and mm_struct with
__attribute__((preserve_access_index)

Cool, updated!

quoted

+               is_heap = (vma->vm_start >= vma->vm_mm->start_brk &&
+                                    vma->vm_end <= vma->vm_mm->brk);
+       }));
+
+       audit_log.magic = MPROTECT_AUDIT_MAGIC;
+       audit_log.is_heap = is_heap;
+       bpf_lsm_event_output(&perf_buf_map, BPF_F_CURRENT_CPU, &audit_log,
+                            sizeof(audit_log));

You test would be much simpler if you use global variables to pass
data back to userspace, instead of using perf buffer.

Also please see fentry_fexit.c test for example of using BPF skeleton
to shorten and simpify userspace part of test.

Thanks for the skeleton work!

This makes using global variables easier and the tests are indeed much
simpler, I have updated it for the next revision.

One follow up question regarding global variables, let's say I have
the following global variable defined in the BPF program:

struct result_info {
        __u32 count;
}; 

struct result_info result = {
        .count = 0,
};

The defintion of result_info needs to be included before the .skel.h
as it's not automatically generated or maybe I am missing a
trick here?

For now, I have defined this in a header which gets included both in
the program and the test.

- KP

quoted

+       return 0;
+}
--
2.20.1