Hi Eduard,
Thanks for your kind review!
> On Jun 24, 2025, at 12:30 PM, Eduard Zingerman <eddyz87@xxxxxxxxx> wrote:
[...]
>> ---
>
> I double-checked and backtrack_insn operates as expected indeed.
>
> Note, bpf_reg_state->id has to be reset on BPF_NEG otherwise the
> following is possible:
>
> 4: (bf) r2 = r1 ; R1_w=scalar(id=2,...) R2_w=scalar(id=2,...)
> 5: (87) r1 = -r1 ; R1_w=scalar(id=2,...)
>
> On the master the id is reset by mark_reg_unknown.
> This id is used to transfer range knowledge over all scalars with the
> same id.
I think we should use "__mark_reg_known(dst_reg, 0);" here?
[...]
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_precision.c b/tools/testing/selftests/bpf/progs/verifier_precision.c
>> index 9fe5d255ee37..bcff70f8cebb 100644
>> --- a/tools/testing/selftests/bpf/progs/verifier_precision.c
>> +++ b/tools/testing/selftests/bpf/progs/verifier_precision.c
>
> Nit: tests are usually a separate patch.
CI found that a few existing tests need updating. I will include
fixes to existing tests in the same patch as the kernel changes,
and add new tests in a separate patch.
> @@ -231,4 +231,34 @@ __naked void bpf_cond_op_not_r10(void)
>> ::: __clobber_all);
>> }
>>
>> +SEC("lsm.s/socket_connect")
>> +__success
>> +__naked int bpf_neg_2(void)
>
> Nit: I'd match __log_level(2) output to check the actual range
> inferred by verifier.
I tried __log_level(2). However, this program is so simple that
the verifier log is really simple:
VERIFIER LOG:
=============
processed 3 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
=============
So I didn’t include __log_level(2) here.
>
> Maybe add a test that operates on 64-bit registers?
Will add that in v2.
Thanks,
Song
