@@ -50,7 +50,14 @@ struct bpf_reg_state {
s32 off;
union {
/* valid when type == PTR_TO_PACKET */
- int range;
+ struct {
+ int range;
+ /* This is used to tag some PTR_TO_PACKET so that they
+ * cannot be compared existing PTR_TO_PACKET with
+ * different pkt_uid.
+ */
+ u32 pkt_uid;
+ };
/* valid when type == CONST_PTR_TO_MAP | PTR_TO_MAP_VALUE |
* PTR_TO_MAP_VALUE_OR_NULL
@@ -712,8 +712,14 @@ static void print_verifier_state(struct bpf_verifier_env *env,
verbose_a("ref_obj_id=%d", reg->ref_obj_id);
if (t != SCALAR_VALUE)
verbose_a("off=%d", reg->off);
- if (type_is_pkt_pointer(t))
+ if (type_is_pkt_pointer(t)) {
verbose_a("r=%d", reg->range);
+ /* pkt_uid is only set for PTR_TO_PACKET, so
+ * type_is_pkt_pointer check is enough.
+ */
+ if (reg->pkt_uid)
+ verbose_a("pkt_uid=%d", reg->pkt_uid);
+ }
else if (base_type(t) == CONST_PTR_TO_MAP ||
base_type(t) == PTR_TO_MAP_KEY ||
base_type(t) == PTR_TO_MAP_VALUE)
@@ -7604,7 +7610,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
if (reg_is_pkt_pointer(ptr_reg)) {
dst_reg->id = ++env->id_gen;
/* something was added to pkt_ptr, set range to zero */
- memset(&dst_reg->raw, 0, sizeof(dst_reg->raw));
+ dst_reg->range = 0;
}
break;
case BPF_SUB:
@@ -7664,7 +7670,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
dst_reg->id = ++env->id_gen;
/* something was added to pkt_ptr, set range to zero */
if (smin_val < 0)
- memset(&dst_reg->raw, 0, sizeof(dst_reg->raw));
+ dst_reg->range = 0;
}
break;
case BPF_AND:
@@ -8701,7 +8707,8 @@ static void __find_good_pkt_pointers(struct bpf_func_state *state,
for (i = 0; i < MAX_BPF_REG; i++) {
reg = &state->regs[i];
- if (reg->type == type && reg->id == dst_reg->id)
+ if (reg->type == type && reg->id == dst_reg->id &&
+ reg->pkt_uid == dst_reg->pkt_uid)
/* keep the maximum range already checked */
reg->range = max(reg->range, new_range);
}
@@ -8709,7 +8716,8 @@ static void __find_good_pkt_pointers(struct bpf_func_state *state,
bpf_for_each_spilled_reg(i, state, reg) {
if (!reg)
continue;
- if (reg->type == type && reg->id == dst_reg->id)
+ if (reg->type == type && reg->id == dst_reg->id &&
+ reg->pkt_uid == dst_reg->pkt_uid)
reg->range = max(reg->range, new_range);
}
}
@@ -9330,6 +9338,14 @@ static void mark_ptr_or_null_regs(struct bpf_verifier_state *vstate, u32 regno,
__mark_ptr_or_null_regs(vstate->frame[i], id, is_null);
}
+static bool is_bad_pkt_comparison(const struct bpf_reg_state *dst_reg,
+ const struct bpf_reg_state *src_reg)
+{
+ if (!reg_is_pkt_pointer_any(dst_reg) || !reg_is_pkt_pointer_any(src_reg))
+ return false;
+ return dst_reg->pkt_uid != src_reg->pkt_uid;
+}
+
static bool try_match_pkt_pointers(const struct bpf_insn *insn,
struct bpf_reg_state *dst_reg,
struct bpf_reg_state *src_reg,
@@ -9343,6 +9359,9 @@ static bool try_match_pkt_pointers(const struct bpf_insn *insn,
if (BPF_CLASS(insn->code) == BPF_JMP32)
return false;
+ if (is_bad_pkt_comparison(dst_reg, src_reg))
+ return false;
+
switch (BPF_OP(insn->code)) {
case BPF_JGT:
if ((dst_reg->type == PTR_TO_PACKET &&
@@ -9640,11 +9659,17 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
mark_ptr_or_null_regs(other_branch, insn->dst_reg,
opcode == BPF_JEQ);
} else if (!try_match_pkt_pointers(insn, dst_reg, ®s[insn->src_reg],
- this_branch, other_branch) &&
- is_pointer_value(env, insn->dst_reg)) {
- verbose(env, "R%d pointer comparison prohibited\n",
- insn->dst_reg);
- return -EACCES;
+ this_branch, other_branch)) {
+ if (is_pointer_value(env, insn->dst_reg)) {
+ verbose(env, "R%d pointer comparison prohibited\n",
+ insn->dst_reg);
+ return -EACCES;
+ }
+ if (is_bad_pkt_comparison(dst_reg, ®s[insn->src_reg])) {
+ verbose(env, "R%d, R%d pkt pointer comparison prohibited\n",
+ insn->dst_reg, insn->src_reg);
+ return -EACCES;
+ }
}
if (env->log.level & BPF_LOG_LEVEL)
print_insn_state(env, this_branch->frame[this_branch->curframe]);
@@ -10891,6 +10916,8 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
/* id relations must be preserved */
if (rold->id && !check_ids(rold->id, rcur->id, idmap))
return false;
+ if (rold->pkt_uid && !check_ids(rold->pkt_uid, rcur->pkt_uid, idmap))
+ return false;
/* new val must satisfy old val knowledge */
return range_within(rold, rcur) &&
tnum_in(rold->var_off, rcur->var_off);
Add a new member in PTR_TO_PACKET specific register state, namely pkt_uid. This is used to classify packet pointers into different sets, and the invariant is that any pkt pointers not belonging to the same set, i.e. not sharing same pkt_uid, won't be allowed for comparison with each other. During range propagation in __find_good_pkt_pointers, we now need to take care to skip packet pointers with a different pkt_uid. This can be used to set for a packet pointer returned from a helper 'bpf_packet_pointer' in the next patch, that encodes the range from the len parameter it gets. Generating a distinct pkt_uid means this pointer cannot be compared with other packet pointers and its range cannot be manipulated. Note that for helpers which change underlying packet data, we don't make any distinction based on pkt_uid for clear_all_pkt_pointers, since even though the pkt_uid is different, they all point into ctx. regsafe is updated to match non-zero pkt_uid using the idmap to ensure it rejects distinct pkt_uid pkt pointers. We also replace memset of reg->raw to set range to 0, but it is helpful to elaborate on why replacing it with reg->range = 0 is correct. In commit 0962590e5533 ("bpf: fix partial copy of map_ptr when dst is scalar"), the copying was changed to use raw so that all possible members of type specific register state are copied, since at that point the type of register is not known. But inside the reg_is_pkt_pointer block, there is no need to memset the whole 'raw' struct, since we also have a pkt_uid member that we now want to preserve after copying from one register to another, for pkt pointers. A test for this case has been included to prevent regressions. Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> --- include/linux/bpf_verifier.h | 9 ++++++- kernel/bpf/verifier.c | 47 ++++++++++++++++++++++++++++-------- 2 files changed, 45 insertions(+), 11 deletions(-)