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.
Unified · Split
| @@ -661,7 +661,41 @@ static const struct bpf_func_proto bpf_get_stackid_proto_tp = { · 660 unmodified lines | ||
| 661 | 661 | .arg3_type = ARG_ANYTHING, |
| 662 | 662 | }; |
| 663 | 663 | |
| 664 | −BPF_CALL_3(bpf_perf_prog_read_value_tp, struct bpf_perf_event_data_kern *, ctx, | |
| 664 | +static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id) | |
| 665 | +{ | |
| 666 | + switch (func_id) { | |
| 667 | + case BPF_FUNC_perf_event_output: | |
| 668 | + return &bpf_perf_event_output_proto_tp; | |
| 669 | + case BPF_FUNC_get_stackid: | |
| 670 | + return &bpf_get_stackid_proto_tp; | |
| 671 | + default: | |
| 672 | + return tracing_func_proto(func_id); | |
| 673 | + } | |
| 674 | +} | |
| 675 | + | |
| 676 | +static bool tp_prog_is_valid_access(int off, int size, enum bpf_access_type type, | |
| 677 | + struct bpf_insn_access_aux *info) | |
| 678 | +{ | |
| 679 | + if (off < sizeof(void *) || off >= PERF_MAX_TRACE_SIZE) | |
| 680 | + return false; | |
| 681 | + if (type != BPF_READ) | |
| 682 | + return false; | |
| 683 | + if (off % size != 0) | |
| 684 | + return false; | |
| 685 | + | |
| 686 | + BUILD_BUG_ON(PERF_MAX_TRACE_SIZE % sizeof(__u64)); | |
| 687 | + return true; | |
| 688 | +} | |
| 689 | + | |
| 690 | +const struct bpf_verifier_ops tracepoint_verifier_ops = { | |
| 691 | + .get_func_proto = tp_prog_func_proto, | |
| 692 | + .is_valid_access = tp_prog_is_valid_access, | |
| 693 | +}; | |
| 694 | + | |
| 695 | +const struct bpf_prog_ops tracepoint_prog_ops = { | |
| 696 | +}; | |
| 697 | + | |
| 698 | +BPF_CALL_3(bpf_perf_prog_read_value, struct bpf_perf_event_data_kern *, ctx, | |
| 665 | 699 | struct bpf_perf_event_value *, buf, u32, size) |
| 666 | 700 | { |
| 667 | 701 | int err = -EINVAL; |
| @@ -678,8 +712,8 @@ clear: · 10 unmodified lines | ||
| 678 | 712 | return err; |
| 679 | 713 | } |
| 680 | 714 | |
| 681 | −static const struct bpf_func_proto bpf_perf_prog_read_value_proto_tp = { | |
| 682 | − .func = bpf_perf_prog_read_value_tp, | |
| 715 | +static const struct bpf_func_proto bpf_perf_prog_read_value_proto = { | |
| 716 | + .func = bpf_perf_prog_read_value, | |
| 683 | 717 | .gpl_only = true, |
| 684 | 718 | .ret_type = RET_INTEGER, |
| 685 | 719 | .arg1_type = ARG_PTR_TO_CTX, |
| @@ -687,7 +721,7 @@ static const struct bpf_func_proto bpf_perf_prog_read_value_proto_tp = { · 1 unmodified line | ||
| 687 | 721 | .arg3_type = ARG_CONST_SIZE, |
| 688 | 722 | }; |
| 689 | 723 | |
| 690 | −static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id) | |
| 724 | +static const struct bpf_func_proto *pe_prog_func_proto(enum bpf_func_id func_id) | |
| 691 | 725 | { |
| 692 | 726 | switch (func_id) { |
| 693 | 727 | 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) · 1 unmodified line | ||
| 695 | 729 | case BPF_FUNC_get_stackid: |
| 696 | 730 | return &bpf_get_stackid_proto_tp; |
| 697 | 731 | case BPF_FUNC_perf_prog_read_value: |
| 698 | − return &bpf_perf_prog_read_value_proto_tp; | |
| 732 | + return &bpf_perf_prog_read_value_proto; | |
| 699 | 733 | default: |
| 700 | 734 | return tracing_func_proto(func_id); |
| 701 | 735 | } |
| 702 | 736 | } |
| 703 | 737 | |
| 704 | −static bool tp_prog_is_valid_access(int off, int size, enum bpf_access_type type, | |
| 705 | − struct bpf_insn_access_aux *info) | |
| 706 | −{ | |
| 707 | − if (off < sizeof(void *) || off >= PERF_MAX_TRACE_SIZE) | |
| 708 | − return false; | |
| 709 | − if (type != BPF_READ) | |
| 710 | − return false; | |
| 711 | − if (off % size != 0) | |
| 712 | − return false; | |
| 713 | − | |
| 714 | − BUILD_BUG_ON(PERF_MAX_TRACE_SIZE % sizeof(__u64)); | |
| 715 | − return true; | |
| 716 | −} | |
| 717 | − | |
| 718 | −const struct bpf_verifier_ops tracepoint_verifier_ops = { | |
| 719 | − .get_func_proto = tp_prog_func_proto, | |
| 720 | − .is_valid_access = tp_prog_is_valid_access, | |
| 721 | −}; | |
| 722 | − | |
| 723 | −const struct bpf_prog_ops tracepoint_prog_ops = { | |
| 724 | −}; | |
| 725 | − | |
| 726 | 738 | static bool pe_prog_is_valid_access(int off, int size, enum bpf_access_type type, |
| 727 | 739 | struct bpf_insn_access_aux *info) |
| 728 | 740 | { |
| @@ -779,7 +791,7 @@ static u32 pe_prog_convert_ctx_access(enum bpf_access_type type, · 50 unmodified lines | ||
| 779 | 791 | } |
| 780 | 792 | |
| 781 | 793 | const struct bpf_verifier_ops perf_event_verifier_ops = { |
| 782 | − .get_func_proto = tp_prog_func_proto, | |
| 794 | + .get_func_proto = pe_prog_func_proto, | |
| 783 | 795 | .is_valid_access = pe_prog_is_valid_access, |
| 784 | 796 | .convert_ctx_access = pe_prog_convert_ctx_access, |
| 785 | 797 | }; |