On Mon, Nov 23, 2020 at 10:52:57PM -0800, Alexei Starovoitov wrote: > On Mon, Nov 23, 2020 at 05:32:00PM +0000, Brendan Jackman wrote: > > @@ -3644,8 +3649,21 @@ static int check_atomic(struct bpf_verifier_env *env, int insn_idx, struct bpf_i > > return err; > > > > /* check whether we can write into the same memory */ > > - return check_mem_access(env, insn_idx, insn->dst_reg, insn->off, > > - BPF_SIZE(insn->code), BPF_WRITE, -1, true); > > + err = check_mem_access(env, insn_idx, insn->dst_reg, insn->off, > > + BPF_SIZE(insn->code), BPF_WRITE, -1, true); > > + if (err) > > + return err; > > + > > + if (!(insn->imm & BPF_FETCH)) > > + return 0; > > + > > + /* check and record load of old value into src reg */ > > + err = check_reg_arg(env, insn->src_reg, DST_OP); > > + if (err) > > + return err; > > + regs[insn->src_reg].type = SCALAR_VALUE; > > check_reg_arg() will call mark_reg_unknown() which will set type to SCALAR_VALUE. > What is the point of another assignment? Yep, this is just an oversight - thanks, will remove.
