Commit a60dd35d authored by Gianluca Borello's avatar Gianluca Borello Committed by Daniel Borkmann

bpf: change bpf_perf_event_output arg5 type to ARG_CONST_SIZE_OR_ZERO

Commit 9fd29c08 ("bpf: improve verifier ARG_CONST_SIZE_OR_ZERO
semantics") relaxed the treatment of ARG_CONST_SIZE_OR_ZERO due to the way
the compiler generates optimized BPF code when checking boundaries of an
argument from C code. A typical example of this optimized code can be
generated using the bpf_perf_event_output helper when operating on variable
memory:

/* len is a generic scalar */
if (len > 0 && len <= 0x7fff)
        bpf_perf_event_output(ctx, &perf_map, 0, buf, len);

110: (79) r5 = *(u64 *)(r10 -40)
111: (bf) r1 = r5
112: (07) r1 += -1
113: (25) if r1 > 0x7ffe goto pc+6
114: (bf) r1 = r6
115: (18) r2 = 0xffff94e5f166c200
117: (b7) r3 = 0
118: (bf) r4 = r7
119: (85) call bpf_perf_event_output#25
R5 min value is negative, either use unsigned or 'var &= const'

With this code, the verifier loses track of the variable.

Replacing arg5 with ARG_CONST_SIZE_OR_ZERO is thus desirable since it
avoids this quite common case which leads to usability issues, and the
compiler generates code that the verifier can more easily test:

if (len <= 0x7fff)
        bpf_perf_event_output(ctx, &perf_map, 0, buf, len);

or

bpf_perf_event_output(ctx, &perf_map, 0, buf, len & 0x7fff);

No changes to the bpf_perf_event_output helper are necessary since it can
handle a case where size is 0, and an empty frame is pushed.
Reported-by: default avatarArnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: default avatarGianluca Borello <g.borello@gmail.com>
Acked-by: default avatarAlexei Starovoitov <ast@kernel.org>
Acked-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
parent 5c4e1201
...@@ -403,7 +403,7 @@ static const struct bpf_func_proto bpf_perf_event_output_proto = { ...@@ -403,7 +403,7 @@ static const struct bpf_func_proto bpf_perf_event_output_proto = {
.arg2_type = ARG_CONST_MAP_PTR, .arg2_type = ARG_CONST_MAP_PTR,
.arg3_type = ARG_ANYTHING, .arg3_type = ARG_ANYTHING,
.arg4_type = ARG_PTR_TO_MEM, .arg4_type = ARG_PTR_TO_MEM,
.arg5_type = ARG_CONST_SIZE, .arg5_type = ARG_CONST_SIZE_OR_ZERO,
}; };
static DEFINE_PER_CPU(struct pt_regs, bpf_pt_regs); static DEFINE_PER_CPU(struct pt_regs, bpf_pt_regs);
...@@ -605,7 +605,7 @@ static const struct bpf_func_proto bpf_perf_event_output_proto_tp = { ...@@ -605,7 +605,7 @@ static const struct bpf_func_proto bpf_perf_event_output_proto_tp = {
.arg2_type = ARG_CONST_MAP_PTR, .arg2_type = ARG_CONST_MAP_PTR,
.arg3_type = ARG_ANYTHING, .arg3_type = ARG_ANYTHING,
.arg4_type = ARG_PTR_TO_MEM, .arg4_type = ARG_PTR_TO_MEM,
.arg5_type = ARG_CONST_SIZE, .arg5_type = ARG_CONST_SIZE_OR_ZERO,
}; };
BPF_CALL_3(bpf_get_stackid_tp, void *, tp_buff, struct bpf_map *, map, BPF_CALL_3(bpf_get_stackid_tp, void *, tp_buff, struct bpf_map *, map,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment