Skip to content

Commit db1ac49

Browse files
gianlucaborelloborkmann
gianlucaborello
authored and
borkmann
committed
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>
1 parent f1a8b8e commit db1ac49

File tree

4 files changed

+112
-10
lines changed

4 files changed

+112
-10
lines changed

‎include/linux/bpf.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ enum bpf_arg_type {
7878
* functions that access data on eBPF program stack
7979
*/
8080
ARG_PTR_TO_MEM, /* pointer to valid memory (stack, packet, map value) */
81+
ARG_PTR_TO_MEM_OR_NULL, /* pointer to valid memory or NULL */
8182
ARG_PTR_TO_UNINIT_MEM, /* pointer to memory does not need to be initialized,
8283
* helper function must fill all bytes or clear
8384
* them in error case.

‎kernel/bpf/verifier.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1384,13 +1384,15 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
13841384
if (type != expected_type)
13851385
goto err_type;
13861386
} else if (arg_type == ARG_PTR_TO_MEM ||
1387+
arg_type == ARG_PTR_TO_MEM_OR_NULL ||
13871388
arg_type == ARG_PTR_TO_UNINIT_MEM) {
13881389
expected_type = PTR_TO_STACK;
13891390
/* One exception here. In case function allows for NULL to be
13901391
* passed in as argument, it's a SCALAR_VALUE type. Final test
13911392
* happens during stack boundary checking.
13921393
*/
1393-
if (register_is_null(*reg))
1394+
if (register_is_null(*reg) &&
1395+
arg_type == ARG_PTR_TO_MEM_OR_NULL)
13941396
/* final test in check_stack_boundary() */;
13951397
else if (!type_is_pkt_pointer(type) &&
13961398
type != PTR_TO_MAP_VALUE &&

‎net/core/filter.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1646,9 +1646,9 @@ static const struct bpf_func_proto bpf_csum_diff_proto = {
16461646
.gpl_only = false,
16471647
.pkt_access = true,
16481648
.ret_type = RET_INTEGER,
1649-
.arg1_type = ARG_PTR_TO_MEM,
1649+
.arg1_type = ARG_PTR_TO_MEM_OR_NULL,
16501650
.arg2_type = ARG_CONST_SIZE_OR_ZERO,
1651-
.arg3_type = ARG_PTR_TO_MEM,
1651+
.arg3_type = ARG_PTR_TO_MEM_OR_NULL,
16521652
.arg4_type = ARG_CONST_SIZE_OR_ZERO,
16531653
.arg5_type = ARG_ANYTHING,
16541654
};

‎tools/testing/selftests/bpf/test_verifier.c

Lines changed: 106 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5631,7 +5631,7 @@ static struct bpf_test tests[] = {
56315631
.prog_type = BPF_PROG_TYPE_TRACEPOINT,
56325632
},
56335633
{
5634-
"helper access to variable memory: size = 0 allowed on NULL",
5634+
"helper access to variable memory: size = 0 allowed on NULL (ARG_PTR_TO_MEM_OR_NULL)",
56355635
.insns = {
56365636
BPF_MOV64_IMM(BPF_REG_1, 0),
56375637
BPF_MOV64_IMM(BPF_REG_2, 0),
@@ -5645,7 +5645,7 @@ static struct bpf_test tests[] = {
56455645
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
56465646
},
56475647
{
5648-
"helper access to variable memory: size > 0 not allowed on NULL",
5648+
"helper access to variable memory: size > 0 not allowed on NULL (ARG_PTR_TO_MEM_OR_NULL)",
56495649
.insns = {
56505650
BPF_MOV64_IMM(BPF_REG_1, 0),
56515651
BPF_MOV64_IMM(BPF_REG_2, 0),
@@ -5663,7 +5663,7 @@ static struct bpf_test tests[] = {
56635663
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
56645664
},
56655665
{
5666-
"helper access to variable memory: size = 0 allowed on != NULL stack pointer",
5666+
"helper access to variable memory: size = 0 allowed on != NULL stack pointer (ARG_PTR_TO_MEM_OR_NULL)",
56675667
.insns = {
56685668
BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
56695669
BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
@@ -5680,7 +5680,7 @@ static struct bpf_test tests[] = {
56805680
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
56815681
},
56825682
{
5683-
"helper access to variable memory: size = 0 allowed on != NULL map pointer",
5683+
"helper access to variable memory: size = 0 allowed on != NULL map pointer (ARG_PTR_TO_MEM_OR_NULL)",
56845684
.insns = {
56855685
BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
56865686
BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
@@ -5702,7 +5702,7 @@ static struct bpf_test tests[] = {
57025702
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
57035703
},
57045704
{
5705-
"helper access to variable memory: size possible = 0 allowed on != NULL stack pointer",
5705+
"helper access to variable memory: size possible = 0 allowed on != NULL stack pointer (ARG_PTR_TO_MEM_OR_NULL)",
57065706
.insns = {
57075707
BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
57085708
BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
@@ -5727,7 +5727,7 @@ static struct bpf_test tests[] = {
57275727
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
57285728
},
57295729
{
5730-
"helper access to variable memory: size possible = 0 allowed on != NULL map pointer",
5730+
"helper access to variable memory: size possible = 0 allowed on != NULL map pointer (ARG_PTR_TO_MEM_OR_NULL)",
57315731
.insns = {
57325732
BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
57335733
BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
@@ -5750,7 +5750,7 @@ static struct bpf_test tests[] = {
57505750
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
57515751
},
57525752
{
5753-
"helper access to variable memory: size possible = 0 allowed on != NULL packet pointer",
5753+
"helper access to variable memory: size possible = 0 allowed on != NULL packet pointer (ARG_PTR_TO_MEM_OR_NULL)",
57545754
.insns = {
57555755
BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
57565756
offsetof(struct __sk_buff, data)),
@@ -5771,6 +5771,105 @@ static struct bpf_test tests[] = {
57715771
.result = ACCEPT,
57725772
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
57735773
},
5774+
{
5775+
"helper access to variable memory: size = 0 not allowed on NULL (!ARG_PTR_TO_MEM_OR_NULL)",
5776+
.insns = {
5777+
BPF_MOV64_IMM(BPF_REG_1, 0),
5778+
BPF_MOV64_IMM(BPF_REG_2, 0),
5779+
BPF_MOV64_IMM(BPF_REG_3, 0),
5780+
BPF_EMIT_CALL(BPF_FUNC_probe_read),
5781+
BPF_EXIT_INSN(),
5782+
},
5783+
.errstr = "R1 type=inv expected=fp",
5784+
.result = REJECT,
5785+
.prog_type = BPF_PROG_TYPE_TRACEPOINT,
5786+
},
5787+
{
5788+
"helper access to variable memory: size > 0 not allowed on NULL (!ARG_PTR_TO_MEM_OR_NULL)",
5789+
.insns = {
5790+
BPF_MOV64_IMM(BPF_REG_1, 0),
5791+
BPF_MOV64_IMM(BPF_REG_2, 1),
5792+
BPF_MOV64_IMM(BPF_REG_3, 0),
5793+
BPF_EMIT_CALL(BPF_FUNC_probe_read),
5794+
BPF_EXIT_INSN(),
5795+
},
5796+
.errstr = "R1 type=inv expected=fp",
5797+
.result = REJECT,
5798+
.prog_type = BPF_PROG_TYPE_TRACEPOINT,
5799+
},
5800+
{
5801+
"helper access to variable memory: size = 0 allowed on != NULL stack pointer (!ARG_PTR_TO_MEM_OR_NULL)",
5802+
.insns = {
5803+
BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
5804+
BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
5805+
BPF_MOV64_IMM(BPF_REG_2, 0),
5806+
BPF_MOV64_IMM(BPF_REG_3, 0),
5807+
BPF_EMIT_CALL(BPF_FUNC_probe_read),
5808+
BPF_EXIT_INSN(),
5809+
},
5810+
.result = ACCEPT,
5811+
.prog_type = BPF_PROG_TYPE_TRACEPOINT,
5812+
},
5813+
{
5814+
"helper access to variable memory: size = 0 allowed on != NULL map pointer (!ARG_PTR_TO_MEM_OR_NULL)",
5815+
.insns = {
5816+
BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
5817+
BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
5818+
BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
5819+
BPF_LD_MAP_FD(BPF_REG_1, 0),
5820+
BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
5821+
BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 4),
5822+
BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
5823+
BPF_MOV64_IMM(BPF_REG_2, 0),
5824+
BPF_MOV64_IMM(BPF_REG_3, 0),
5825+
BPF_EMIT_CALL(BPF_FUNC_probe_read),
5826+
BPF_EXIT_INSN(),
5827+
},
5828+
.fixup_map1 = { 3 },
5829+
.result = ACCEPT,
5830+
.prog_type = BPF_PROG_TYPE_TRACEPOINT,
5831+
},
5832+
{
5833+
"helper access to variable memory: size possible = 0 allowed on != NULL stack pointer (!ARG_PTR_TO_MEM_OR_NULL)",
5834+
.insns = {
5835+
BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
5836+
BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
5837+
BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
5838+
BPF_LD_MAP_FD(BPF_REG_1, 0),
5839+
BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
5840+
BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 6),
5841+
BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_0, 0),
5842+
BPF_JMP_IMM(BPF_JGT, BPF_REG_2, 8, 4),
5843+
BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
5844+
BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
5845+
BPF_MOV64_IMM(BPF_REG_3, 0),
5846+
BPF_EMIT_CALL(BPF_FUNC_probe_read),
5847+
BPF_EXIT_INSN(),
5848+
},
5849+
.fixup_map1 = { 3 },
5850+
.result = ACCEPT,
5851+
.prog_type = BPF_PROG_TYPE_TRACEPOINT,
5852+
},
5853+
{
5854+
"helper access to variable memory: size possible = 0 allowed on != NULL map pointer (!ARG_PTR_TO_MEM_OR_NULL)",
5855+
.insns = {
5856+
BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
5857+
BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
5858+
BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
5859+
BPF_LD_MAP_FD(BPF_REG_1, 0),
5860+
BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
5861+
BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 5),
5862+
BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
5863+
BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_0, 0),
5864+
BPF_JMP_IMM(BPF_JGT, BPF_REG_2, 8, 2),
5865+
BPF_MOV64_IMM(BPF_REG_3, 0),
5866+
BPF_EMIT_CALL(BPF_FUNC_probe_read),
5867+
BPF_EXIT_INSN(),
5868+
},
5869+
.fixup_map1 = { 3 },
5870+
.result = ACCEPT,
5871+
.prog_type = BPF_PROG_TYPE_TRACEPOINT,
5872+
},
57745873
{
57755874
"helper access to variable memory: 8 bytes leak",
57765875
.insns = {

0 commit comments

Comments
 (0)