[bpf,v2,0/2] Forbid bpf_ktime_get_coarse_ns and bpf_timer_* in tracing progs

Message

Dmitrii Banshchikov Nov. 13, 2021, 2:22 p.m. UTC

Various locking issues are possible with bpf_ktime_get_coarse_ns() and
bpf_timer_* set of helpers.

syzbot found a locking issue with bpf_ktime_get_coarse_ns() helper executed in
BPF_PROG_TYPE_PERF_EVENT prog type - [1]. The issue is possible because the
helper uses non fast version of time accessor that isn't safe for any context.
The helper was added because it provided performance benefits in comparison to
bpf_ktime_get_ns() helper.

A similar locking issue is possible with bpf_timer_* set of helpers when used
in tracing progs.

The solution is to restrict use of the helpers in tracing progs.

In the [1] discussion it was stated that bpf_spin_lock related helpers shall
also be excluded for tracing progs. The verifier has a compatibility check
between a map and a program. If a tracing program tries to use a map which
value has struct bpf_spin_lock the verifier fails that is why bpf_spin_lock is
already restricted.

Patch 1 restricts helpers
Patch 2 adds tests

v1 -> v2:
 * Limit the helpers via func proto getters instead of allowed callback
 * Add note about helpers' restrictions to linux/bpf.h
 * Add Fixes tag
 * Remove extra \0 from btf_str_sec
 * Beside asm tests add prog tests
 * Trim CC

1. https://lore.kernel.org/all/[e-mail]/

Dmitrii Banshchikov (2):
  bpf: Forbid bpf_ktime_get_coarse_ns and bpf_timer_* in tracing progs
  selftests/bpf: Add tests for restricted helpers

 include/uapi/linux/bpf.h                      |   6 +
 kernel/bpf/cgroup.c                           |   2 +
 kernel/bpf/helpers.c                          |   2 -
 kernel/bpf/verifier.c                         |   7 +
 kernel/trace/bpf_trace.c                      |   2 -
 net/core/filter.c                             |   6 +
 net/ipv4/bpf_tcp_ca.c                         |   2 +
 tools/include/uapi/linux/bpf.h                |   6 +
 .../bpf/prog_tests/helper_restricted.c        |  33 +++
 .../bpf/progs/test_helper_restricted.c        | 123 +++++++++++
 tools/testing/selftests/bpf/test_verifier.c   |  46 +++-
 .../bpf/verifier/helper_restricted.c          | 196 ++++++++++++++++++
 12 files changed, 426 insertions(+), 5 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/helper_restricted.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_helper_restricted.c
 create mode 100644 tools/testing/selftests/bpf/verifier/helper_restricted.c

Comments

Alexei Starovoitov Nov. 14, 2021, 6:38 p.m. UTC | #1

On Sat, Nov 13, 2021 at 6:22 AM Dmitrii Banshchikov <[e-mail]> wrote:
>
> Various locking issues are possible with bpf_ktime_get_coarse_ns() and
> bpf_timer_* set of helpers.
>
> syzbot found a locking issue with bpf_ktime_get_coarse_ns() helper executed in
> BPF_PROG_TYPE_PERF_EVENT prog type - [1]. The issue is possible because the
> helper uses non fast version of time accessor that isn't safe for any context.
> The helper was added because it provided performance benefits in comparison to
> bpf_ktime_get_ns() helper.
>
> A similar locking issue is possible with bpf_timer_* set of helpers when used
> in tracing progs.
>
> The solution is to restrict use of the helpers in tracing progs.
>
> In the [1] discussion it was stated that bpf_spin_lock related helpers shall
> also be excluded for tracing progs. The verifier has a compatibility check
> between a map and a program. If a tracing program tries to use a map which
> value has struct bpf_spin_lock the verifier fails that is why bpf_spin_lock is
> already restricted.
>
> Patch 1 restricts helpers
> Patch 2 adds tests
>
> v1 -> v2:
>  * Limit the helpers via func proto getters instead of allowed callback
>  * Add note about helpers' restrictions to linux/bpf.h
>  * Add Fixes tag
>  * Remove extra \0 from btf_str_sec
>  * Beside asm tests add prog tests
>  * Trim CC
>
> 1. https://lore.kernel.org/all/[e-mail]/

Applied. Thanks

Thomas Gleixner Nov. 16, 2021, 2:18 a.m. UTC | #2

Alexei,

On Sun, Nov 14 2021 at 10:38, Alexei Starovoitov wrote:
> On Sat, Nov 13, 2021 at 6:22 AM Dmitrii Banshchikov <[e-mail]> wrote:
>> v1 -> v2:
>>  * Limit the helpers via func proto getters instead of allowed callback
>>  * Add note about helpers' restrictions to linux/bpf.h
>>  * Add Fixes tag
>>  * Remove extra \0 from btf_str_sec
>>  * Beside asm tests add prog tests
>>  * Trim CC
>>
>> 1. https://lore.kernel.org/all/[e-mail]/
>
> Applied. Thanks

applying crap faster than anyone involved can look at (hint: weekend)
and without actually looking at the nonsense propagated by this 'hot
fix' is what is going to cause long term maintaience trouble. Please
revert the offending and obvious bogus comments in those commits.

Thanks,

        tglx