All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: bpf@vger.kernel.org, ast@kernel.org
Cc: andrii@kernel.org, daniel@iogearbox.net, martin.lau@linux.dev,
	kernel-team@fb.com, yhs@fb.com,
	Eduard Zingerman <eddyz87@gmail.com>
Subject: [PATCH bpf-next v1 0/2] bpf: verify scalar ids mapping in regsafe()
Date: Fri, 26 May 2023 21:41:24 +0300	[thread overview]
Message-ID: <20230526184126.3104040-1-eddyz87@gmail.com> (raw)

Update regsafe() to use check_ids() for scalar values.
Otherwise the following unsafe pattern is accepted by verifier:

  1: r9 = ... some pointer with range X ...
  2: r6 = ... unbound scalar ID=a ...
  3: r7 = ... unbound scalar ID=b ...
  4: if (r6 > r7) goto +1
  5: r6 = r7
  6: if (r6 > X) goto ...
  --- checkpoint ---
  7: r9 += r7
  8: *(u64 *)r9 = Y

See patch #1 for detailed description.

The change has limited impact on verification performance.
Here is veristat log comparing this patch with current master on a set
of selftest binaries listed in tools/testing/selftests/bpf/veristat.cfg
and cilium BPF binaries (see [1]):

$ ./veristat -e file,prog,states -f 'insns_pct>1' -C master-baseline.log current.log
File                      Program                         States (A)  States (B)  States   (DIFF)
------------------------  ------------------------------  ----------  ----------  ---------------
bpf_xdp.o                 tail_handle_nat_fwd_ipv6               648         660     +12 (+1.85%)
bpf_xdp.o                 tail_nodeport_nat_ingress_ipv4         375         455    +80 (+21.33%)
bpf_xdp.o                 tail_rev_nodeport_lb4                  398         472    +74 (+18.59%)
pyperf600_nounroll.bpf.o  on_event                             34169       39465  +5296 (+15.50%)
test_verif_scale1.bpf.o   balancer_ingress                      8636        8942    +306 (+3.54%)
test_verif_scale2.bpf.o   balancer_ingress                      3048        3149    +101 (+3.31%)
test_verif_scale3.bpf.o   balancer_ingress                      8636        8942    +306 (+3.54%)

This was previously posted as an RFC [2].

Changelog:
- RFC -> V1:
  - Function verifier.c:mark_equal_scalars_as_read() is dropped,
    as it was an incorrect fix for problem solved by commit [3].
  - check_ids() is called only for precise scalar values.
  - Test case updated to use inline assembly.

[1] git@github.com:anakryiko/cilium.git
[2] https://lore.kernel.org/bpf/20221128163442.280187-1-eddyz87@gmail.com/
[3] commit 71f656a50176 ("bpf: Fix to preserve reg parent/live fields when copying range info")

Eduard Zingerman (2):
  bpf: verify scalar ids mapping in regsafe() using check_ids()
  selftests/bpf: verify that check_ids() is used for scalars in
    regsafe()

 kernel/bpf/verifier.c                         | 31 +++++++++-
 .../selftests/bpf/prog_tests/verifier.c       |  2 +
 .../selftests/bpf/progs/verifier_scalar_ids.c | 59 +++++++++++++++++++
 3 files changed, 91 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/progs/verifier_scalar_ids.c

-- 
2.40.1


             reply	other threads:[~2023-05-26 18:42 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-26 18:41 Eduard Zingerman [this message]
2023-05-26 18:41 ` [PATCH bpf-next v1 1/2] bpf: verify scalar ids mapping in regsafe() using check_ids() Eduard Zingerman
2023-05-27  0:40   ` Yonghong Song
2023-05-27 12:21     ` Eduard Zingerman
2023-05-27 12:29       ` Eduard Zingerman
2023-05-27 23:43         ` Yonghong Song
2023-05-29  0:59           ` Eduard Zingerman
2023-05-26 18:41 ` [PATCH bpf-next v1 2/2] selftests/bpf: verify that check_ids() is used for scalars in regsafe() Eduard Zingerman

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=20230526184126.3104040-1-eddyz87@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@linux.dev \
    --cc=yhs@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.