On Mon, May 9, 2022 at 4:44 PM Andrii Nakryiko
[off-list ref] wrote:

On Mon, May 9, 2022 at 4:38 PM Stanislav Fomichev [off-list ref] wrote:

quoted

On Mon, May 9, 2022 at 2:54 PM Andrii Nakryiko
[off-list ref] wrote:

quoted

On Fri, Apr 29, 2022 at 2:16 PM Stanislav Fomichev [off-list ref] wrote:

quoted

sk_priority & sk_mark are writable, the rest is readonly.

Add new ldx_offset fixups to lookup the offset of struct field.
Allow using test.kfunc regardless of prog_type.

One interesting thing here is that the verifier doesn't
really force me to add NULL checks anywhere :-/

Signed-off-by: Stanislav Fomichev <redacted>
---
 tools/testing/selftests/bpf/test_verifier.c   | 54 ++++++++++++++++++-
 .../selftests/bpf/verifier/lsm_cgroup.c       | 34 ++++++++++++
 2 files changed, 87 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/verifier/lsm_cgroup.c

[...]

quoted

diff --git a/tools/testing/selftests/bpf/verifier/lsm_cgroup.c b/tools/testing/selftests/bpf/verifier/lsm_cgroup.c
new file mode 100644
index 000000000000..af0efe783511
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/lsm_cgroup.c

@@ -0,0 +1,34 @@
+#define SK_WRITABLE_FIELD(tp, field, size, res) \
+{ \
+       .descr = field, \
+       .insns = { \
+               /* r1 = *(u64 *)(r1 + 0) */ \
+               BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, 0), \
+               /* r1 = *(u64 *)(r1 + offsetof(struct socket, sk)) */ \
+               BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, 0), \
+               /* r2 = *(u64 *)(r1 + offsetof(struct sock, <field>)) */ \
+               BPF_LDX_MEM(size, BPF_REG_2, BPF_REG_1, 0), \
+               /* *(u64 *)(r1 + offsetof(struct sock, <field>)) = r2 */ \
+               BPF_STX_MEM(size, BPF_REG_1, BPF_REG_2, 0), \
+               BPF_MOV64_IMM(BPF_REG_0, 1), \
+               BPF_EXIT_INSN(), \
+       }, \
+       .result = res, \
+       .errstr = res ? "no write support to 'struct sock' at off" : "", \
+       .prog_type = BPF_PROG_TYPE_LSM, \
+       .expected_attach_type = BPF_LSM_CGROUP, \
+       .kfunc = "socket_post_create", \
+       .fixup_ldx = { \
+               { "socket", "sk", 1 }, \
+               { tp, field, 2 }, \
+               { tp, field, 3 }, \
+       }, \
+}
+
+SK_WRITABLE_FIELD("sock_common", "skc_family", BPF_H, REJECT),
+SK_WRITABLE_FIELD("sock", "sk_sndtimeo", BPF_DW, REJECT),
+SK_WRITABLE_FIELD("sock", "sk_priority", BPF_W, ACCEPT),
+SK_WRITABLE_FIELD("sock", "sk_mark", BPF_W, ACCEPT),
+SK_WRITABLE_FIELD("sock", "sk_pacing_rate", BPF_DW, REJECT),
+

have you tried writing it as C program and adding the test to
test_progs? Does something not work there?

Seems like it should work, I don't see any issues with writing 5
programs to test each field.
But test_verified still feels like a better fit? Any reason in
particular you'd prefer test_progs over test_verifier?

Adding that fixup_ldx->strct special handling didn't feel like the
best fit, tbh. test_progs is generally much nicer to deal with in
terms of CI and in terms of comprehending what's going on and
supporting the code longer term.

This is not new, right? We already have a bunch of fixup_xxx things.
I can try to move this into test_progs in largely the same manner if
you prefer, having a C file per field seems like an overkill.