On 07/08/17 00:35, Daniel Borkmann wrote: > On 08/03/2017 06:11 PM, Edward Cree wrote: >> Unifies adjusted and unadjusted register value types (e.g. FRAME_POINTER is >> now just a PTR_TO_STACK with zero offset). >> Tracks value alignment by means of tracking known & unknown bits. This >> also replaces the 'reg->imm' (leading zero bits) calculations for (what >> were) UNKNOWN_VALUEs. >> If pointer leaks are allowed, and adjust_ptr_min_max_vals returns -EACCES, >> treat the pointer as an unknown scalar and try again, because we might be >> able to conclude something about the result (e.g. pointer & 0x40 is either >> 0 or 0x40). >> >> Signed-off-by: Edward Cree <ec...@solarflare.com> > [...] >> - dst_reg->max_value = BPF_REGISTER_MAX_RANGE; >> + if (BPF_CLASS(insn->code) != BPF_ALU64) { >> + /* 32-bit ALU ops are (32,32)->64 */ >> + coerce_reg_to_32(dst_reg); >> + coerce_reg_to_32(src_reg); >> } > > Looks like the same check was added twice here right after > the first one? Yes, it must've gotten duplicated when I rebased. Thanks for spotting it! > Shouldn't we just temporarily coerce the src > reg to 32 bit here given in the actual op the src reg is not > being modified? You're quite right, I need to make a copy of the src_reg state and use that, at least in the case where it's a real register. Probably the place to do it is at the call sites in adjust_reg_min_max_vals(). I'll sprinkle a few consts around as well, to catch that sort of thing.
-Ed