From: Andrii Nakryiko <andrii@kernel.org>
To: <bpf@vger.kernel.org>, <ast@kernel.org>, <daniel@iogearbox.net>
Cc: <andrii@kernel.org>, <kernel-team@fb.com>
Subject: [PATCH bpf-next 4/7] bpf: reject non-exact register type matches in regsafe()
Date: Thu, 22 Dec 2022 21:49:18 -0800 [thread overview]
Message-ID: <20221223054921.958283-5-andrii@kernel.org> (raw)
In-Reply-To: <20221223054921.958283-1-andrii@kernel.org>
Generalize the (somewhat implicit) rule of regsafe(), which states that
if register types in old and current states do not match *exactly*, they
can't be safely considered equivalent.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
kernel/bpf/verifier.c | 45 ++++++++++++++++++++-----------------------
1 file changed, 21 insertions(+), 24 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 218a7ace4210..5133d0a5b0cb 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13075,18 +13075,28 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
if (rcur->type == NOT_INIT)
return false;
- /* Register types that are *not* MAYBE_NULL could technically be safe
- * to use as their MAYBE_NULL variants (e.g., PTR_TO_MAP_VALUE is
- * safe to be used as PTR_TO_MAP_VALUE_OR_NULL, provided both point to
- * the same map).
+ /* Enforce that register types have to match exactly, including their
+ * modifiers (like PTR_MAYBE_NULL, MEM_RDONLY, etc), as a general
+ * rule.
+ *
+ * One can make a point that using a pointer register as unbounded
+ * SCALAR would be technically acceptable, but this could lead to
+ * pointer leaks because scalars are allowed to leak while pointers
+ * are not. We could make this safe in special cases if root is
+ * calling us, but it's probably not worth the hassle.
+ *
+ * Also, register types that are *not* MAYBE_NULL could technically be
+ * safe to use as their MAYBE_NULL variants (e.g., PTR_TO_MAP_VALUE
+ * is safe to be used as PTR_TO_MAP_VALUE_OR_NULL, provided both point
+ * to the same map).
* However, if the old MAYBE_NULL register then got NULL checked,
* doing so could have affected others with the same id, and we can't
* check for that because we lost the id when we converted to
* a non-MAYBE_NULL variant.
* So, as a general rule we don't allow mixing MAYBE_NULL and
- * non-MAYBE_NULL registers.
+ * non-MAYBE_NULL registers as well.
*/
- if (type_may_be_null(rold->type) != type_may_be_null(rcur->type))
+ if (rold->type != rcur->type)
return false;
switch (base_type(rold->type)) {
@@ -13095,22 +13105,11 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
return true;
if (env->explore_alu_limits)
return false;
- if (rcur->type == SCALAR_VALUE) {
- if (!rold->precise)
- return true;
- /* new val must satisfy old val knowledge */
- return range_within(rold, rcur) &&
- tnum_in(rold->var_off, rcur->var_off);
- } else {
- /* We're trying to use a pointer in place of a scalar.
- * Even if the scalar was unbounded, this could lead to
- * pointer leaks because scalars are allowed to leak
- * while pointers are not. We could make this safe in
- * special cases if root is calling us, but it's
- * probably not worth the hassle.
- */
- return false;
- }
+ if (!rold->precise)
+ return true;
+ /* new val must satisfy old val knowledge */
+ return range_within(rold, rcur) &&
+ tnum_in(rold->var_off, rcur->var_off);
case PTR_TO_MAP_KEY:
case PTR_TO_MAP_VALUE:
/* If the new min/max/var_off satisfy the old ones and
@@ -13122,8 +13121,6 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
check_ids(rold->id, rcur->id, idmap);
case PTR_TO_PACKET_META:
case PTR_TO_PACKET:
- if (rcur->type != rold->type)
- return false;
/* We must have at least as much range as the old ptr
* did, so that any accesses which were safe before are
* still safe. This is true even if old range < old off,
--
2.30.2
next prev parent reply other threads:[~2022-12-23 5:49 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-23 5:49 [PATCH bpf-next 0/7] BPF verifier state equivalence checks improvements Andrii Nakryiko
2022-12-23 5:49 ` [PATCH bpf-next 1/7] bpf: teach refsafe() to take into account ID remapping Andrii Nakryiko
2022-12-23 5:49 ` [PATCH bpf-next 2/7] bpf: reorganize struct bpf_reg_state fields Andrii Nakryiko
2022-12-23 5:49 ` [PATCH bpf-next 3/7] bpf: generalize MAYBE_NULL vs non-MAYBE_NULL rule Andrii Nakryiko
2022-12-23 5:49 ` Andrii Nakryiko [this message]
2022-12-23 5:49 ` [PATCH bpf-next 5/7] bpf: perform byte-by-byte comparison only when necessary in regsafe() Andrii Nakryiko
2022-12-23 5:49 ` [PATCH bpf-next 6/7] bpf: fix regs_exact() logic in regsafe() to remap IDs correctly Andrii Nakryiko
2022-12-23 5:49 ` [PATCH bpf-next 7/7] bpf: unify PTR_TO_MAP_{KEY,VALUE} with default case in regsafe() Andrii Nakryiko
2022-12-28 2:00 ` Alexei Starovoitov
2022-12-29 21:59 ` Andrii Nakryiko
2022-12-30 2:19 ` Alexei Starovoitov
2023-01-03 22:04 ` Andrii Nakryiko
2023-01-04 22:35 ` Alexei Starovoitov
2023-01-04 23:03 ` Andrii Nakryiko
2023-01-05 0:14 ` Alexei Starovoitov
2023-01-11 19:08 ` Andrii Nakryiko
2022-12-28 2:10 ` [PATCH bpf-next 0/7] BPF verifier state equivalence checks improvements patchwork-bot+netdevbpf
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=20221223054921.958283-5-andrii@kernel.org \
--to=andrii@kernel.org \
--cc=ast@kernel.org \
--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 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).