mbox series

[bpf,v4,0/2] Fix verifier id tracking of scalars on spill

Message ID 20230607123951.558971-1-maxtram95@gmail.com (mailing list archive)
Headers show
Series Fix verifier id tracking of scalars on spill | expand

Message

Maxim Mikityanskiy June 7, 2023, 12:39 p.m. UTC 
From: Maxim Mikityanskiy <maxim@isovalent.com>

See the details in the commit message (TL/DR: under CAP_BPF, the
verifier can incorrectly conclude that a scalar is zero while in
fact it can be crafted to a predefined number.)

v1 and v2 were sent off-list.

v2 changes:

Added more tests, migrated them to inline asm, started using
bpf_get_prandom_u32, switched to a more bulletproof dead branch check
and modified the failing spill test scenarios so that an unauthorized
access attempt is performed in both branches.

v3 changes:

Dropped an improvement not necessary for the fix, changed the Fixes tag.

v4 changes:

Dropped supposedly redundant tests, kept the ones that result in
different verifier verdicts. Dropped the variable that is not yet
useful in this patch. Rephrased the commit message with Daniel's
suggestions.

Maxim Mikityanskiy (2):
  bpf: Fix verifier id tracking of scalars on spill
  selftests/bpf: Add test cases to assert proper ID tracking on spill

 kernel/bpf/verifier.c                         |  3 +
 .../selftests/bpf/progs/verifier_spill_fill.c | 79 +++++++++++++++++++
 2 files changed, 82 insertions(+)

Comments

Daniel Borkmann June 8, 2023, 11:21 a.m. UTC | #1 
On 6/7/23 2:39 PM, Maxim Mikityanskiy wrote:
[...]
> v4 changes:
> 
> Dropped supposedly redundant tests, kept the ones that result in
> different verifier verdicts. Dropped the variable that is not yet
> useful in this patch. Rephrased the commit message with Daniel's
> suggestions.

Andrii mentioned he did some veristat measurements and they looked good
to him. Looks like patchbot didn't reply, I've pushed it to bpf, thanks!