On 12/07/17 23:07, Nadav Amit wrote: > Edward Cree <ec...@solarflare.com> wrote: >> In this specific case, there was a bug before: if (say) src and dst were >> both unknown bytes (so range 0 to 255), it would compute the new min and max >> to be 0, so it would think the result is known to be 0. But that's wrong, >> because it could be anything from -255 to +255. The bug's implications are >> that it could be used to construct an out-of-range offset to (say) a map >> pointer which the verifier would think was in-range and thus accept. > This sounds like a serious bug that may need to be backported to stable > versions, no? In this case I would assume it should be in a separate patch > so it could be applied separately. Having looked deeper into this in attempting to create a test that the existing verifier would fail, it turns out that in the existing verifier that BPF_SUB handling is dead code. If (for instance) we subtract an UNKNOWN_VALUE from a PTR_TO_MAP_VALUE_ADJ, that code will be run, but afterwards we will mark_reg_unknown_value() the register (bottom of check_alu_op()) making our previous min/max determination irrelevant. So there's nothing to backport, and if I did change this in its own patch, there'd be no way to test it. (I have, however, added a test covering this codepath in the new verifier.)
-Ed