Make default case in regsafe() safer. Instead of doing byte-by-byte
comparison, take into account ID remapping and also range and var_off
checks. For most of registers range and var_off will be zero (not set),
which doesn't matter. For some, like PTR_TO_MAP_{KEY,VALUE}, this
generalized logic is exactly matching what regsafe() was doing as
a special case. But in any case, if register has id and/or ref_obj_id
set, check it using check_ids() logic, taking into account idmap.
With these changes, default case should be handling most registers more
correctly, and even for future register would be a good default. For some
special cases, like PTR_TO_PACKET, one would still need to implement extra
checks (like special handling of reg->range) to get better state
equivalence, but default logic shouldn't be wrong.
With these changes veristat shows no difference in verifier stats across
selftests and Cilium BPF programs (which is a good thing).
Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
---
kernel/bpf/verifier.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b23812d2bb49..7e0fa3924f01 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12919,8 +12919,8 @@ static int check_btf_info(struct bpf_verifier_env *env,
}
/* check %cur's range satisfies %old's */
-static bool range_within(struct bpf_reg_state *old,
- struct bpf_reg_state *cur)
+static bool range_within(const struct bpf_reg_state *old,
+ const struct bpf_reg_state *cur)
{
return old->umin_value <= cur->umin_value &&
old->umax_value >= cur->umax_value &&
@@ -13073,6 +13073,17 @@ static bool regs_exact(const struct bpf_reg_state *rold,
check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
}
+static bool regs_equal(const struct bpf_reg_state *rold,
+ const struct bpf_reg_state *rcur,
+ struct bpf_id_pair *idmap)
+{
+ return memcmp(rold, rcur, offsetof(struct bpf_reg_state, var_off)) == 0 &&
+ range_within(rold, rcur) &&
+ tnum_in(rold->var_off, rcur->var_off) &&
+ check_ids(rold->id, rcur->id, idmap) &&
+ check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
+}
+
/* Returns true if (rold safe implies rcur safe) */
static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
struct bpf_reg_state *rcur, struct bpf_id_pair *idmap)
@@ -13121,15 +13132,6 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
/* 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
- * everything else matches, we are OK.
- */
- return memcmp(rold, rcur, offsetof(struct bpf_reg_state, var_off)) == 0 &&
- range_within(rold, rcur) &&
- tnum_in(rold->var_off, rcur->var_off) &&
- check_ids(rold->id, rcur->id, idmap);
case PTR_TO_PACKET_META:
case PTR_TO_PACKET:
/* We must have at least as much range as the old ptr
@@ -13157,7 +13159,7 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
*/
return regs_exact(rold, rcur, idmap) && rold->frameno == rcur->frameno;
default:
- return regs_exact(rold, rcur, idmap);
+ return regs_equal(rold, rcur, idmap);
}
}
--
2.30.2
