Make sure that returning a struct nf_conn * reference invokes the reference tracking machinery in the verifier.
Signed-off-by: Matthew Cover matthew.cover@stackpath.com --- tools/testing/selftests/bpf/test_verifier.c | 18 ++++++++ .../testing/selftests/bpf/verifier/ref_tracking.c | 48 ++++++++++++++++++++++ 2 files changed, 66 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c index 87eaa49..7569db2 100644 --- a/tools/testing/selftests/bpf/test_verifier.c +++ b/tools/testing/selftests/bpf/test_verifier.c @@ -294,6 +294,24 @@ static void bpf_fill_scale(struct bpf_test *self) } }
+/* BPF_CT_LOOKUP contains 13 instructions, if you need to fix up maps */ +#define BPF_CT_LOOKUP(func) \ + /* struct bpf_nf_conntrack_tuple tuple = {} */ \ + BPF_MOV64_IMM(BPF_REG_2, 0), \ + BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_2, -8), \ + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -16), \ + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -24), \ + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -32), \ + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -40), \ + BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_2, -48), \ + /* ct = func(ctx, &tuple, sizeof tuple, 0, 0) */ \ + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10), \ + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -48), \ + BPF_MOV64_IMM(BPF_REG_3, sizeof(struct bpf_nf_conntrack_tuple)),\ + BPF_MOV64_IMM(BPF_REG_4, 0), \ + BPF_MOV64_IMM(BPF_REG_5, 0), \ + BPF_EMIT_CALL(BPF_FUNC_ ## func) + /* BPF_SK_LOOKUP contains 13 instructions, if you need to fix up maps */ #define BPF_SK_LOOKUP(func) \ /* struct bpf_sock_tuple tuple = {} */ \ diff --git a/tools/testing/selftests/bpf/verifier/ref_tracking.c b/tools/testing/selftests/bpf/verifier/ref_tracking.c index 604b461..de5c550a 100644 --- a/tools/testing/selftests/bpf/verifier/ref_tracking.c +++ b/tools/testing/selftests/bpf/verifier/ref_tracking.c @@ -21,6 +21,17 @@ .result = REJECT, }, { + "reference tracking: leak potential reference to nf_conn", + .insns = { + BPF_CT_LOOKUP(ct_lookup_tcp), + BPF_MOV64_REG(BPF_REG_6, BPF_REG_0), /* leak reference */ + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_SCHED_CLS, + .errstr = "Unreleased reference", + .result = REJECT, +}, +{ "reference tracking: leak potential reference on stack", .insns = { BPF_SK_LOOKUP(sk_lookup_tcp), @@ -72,6 +83,17 @@ .result = REJECT, }, { + "reference tracking: zero potential reference to nf_conn", + .insns = { + BPF_CT_LOOKUP(ct_lookup_tcp), + BPF_MOV64_IMM(BPF_REG_0, 0), /* leak reference */ + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_SCHED_CLS, + .errstr = "Unreleased reference", + .result = REJECT, +}, +{ "reference tracking: copy and zero potential references", .insns = { BPF_SK_LOOKUP(sk_lookup_tcp), @@ -113,6 +135,20 @@ .result = REJECT, }, { + "reference tracking: release reference to nf_conn without check", + .insns = { + BPF_CT_LOOKUP(ct_lookup_tcp), + /* reference in r0 may be NULL */ + BPF_MOV64_REG(BPF_REG_1, BPF_REG_0), + BPF_MOV64_IMM(BPF_REG_2, 0), + BPF_EMIT_CALL(BPF_FUNC_ct_release), + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_SCHED_CLS, + .errstr = "type=nf_conn_or_null expected=nf_conn", + .result = REJECT, +}, +{ "reference tracking: release reference", .insns = { BPF_SK_LOOKUP(sk_lookup_tcp), @@ -137,6 +173,18 @@ .result = ACCEPT, }, { + "reference tracking: release reference to nf_conn", + .insns = { + BPF_CT_LOOKUP(ct_lookup_tcp), + BPF_MOV64_REG(BPF_REG_1, BPF_REG_0), + BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1), + BPF_EMIT_CALL(BPF_FUNC_ct_release), + BPF_EXIT_INSN(), + }, + .prog_type = BPF_PROG_TYPE_SCHED_CLS, + .result = ACCEPT, +}, +{ "reference tracking: release reference 2", .insns = { BPF_SK_LOOKUP(sk_lookup_tcp),