trace/bpf: remove helper bpf_perf_prog_read_value from tracepoint type programs
Subsystems (6)
PATCH 1/1 trace/bpf: remove helper bpf_perf_prog_read_value from tracepoint type programs
Commit 4bebdc7a85aa ("bpf: add helper bpf_perf_prog_read_value") added helper bpf_perf_prog_read_value so that perf_event type program can read event counter and enabled/running time.
This commit, however, introduced a bug which allows this helper for tracepoint type programs. This is incorrect as bpf_perf_prog_read_value needs to access perf_event through its bpf_perf_event_data_kern type context, which is not available for tracepoint type program.
This patch fixed the issue by separating bpf_func_proto between tracepoint and perf_event type programs and removed bpf_perf_prog_read_value from tracepoint func prototype.
--- kernel/trace/bpf_trace.c | 68 ++++++++++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 28 deletions(-)
kernel/trace/bpf_trace.c +40 -28
@@ -661,7 +661,41 @@ static const struct bpf_func_proto bpf_get_stackid_proto_tp = { .arg3_type = ARG_ANYTHING, }; -BPF_CALL_3(bpf_perf_prog_read_value_tp, struct bpf_perf_event_data_kern *, ctx, +static const struct bpf_func_proto *tp_prog_func_proto (enum bpf_func_id func_id) +{ +switch (func_id) { +case BPF_FUNC_perf_event_output: +return &bpf_perf_event_output_proto_tp; +case BPF_FUNC_get_stackid: +return &bpf_get_stackid_proto_tp; +default : +return tracing_func_proto (func_id); + } +} + +static bool tp_prog_is_valid_access (int off,int size,enum bpf_access_type type, +struct bpf_insn_access_aux *info) +{ +if (off <sizeof (void *) || off >=PERF_MAX_TRACE_SIZE ) +return false; +if (type !=BPF_READ ) +return false; +if (off % size !=0 ) +return false; + +BUILD_BUG_ON (PERF_MAX_TRACE_SIZE %sizeof (__u64)); +return true; +} + +const struct bpf_verifier_ops tracepoint_verifier_ops = { + .get_func_proto = tp_prog_func_proto, + .is_valid_access = tp_prog_is_valid_access, +}; + +const struct bpf_prog_ops tracepoint_prog_ops = { +}; + +BPF_CALL_3(bpf_perf_prog_read_value, struct bpf_perf_event_data_kern *, ctx, struct bpf_perf_event_value *, buf, u32, size) {int err = -EINVAL ;
@@ -678,8 +712,8 @@ BPF_CALL_3(bpf_perf_prog_read_value_tp, struct bpf_perf_event_data_kern *, ctx,return err; } -static const struct bpf_func_proto bpf_perf_prog_read_value_proto_tp = { - .func = bpf_perf_prog_read_value_tp, +static const struct bpf_func_proto bpf_perf_prog_read_value_proto = { + .func = bpf_perf_prog_read_value, .gpl_only = true, .ret_type =RET_INTEGER , .arg1_type =ARG_PTR_TO_CTX ,
@@ -687,7 +721,7 @@ static const struct bpf_func_proto bpf_perf_prog_read_value_proto_tp = { .arg3_type = ARG_CONST_SIZE, }; -static const struct bpf_func_proto *tp_prog_func_proto (enum bpf_func_id func_id) +static const struct bpf_func_proto *pe_prog_func_proto (enum bpf_func_id func_id) {switch (func_id) {case BPF_FUNC_perf_event_output:
@@ -695,34 +729,12 @@ static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)case BPF_FUNC_get_stackid:return &bpf_get_stackid_proto_tp;case BPF_FUNC_perf_prog_read_value: -return &bpf_perf_prog_read_value_proto_tp; +return &bpf_perf_prog_read_value_proto;default :return tracing_func_proto (func_id); } } -static bool tp_prog_is_valid_access (int off,int size,enum bpf_access_type type, -struct bpf_insn_access_aux *info) -{ -if (off <sizeof (void *) || off >=PERF_MAX_TRACE_SIZE ) -return false; -if (type !=BPF_READ ) -return false; -if (off % size !=0 ) -return false; - -BUILD_BUG_ON (PERF_MAX_TRACE_SIZE %sizeof (__u64)); -return true; -} - -const struct bpf_verifier_ops tracepoint_verifier_ops = { - .get_func_proto = tp_prog_func_proto, - .is_valid_access = tp_prog_is_valid_access, -}; - -const struct bpf_prog_ops tracepoint_prog_ops = { -}; -static bool pe_prog_is_valid_access (int off,int size,enum bpf_access_type type,struct bpf_insn_access_aux *info) {
@@ -791,7 +803,7 @@ static u32 pe_prog_convert_ctx_access(enum bpf_access_type type, }const struct bpf_verifier_ops perf_event_verifier_ops = { - .get_func_proto = tp_prog_func_proto, + .get_func_proto = pe_prog_func_proto, .is_valid_access = pe_prog_is_valid_access, .convert_ctx_access = pe_prog_convert_ctx_access, };
Signature · 1 line
2.9.5
Daniel Borkmann
maintainerQuoted 8 lines · Yonghong Song
Commit
4bebdc7a85aa("bpf: add helperbpf_perf_prog_read_value") added helperbpf_perf_prog_read_valueso thatperf_eventtype program can read event counter and enabled/running time.
This commit, however, introduced a bug which allows this helper for tracepoint type programs. This is incorrect asbpf_perf_prog_read_valueneeds to accessperf_eventthrough itsbpf_perf_event_data_kerntype context, which is not available for tracepoint type program.
Quoted 7 lines
This patch fixed the issue by separating
bpf_func_protobetween tracepoint andperf_eventtype programs and removedbpf_perf_prog_read_valuefrom tracepoint func prototype.Fixes Commit 4bebdc7a85aa ("bpf: add helper bpf_perf_prog_read_value")Reported-by Alexei Starovoitov <[email protected]>Signed-off-by Yonghong Song <[email protected]>
Sigh, also makes sense to split given this is so subtle. Applied to bpf, thanks Yonghong!