All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kumar Kartikeya Dwivedi <memxor@gmail.com>
To: bpf@vger.kernel.org
Cc: kkd@meta.com, Alexei Starovoitov <ast@kernel.org>,
	Andrii Nakryiko <andrii@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Martin KaFai Lau <martin.lau@kernel.org>,
	Eduard Zingerman <eddyz87@gmail.com>,
	Manu Bretelle <chantra@meta.com>,
	kernel-team@fb.com
Subject: [PATCH bpf v3 0/3] Fix for raw_tp PTR_MAYBE_NULL handling
Date: Fri,  6 Dec 2024 08:10:50 -0800	[thread overview]
Message-ID: <20241206161053.809580-1-memxor@gmail.com> (raw)

A production BPF program had the following code produced by LLVM.

r0 = 1024;
r1 = ...; // r1 = trusted_or_null_(id=1)
r3 = r1;  // r3 = trusted_or_null_(id=1) r1 = trusted_or_null_(id=1)
r3 += r0; // r3 = trusted_or_null_(id=1, off=1024)
if r1 == 0 goto pc+X;

After cb4158ce8ec8 ("bpf: Mark raw_tp arguments with PTR_MAYBE_NULL"),
the production BPF program began throwing a warning in the verifier
because for the code above, when unmarking null mark from r1, the
verifier will notice another register r3 with same id but off != 0,
which is unexpected, since offset modification on PTR_MAYBE_NULL is not
permitted, but the aforementioned commit relaxed that restriction to
preserve compatibility with non-NULL raw_tp args.

Another production program hit a case where generic code it was calling
into would perform a NULL check, while the program knows and is written
with the knowledge that the raw_tp arg can never be NULL.

In earlier versions before the raw_tp change, verifier would never walk
the path where raw_tp arg was seen as scalar zero, but now it will,
hence code in the program that operates on the raw_tp arg later on will
fail on dereferencing a scalar.

Provide a fix to suppress the warning for raw_tp args, and not mark NULL
checked raw_tp args as scalars. We will follow up with a more generic
fix to handle such patterns for all pointer types in the verifier, which
currently involves playing whack-a-mole with suppressing such LLVM
optimizations and reworking BPF programs to avoid verifier errors.

Changelog:
----------
v2 -> v3
v2: https://lore.kernel.org/bpf/20241205223152.2434683-1-memxor@gmail.com

 * Add Acked-by for Patch 1
 * Add fix for scalar dereference issue
 * Roll both fixes into one, as second fix undoes first
 * Fix nits

v1 -> v2
v1: https://lore.kernel.org/bpf/20241204024154.21386-1-memxor@gmail.com

 * Fix eager unmarking bug (Eduard)
 * Generalize approach, always unmark NULL when off == 0 is checked
 * Make NULL check noop if operand has off != 0
 * Do not reset id when treating as noop
 * Trim comment (Alexei)
 * Adjust selftests

Kumar Kartikeya Dwivedi (3):
  bpf: Suppress warning for non-zero off raw_tp arg NULL check
  bpf: Do not mark NULL-checked raw_tp arg as scalar
  selftests/bpf: Add raw_tp tests for PTR_MAYBE_NULL marking

 kernel/bpf/verifier.c                         | 44 +++++++--
 .../selftests/bpf/prog_tests/raw_tp_null.c    |  6 ++
 .../selftests/bpf/progs/raw_tp_null_fail.c    | 90 +++++++++++++++++++
 3 files changed, 133 insertions(+), 7 deletions(-)
 create mode 100644 tools/testing/selftests/bpf/progs/raw_tp_null_fail.c


base-commit: 5a6ea7022ff4d2a65ae328619c586d6a8909b48b
-- 
2.43.5


             reply	other threads:[~2024-12-06 16:10 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-06 16:10 Kumar Kartikeya Dwivedi [this message]
2024-12-06 16:10 ` [PATCH bpf v3 1/3] bpf: Suppress warning for non-zero off raw_tp arg NULL check Kumar Kartikeya Dwivedi
2024-12-06 16:10 ` [PATCH bpf v3 2/3] bpf: Do not mark NULL-checked raw_tp arg as scalar Kumar Kartikeya Dwivedi
2024-12-06 17:59   ` Alexei Starovoitov
2024-12-06 18:10     ` Kumar Kartikeya Dwivedi
2024-12-06 18:37       ` Alexei Starovoitov
2024-12-06 19:09         ` Kumar Kartikeya Dwivedi
2024-12-06 19:14           ` Alexei Starovoitov
2024-12-09 23:35       ` Jiri Olsa
2024-12-06 18:15     ` Eduard Zingerman
2024-12-06 18:24       ` Kumar Kartikeya Dwivedi
2024-12-06 18:36       ` Alexei Starovoitov
2024-12-06 19:10         ` Kumar Kartikeya Dwivedi
2024-12-06 19:18           ` Alexei Starovoitov
2024-12-06 16:10 ` [PATCH bpf v3 3/3] selftests/bpf: Add raw_tp tests for PTR_MAYBE_NULL marking Kumar Kartikeya Dwivedi

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=20241206161053.809580-1-memxor@gmail.com \
    --to=memxor@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=chantra@meta.com \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=kernel-team@fb.com \
    --cc=kkd@meta.com \
    --cc=martin.lau@kernel.org \
    /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.