[v4,bpf-next,00/11] BPF verifier retval logic fixes

Message

Andrii Nakryiko Dec. 1, 2023, 6:33 p.m. UTC

This patch set fixes BPF verifier logic around validating and enforcing return
values for BPF programs that have specific range of expected return values.
Both sync and async callbacks have similar logic and are fixes as well.
A few tests are added that would fail without the fixes in this patch set.

Also, while at it, we update retval checking logic to use smin/smax range
instead of tnum, avoiding future potential issues if expected range cannot be
represented precisely by tnum (e.g., [0, 2] is not representable by tnum and
is treated as [0, 3]).

There is a little bit of refactoring to unify async callback and program exit
logic to avoid duplication of checks as much as possible.

v3->v4:
  - add back bpf_func_state rearrangement patch;
  - simplified patch #4 as suggested (Shung-Hsi);
v2->v3:
  - more carefullly switch from umin/umax to smin/smax;
v1->v2:
  - drop tnum from retval checks (Eduard);
  - use smin/smax instead of umin/umax (Alexei).

Andrii Nakryiko (11):
  bpf: rearrange bpf_func_state fields to save a bit of memory
  bpf: provide correct register name for exception callback retval check
  bpf: enforce precision of R0 on callback return
  bpf: enforce exact retval range on subprog/callback exit
  selftests/bpf: add selftest validating callback result is enforced
  bpf: enforce precise retval range on program exit
  bpf: unify async callback and program retval checks
  bpf: enforce precision of R0 on program/async callback return
  selftests/bpf: validate async callback return value check correctness
  selftests/bpf: adjust global_func15 test to validate prog exit
    precision
  bpf: simplify tnum output if a fully known constant

 include/linux/bpf_verifier.h                  |   9 +-
 kernel/bpf/log.c                              |  13 ++
 kernel/bpf/tnum.c                             |   6 -
 kernel/bpf/verifier.c                         | 120 ++++++++++--------
 .../selftests/bpf/progs/exceptions_assert.c   |   2 +-
 .../selftests/bpf/progs/exceptions_fail.c     |   2 +-
 .../selftests/bpf/progs/test_global_func15.c  |  34 ++++-
 .../selftests/bpf/progs/timer_failure.c       |  36 ++++--
 .../selftests/bpf/progs/user_ringbuf_fail.c   |   2 +-
 .../bpf/progs/verifier_cgroup_inv_retcode.c   |   8 +-
 .../bpf/progs/verifier_direct_packet_access.c |   2 +-
 .../selftests/bpf/progs/verifier_int_ptr.c    |   2 +-
 .../bpf/progs/verifier_netfilter_retcode.c    |   2 +-
 .../selftests/bpf/progs/verifier_stack_ptr.c  |   4 +-
 .../bpf/progs/verifier_subprog_precision.c    |  50 ++++++++
 15 files changed, 212 insertions(+), 80 deletions(-)

Comments

Andrii Nakryiko Dec. 1, 2023, 8:11 p.m. UTC | #1

On Fri, Dec 1, 2023 at 10:34 AM Andrii Nakryiko <[e-mail]> wrote:
>
> This patch set fixes BPF verifier logic around validating and enforcing return
> values for BPF programs that have specific range of expected return values.
> Both sync and async callbacks have similar logic and are fixes as well.
> A few tests are added that would fail without the fixes in this patch set.
>
> Also, while at it, we update retval checking logic to use smin/smax range
> instead of tnum, avoiding future potential issues if expected range cannot be
> represented precisely by tnum (e.g., [0, 2] is not representable by tnum and
> is treated as [0, 3]).
>
> There is a little bit of refactoring to unify async callback and program exit
> logic to avoid duplication of checks as much as possible.
>
> v3->v4:
>   - add back bpf_func_state rearrangement patch;
>   - simplified patch #4 as suggested (Shung-Hsi);
> v2->v3:
>   - more carefullly switch from umin/umax to smin/smax;
> v1->v2:
>   - drop tnum from retval checks (Eduard);
>   - use smin/smax instead of umin/umax (Alexei).

This patch set must be cursed or something :) CI caught regression for
no-alu32 test_progs variant in test_bad_ret:

EXPECTED MSG: 'mark_precise: frame0: regs=r0 stack= before 22: (b4) w0 = 0'

I'll check, fix, and will try again, maybe v5 will be luckier.

>
> Andrii Nakryiko (11):
>   bpf: rearrange bpf_func_state fields to save a bit of memory
>   bpf: provide correct register name for exception callback retval check
>   bpf: enforce precision of R0 on callback return
>   bpf: enforce exact retval range on subprog/callback exit
>   selftests/bpf: add selftest validating callback result is enforced
>   bpf: enforce precise retval range on program exit
>   bpf: unify async callback and program retval checks
>   bpf: enforce precision of R0 on program/async callback return
>   selftests/bpf: validate async callback return value check correctness
>   selftests/bpf: adjust global_func15 test to validate prog exit
>     precision
>   bpf: simplify tnum output if a fully known constant
>
>  include/linux/bpf_verifier.h                  |   9 +-
>  kernel/bpf/log.c                              |  13 ++
>  kernel/bpf/tnum.c                             |   6 -
>  kernel/bpf/verifier.c                         | 120 ++++++++++--------
>  .../selftests/bpf/progs/exceptions_assert.c   |   2 +-
>  .../selftests/bpf/progs/exceptions_fail.c     |   2 +-
>  .../selftests/bpf/progs/test_global_func15.c  |  34 ++++-
>  .../selftests/bpf/progs/timer_failure.c       |  36 ++++--
>  .../selftests/bpf/progs/user_ringbuf_fail.c   |   2 +-
>  .../bpf/progs/verifier_cgroup_inv_retcode.c   |   8 +-
>  .../bpf/progs/verifier_direct_packet_access.c |   2 +-
>  .../selftests/bpf/progs/verifier_int_ptr.c    |   2 +-
>  .../bpf/progs/verifier_netfilter_retcode.c    |   2 +-
>  .../selftests/bpf/progs/verifier_stack_ptr.c  |   4 +-
>  .../bpf/progs/verifier_subprog_precision.c    |  50 ++++++++
>  15 files changed, 212 insertions(+), 80 deletions(-)
>
> --
> 2.34.1
>