All of lore.kernel.org
 help / color / mirror / Atom feed
From: Yonghong Song <yhs@fb.com>
To: <bpf@vger.kernel.org>
Cc: Alexei Starovoitov <ast@fb.com>,
	Daniel Borkmann <daniel@iogearbox.net>, <kernel-team@fb.com>
Subject: [PATCH bpf-next 0/2] bpf: improve verifier handling for 32bit signed compare operations
Date: Thu, 23 Jan 2020 11:18:15 -0800	[thread overview]
Message-ID: <20200123191815.1364298-1-yhs@fb.com> (raw)

Commit b7a0d65d80a0 ("bpf, testing: Workaround a verifier failure
for test_progs") worked around a verifier failure where the
register is copied to another later refined register, but the
original register is used after refinement. The pattern
looks like:
   call ...
   w1 = w0
   w1 += -1
   if w1 > 6 goto -24
   w0 += w8
   ...
Register "w1" is refined, but "w0" is used later without taking
advantage of new "w1" range, and eventually leading verifier to
reject the program.

Instead of complicating verifier for such analysis,
llvm patch https://reviews.llvm.org/D72787 added a phase to
undo some original optimization and produces the code like below:
  call ...
  if w0 s< 0x1 goto pc-22
  if w0 s> 0x7 goto pc-23
  w0 += w8
  ...
Current verifier still rejects the above code. This patch
intends to enhance verifier to handle the above pattern.

Note that the verifier is able to handle the case at non-alu32 mode,
  call ...
  if r0 s< 0x1 goto pc-22
  if r0 s> 0x7 goto pc-23
  r0 += r8
So the problem as described in
  https://lore.kernel.org/netdev/871019a0-71f8-c26d-0ae8-c7fd8c8867fc@fb.com/
can be resolved with just compiler change.

The implementation in this patch set is just to cater the above pattern
or similar to the above pattern. If you have some cases where the compiler
generates a copy of register to refine but still use the original register
later, please let me know, we could improve llvm/kernel to accommodate
your use case.

Yonghong Song (2):
  bpf: improve verifier handling for 32bit signed compare operations
  selftests/bpf: add selftests for verifier handling 32bit signed
    compares

 include/linux/bpf_verifier.h                  |  2 +
 kernel/bpf/verifier.c                         | 73 +++++++++++++++---
 .../selftests/bpf/progs/test_sysctl_loop1.c   |  5 +-
 tools/testing/selftests/bpf/verifier/jmp32.c  | 76 +++++++++++++++++++
 4 files changed, 142 insertions(+), 14 deletions(-)

-- 
2.17.1


             reply	other threads:[~2020-01-23 19:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-23 19:18 Yonghong Song [this message]
2020-01-23 19:18 ` [PATCH bpf-next 1/2] bpf: improve verifier handling for 32bit signed compare operations Yonghong Song
2020-01-24  3:06   ` John Fastabend
2020-01-24  7:14     ` Yonghong Song
2020-01-23 19:18 ` [PATCH bpf-next 2/2] selftests/bpf: add selftests for verifier handling 32bit signed compares Yonghong Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200123191815.1364298-1-yhs@fb.com \
    --to=yhs@fb.com \
    --cc=ast@fb.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.