On Thu, Dec 29, 2022 at 5:07 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
> @@ -11959,7 +11956,10 @@ static int check_ld_imm(struct bpf_verifier_env *env, struct bpf_insn *insn)
> dst_reg->type = PTR_TO_MAP_VALUE;
> dst_reg->off = aux->map_off;
> WARN_ON_ONCE(map->max_entries != 1);
> - /* We want reg->id to be same (0) as map_value is not distinct */
> + /* map->id is positive s32. Negative map->id will not collide with env->id_gen.
> + * This lets us track active_lock state as single u32 instead of pair { map|btf, id }
> + */
> + dst_reg->id = -map->id;
Here is what I meant in my earlier reply to Dave's patches 1 and 2.
imo this is a simpler implementation of the same logic.
The only tricky part is above bit that is necessary
to use a single u32 in bpf_reg_state to track active_lock.
We can follow up with clean up of active_lock comparison
in other places of the verifier.
Instead of:
if (map)
cur->active_lock.ptr = map;
else
cur->active_lock.ptr = btf;
cur->active_lock.id = reg->id;
it will be:
cur->active_lock_id = reg->id;
Another cleanup is needed to compare active_lock_id properly
in regsafe().
Thoughts?
