netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Paul Chaignon <paul.chaignon@orange.com>
To: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	netdev@vger.kernel.org, bpf@vger.kernel.org
Cc: Xiao Han <xiao.han@orange.com>, Martin KaFai Lau <kafai@fb.com>,
	Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>
Subject: [PATCH bpf v2 0/2] bpf: mark registers as safe or unknown in all frames
Date: Mon, 22 Apr 2019 20:34:02 +0200	[thread overview]
Message-ID: <cover.1555957346.git.paul.chaignon@orange.com> (raw)

In case of a null check on a pointer inside a subprog, we should mark all
registers with this pointer as either safe or unknown, in both the current
and previous frames.  Currently, only spilled registers and registers in
the current frame are marked.  This first patch also marks registers in
previous frames.

A good reproducer looks as follow:

1: ptr = bpf_map_lookup_elem(map, &key);
2: ret = subprog(ptr) {
3:   return ptr != NULL;
4: }
5: if (ret)
6:   value = *ptr;

With the above, the verifier will complain on line 6 because it sees ptr
as map_value_or_null despite the null check in subprog 1.  The second
patch implements the above as a new test case.

Note that this patch fixes another resulting bug when using
bpf_sk_release():

1: sk = bpf_sk_lookup_tcp(...);
2: subprog(sk) {
3:   if (sk)
4:     bpf_sk_release(sk);
5: }
6: if (!sk)
7:   return 0;
8: return 1;

In the above, mark_ptr_or_null_regs will warn on line 6 because it will
try to free the reference state, even though it was already freed on
line 3.

Changelogs:
  Changes in v2:
    - Fix example codes in commit message.

Paul Chaignon (2):
  bpf: mark registers as safe or unknown in all frames
  selftests/bpf: test case for pointer null check in subprog

 kernel/bpf/verifier.c                        |  6 ++---
 tools/testing/selftests/bpf/verifier/calls.c | 25 ++++++++++++++++++++
 2 files changed, 28 insertions(+), 3 deletions(-)

-- 
2.17.1


             reply	other threads:[~2019-04-22 18:34 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-22 18:34 Paul Chaignon [this message]
2019-04-22 18:34 ` [PATCH bpf v2 1/2] bpf: mark registers as safe or unknown in all frames Paul Chaignon
2019-04-22 23:47   ` Daniel Borkmann
2019-04-24 17:13     ` Paul Chaignon
2019-04-22 18:35 ` [PATCH bpf v2 2/2] selftests/bpf: test case for pointer null check in subprog Paul Chaignon
2019-04-22 22:07 ` [PATCH bpf v2 0/2] bpf: mark registers as safe or unknown in all frames 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=cover.1555957346.git.paul.chaignon@orange.com \
    --to=paul.chaignon@orange.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=kafai@fb.com \
    --cc=netdev@vger.kernel.org \
    --cc=songliubraving@fb.com \
    --cc=xiao.han@orange.com \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).