mbox series

[bpf-next,v2,0/4] ] verifier, improve ptr is_branch_taken logic

Message ID 159009128301.6313.11384218513010252427.stgit@john-Precision-5820-Tower
Headers show
Series ] verifier, improve ptr is_branch_taken logic | expand

Message

John Fastabend May 21, 2020, 8:07 p.m. UTC
This series adds logic to the verifier to handle the case where a
pointer is known to be non-null but then the verifier encountesr a
instruction, such as 'if ptr == 0 goto X' or 'if ptr != 0 goto X',
where the pointer is compared against 0. Because the verifier tracks
if a pointer may be null in many cases we can improve the branch
tracking by following the case known to be true.

The first patch adds the verifier logic and patches 2-4 add the
test cases.

v1->v2: fix verifier logic to return -1 indicating both paths must 
still be walked if value is not zero. Move mark_precision skip for
this case into caller of mark_precision to ensure mark_precision can
still catch other misuses. And add PTR_TYPE_BTF_ID to our list of
supported types. Finally add one more test to catch the value not
equal zero case. Thanks to Andrii for original review. 

Also fixed up commit messages hopefully its better now.

---

John Fastabend (4):
      bpf: verifier track null pointer branch_taken with JNE and JEQ
      bpf: selftests, verifier case for non null pointer check branch taken
      bpf: selftests, verifier case for non null pointer map value branch
      bpf: selftests, add printk to test_sk_lookup_kern to encode null ptr check


 kernel/bpf/verifier.c                              |   36 ++++++++++++++++++--
 .../selftests/bpf/progs/test_sk_lookup_kern.c      |    1 +
 .../testing/selftests/bpf/verifier/ref_tracking.c  |   33 ++++++++++++++++++
 .../testing/selftests/bpf/verifier/value_or_null.c |   19 +++++++++++
 4 files changed, 86 insertions(+), 3 deletions(-)

--
Signature

Comments

Andrii Nakryiko May 21, 2020, 10:21 p.m. UTC | #1
On Thu, May 21, 2020 at 1:07 PM John Fastabend <john.fastabend@gmail.com> wrote:
>
> This series adds logic to the verifier to handle the case where a
> pointer is known to be non-null but then the verifier encountesr a
> instruction, such as 'if ptr == 0 goto X' or 'if ptr != 0 goto X',
> where the pointer is compared against 0. Because the verifier tracks
> if a pointer may be null in many cases we can improve the branch
> tracking by following the case known to be true.
>
> The first patch adds the verifier logic and patches 2-4 add the
> test cases.
>
> v1->v2: fix verifier logic to return -1 indicating both paths must
> still be walked if value is not zero. Move mark_precision skip for
> this case into caller of mark_precision to ensure mark_precision can
> still catch other misuses. And add PTR_TYPE_BTF_ID to our list of
> supported types. Finally add one more test to catch the value not
> equal zero case. Thanks to Andrii for original review.
>
> Also fixed up commit messages hopefully its better now.
>

Yeah, much better, thanks! Few typos don't count ;)

For the series:

Acked-by: Andrii Nakryiko <andriin@fb.com>

> ---
>
> John Fastabend (4):
>       bpf: verifier track null pointer branch_taken with JNE and JEQ
>       bpf: selftests, verifier case for non null pointer check branch taken
>       bpf: selftests, verifier case for non null pointer map value branch
>       bpf: selftests, add printk to test_sk_lookup_kern to encode null ptr check
>
>
>  kernel/bpf/verifier.c                              |   36 ++++++++++++++++++--
>  .../selftests/bpf/progs/test_sk_lookup_kern.c      |    1 +
>  .../testing/selftests/bpf/verifier/ref_tracking.c  |   33 ++++++++++++++++++
>  .../testing/selftests/bpf/verifier/value_or_null.c |   19 +++++++++++
>  4 files changed, 86 insertions(+), 3 deletions(-)
>
> --
> Signature
Alexei Starovoitov May 22, 2020, 12:49 a.m. UTC | #2
On Thu, May 21, 2020 at 3:21 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, May 21, 2020 at 1:07 PM John Fastabend <john.fastabend@gmail.com> wrote:
> >
> > This series adds logic to the verifier to handle the case where a
> > pointer is known to be non-null but then the verifier encountesr a
> > instruction, such as 'if ptr == 0 goto X' or 'if ptr != 0 goto X',
> > where the pointer is compared against 0. Because the verifier tracks
> > if a pointer may be null in many cases we can improve the branch
> > tracking by following the case known to be true.
> >
> > The first patch adds the verifier logic and patches 2-4 add the
> > test cases.
> >
> > v1->v2: fix verifier logic to return -1 indicating both paths must
> > still be walked if value is not zero. Move mark_precision skip for
> > this case into caller of mark_precision to ensure mark_precision can
> > still catch other misuses. And add PTR_TYPE_BTF_ID to our list of
> > supported types. Finally add one more test to catch the value not
> > equal zero case. Thanks to Andrii for original review.
> >
> > Also fixed up commit messages hopefully its better now.
> >
>
> Yeah, much better, thanks! Few typos don't count ;)
>
> For the series:
>
> Acked-by: Andrii Nakryiko <andriin@fb.com>

Applied. Thanks a lot everyone.