bpf: introduce ARG_PTR_TO_MEM_OR_NULL

With the current ARG_PTR_TO_MEM/ARG_PTR_TO_UNINIT_MEM semantics, an helper
argument can be NULL when the next argument type is ARG_CONST_SIZE_OR_ZERO
and the verifier can prove the value of this next argument is 0. However,
most helpers are just interested in handling <!NULL, 0>, so forcing them to
deal with <NULL, 0> makes the implementation of those helpers more
complicated for no apparent benefits, requiring them to explicitly handle
those corner cases with checks that bpf programs could start relying upon,
preventing the possibility of removing them later.

Solve this by making ARG_PTR_TO_MEM/ARG_PTR_TO_UNINIT_MEM never accept NULL
even when ARG_CONST_SIZE_OR_ZERO is set, and introduce a new argument type
ARG_PTR_TO_MEM_OR_NULL to explicitly deal with the NULL case.

Currently, the only helper that needs this is bpf_csum_diff_proto(), so
change arg1 and arg3 to this new type as well.

Also add a new battery of tests that explicitly test the
!ARG_PTR_TO_MEM_OR_NULL combination: all the current ones testing the
various <NULL, 0> variations are focused on bpf_csum_diff, so cover also
other helpers.

Signed-off-by: Gianluca Borello <g.borello@gmail.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 76c577281d78da19721b637bf4fd3b0d99b34270..e55e4255a21082325f0888ffcd464615add708b8 100644 (file)
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -78,6 +78,7 @@ enum bpf_arg_type {
         * functions that access data on eBPF program stack
         */
        ARG_PTR_TO_MEM,         /* pointer to valid memory (stack, packet, map value) */
+       ARG_PTR_TO_MEM_OR_NULL, /* pointer to valid memory or NULL */
        ARG_PTR_TO_UNINIT_MEM,  /* pointer to memory does not need to be initialized,
                                 * helper function must fill all bytes or clear
                                 * them in error case.

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index dd54d20ace2ff7bc3aa942f4a08e08dadfb1a4e6..308b0638ec5d58834e6352a4d67690796b7b61b7 100644 (file)
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1384,13 +1384,15 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
                if (type != expected_type)
                        goto err_type;
        } else if (arg_type == ARG_PTR_TO_MEM ||
+                  arg_type == ARG_PTR_TO_MEM_OR_NULL ||
                   arg_type == ARG_PTR_TO_UNINIT_MEM) {
                expected_type = PTR_TO_STACK;
                /* One exception here. In case function allows for NULL to be
                 * passed in as argument, it's a SCALAR_VALUE type. Final test
                 * happens during stack boundary checking.
                 */
-               if (register_is_null(*reg))
+               if (register_is_null(*reg) &&
+                   arg_type == ARG_PTR_TO_MEM_OR_NULL)
                        /* final test in check_stack_boundary() */;
                else if (!type_is_pkt_pointer(type) &&
                         type != PTR_TO_MAP_VALUE &&

diff --git a/net/core/filter.c b/net/core/filter.c
index 1afa17935954b71ff6ae30d84511a0023afa9cc8..6a85e67fafce224b534dd87bb9407ae115f8ba7a 100644 (file)
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1646,9 +1646,9 @@ static const struct bpf_func_proto bpf_csum_diff_proto = {
        .gpl_only       = false,
        .pkt_access     = true,
        .ret_type       = RET_INTEGER,
-       .arg1_type      = ARG_PTR_TO_MEM,
+       .arg1_type      = ARG_PTR_TO_MEM_OR_NULL,
        .arg2_type      = ARG_CONST_SIZE_OR_ZERO,
-       .arg3_type      = ARG_PTR_TO_MEM,
+       .arg3_type      = ARG_PTR_TO_MEM_OR_NULL,
        .arg4_type      = ARG_CONST_SIZE_OR_ZERO,
        .arg5_type      = ARG_ANYTHING,
 };

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 2a5267bef1607c18b3212d95e2db3f56f7801a61..3c64f30cf63cc2b6adb532a3b1f3201533193f7f 100644 (file)
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -5631,7 +5631,7 @@ static struct bpf_test tests[] = {
                .prog_type = BPF_PROG_TYPE_TRACEPOINT,
        },
        {
-               "helper access to variable memory: size = 0 allowed on NULL",
+               "helper access to variable memory: size = 0 allowed on NULL (ARG_PTR_TO_MEM_OR_NULL)",
                .insns = {
                        BPF_MOV64_IMM(BPF_REG_1, 0),
                        BPF_MOV64_IMM(BPF_REG_2, 0),
@@ -5645,7 +5645,7 @@ static struct bpf_test tests[] = {
                .prog_type = BPF_PROG_TYPE_SCHED_CLS,
        },
        {
-               "helper access to variable memory: size > 0 not allowed on NULL",
+               "helper access to variable memory: size > 0 not allowed on NULL (ARG_PTR_TO_MEM_OR_NULL)",
                .insns = {
                        BPF_MOV64_IMM(BPF_REG_1, 0),
                        BPF_MOV64_IMM(BPF_REG_2, 0),
@@ -5663,7 +5663,7 @@ static struct bpf_test tests[] = {
                .prog_type = BPF_PROG_TYPE_SCHED_CLS,
        },
        {
-               "helper access to variable memory: size = 0 allowed on != NULL stack pointer",
+               "helper access to variable memory: size = 0 allowed on != NULL stack pointer (ARG_PTR_TO_MEM_OR_NULL)",
                .insns = {
                        BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
                        BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
@@ -5680,7 +5680,7 @@ static struct bpf_test tests[] = {
                .prog_type = BPF_PROG_TYPE_SCHED_CLS,
        },
        {
-               "helper access to variable memory: size = 0 allowed on != NULL map pointer",
+               "helper access to variable memory: size = 0 allowed on != NULL map pointer (ARG_PTR_TO_MEM_OR_NULL)",
                .insns = {
                        BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
                        BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
@@ -5702,7 +5702,7 @@ static struct bpf_test tests[] = {
                .prog_type = BPF_PROG_TYPE_SCHED_CLS,
        },
        {
-               "helper access to variable memory: size possible = 0 allowed on != NULL stack pointer",
+               "helper access to variable memory: size possible = 0 allowed on != NULL stack pointer (ARG_PTR_TO_MEM_OR_NULL)",
                .insns = {
                        BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
                        BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
@@ -5727,7 +5727,7 @@ static struct bpf_test tests[] = {
                .prog_type = BPF_PROG_TYPE_SCHED_CLS,
        },
        {
-               "helper access to variable memory: size possible = 0 allowed on != NULL map pointer",
+               "helper access to variable memory: size possible = 0 allowed on != NULL map pointer (ARG_PTR_TO_MEM_OR_NULL)",
                .insns = {
                        BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
                        BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
@@ -5750,7 +5750,7 @@ static struct bpf_test tests[] = {
                .prog_type = BPF_PROG_TYPE_SCHED_CLS,
        },
        {
-               "helper access to variable memory: size possible = 0 allowed on != NULL packet pointer",
+               "helper access to variable memory: size possible = 0 allowed on != NULL packet pointer (ARG_PTR_TO_MEM_OR_NULL)",
                .insns = {
                        BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
                                    offsetof(struct __sk_buff, data)),
@@ -5771,6 +5771,105 @@ static struct bpf_test tests[] = {
                .result = ACCEPT,
                .prog_type = BPF_PROG_TYPE_SCHED_CLS,
        },
+       {
+               "helper access to variable memory: size = 0 not allowed on NULL (!ARG_PTR_TO_MEM_OR_NULL)",
+               .insns = {
+                       BPF_MOV64_IMM(BPF_REG_1, 0),
+                       BPF_MOV64_IMM(BPF_REG_2, 0),
+                       BPF_MOV64_IMM(BPF_REG_3, 0),
+                       BPF_EMIT_CALL(BPF_FUNC_probe_read),
+                       BPF_EXIT_INSN(),
+               },
+               .errstr = "R1 type=inv expected=fp",
+               .result = REJECT,
+               .prog_type = BPF_PROG_TYPE_TRACEPOINT,
+       },
+       {
+               "helper access to variable memory: size > 0 not allowed on NULL (!ARG_PTR_TO_MEM_OR_NULL)",
+               .insns = {
+                       BPF_MOV64_IMM(BPF_REG_1, 0),
+                       BPF_MOV64_IMM(BPF_REG_2, 1),
+                       BPF_MOV64_IMM(BPF_REG_3, 0),
+                       BPF_EMIT_CALL(BPF_FUNC_probe_read),
+                       BPF_EXIT_INSN(),
+               },
+               .errstr = "R1 type=inv expected=fp",
+               .result = REJECT,
+               .prog_type = BPF_PROG_TYPE_TRACEPOINT,
+       },
+       {
+               "helper access to variable memory: size = 0 allowed on != NULL stack pointer (!ARG_PTR_TO_MEM_OR_NULL)",
+               .insns = {
+                       BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+                       BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+                       BPF_MOV64_IMM(BPF_REG_2, 0),
+                       BPF_MOV64_IMM(BPF_REG_3, 0),
+                       BPF_EMIT_CALL(BPF_FUNC_probe_read),
+                       BPF_EXIT_INSN(),
+               },
+               .result = ACCEPT,
+               .prog_type = BPF_PROG_TYPE_TRACEPOINT,
+       },
+       {
+               "helper access to variable memory: size = 0 allowed on != NULL map pointer (!ARG_PTR_TO_MEM_OR_NULL)",
+               .insns = {
+                       BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+                       BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+                       BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+                       BPF_LD_MAP_FD(BPF_REG_1, 0),
+                       BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+                       BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
+                       BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+                       BPF_MOV64_IMM(BPF_REG_2, 0),
+                       BPF_MOV64_IMM(BPF_REG_3, 0),
+                       BPF_EMIT_CALL(BPF_FUNC_probe_read),
+                       BPF_EXIT_INSN(),
+               },
+               .fixup_map1 = { 3 },
+               .result = ACCEPT,
+               .prog_type = BPF_PROG_TYPE_TRACEPOINT,
+       },
+       {
+               "helper access to variable memory: size possible = 0 allowed on != NULL stack pointer (!ARG_PTR_TO_MEM_OR_NULL)",
+               .insns = {
+                       BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+                       BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+                       BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+                       BPF_LD_MAP_FD(BPF_REG_1, 0),
+                       BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+                       BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 6),
+                       BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_0, 0),
+                       BPF_JMP_IMM(BPF_JGT, BPF_REG_2, 8, 4),
+                       BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+                       BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+                       BPF_MOV64_IMM(BPF_REG_3, 0),
+                       BPF_EMIT_CALL(BPF_FUNC_probe_read),
+                       BPF_EXIT_INSN(),
+               },
+               .fixup_map1 = { 3 },
+               .result = ACCEPT,
+               .prog_type = BPF_PROG_TYPE_TRACEPOINT,
+       },
+       {
+               "helper access to variable memory: size possible = 0 allowed on != NULL map pointer (!ARG_PTR_TO_MEM_OR_NULL)",
+               .insns = {
+                       BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+                       BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+                       BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+                       BPF_LD_MAP_FD(BPF_REG_1, 0),
+                       BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
+                       BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 5),
+                       BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
+                       BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_0, 0),
+                       BPF_JMP_IMM(BPF_JGT, BPF_REG_2, 8, 2),
+                       BPF_MOV64_IMM(BPF_REG_3, 0),
+                       BPF_EMIT_CALL(BPF_FUNC_probe_read),
+                       BPF_EXIT_INSN(),
+               },
+               .fixup_map1 = { 3 },
+               .result = ACCEPT,
+               .prog_type = BPF_PROG_TYPE_TRACEPOINT,
+       },
        {
                "helper access to variable memory: 8 bytes leak",
                .insns = {