On Thu Jul 16, 2026 at 8:17 AM CEST, Alexis Lothoré wrote:
[...]
>>> In order to introduce KASAN support for eBPF JIT, we need to mark any
>>> load/store instruction that accesses non-stack memory, but updating this
>>> new marking after a patch is not as straightforward as for indirect
>>> calls: the original BPF_ST/BPF_STX/BPF_LDX can be at the beginning, at
>>> the end or somewhere in the middle of the new patch: we then need some
>>> additional info to properly update this marking.
>>
>> I don't think we need to track the exact offset here.
>>
>> It looks like .non_stack_access is set to is_mem_insn(insn + off) in
>> every single case *except* for when a stack access happens to be in
>> the middle of a patch.
>>
>> Given that the cost of getting the flag "wrong" is an unnecessary
>> kasan check only for that case, I think it'll be cleaner to just
>> unconditionally do:
>>
>> data[off].non_stack_access = is_mem_insn(insn + off);
>>
>> in adjust_insn_aux_data()
>>
>> And then the code change can be folded in patch #2
>>
>> The only problem with this I can think of is that in reality *most*
>> stack accesses go trough that special case, which would defeat the
>> purpose of the .non_stack_access flag. This can be checked empirically
>> however.
>
>> Anything else I'm missing here?
>
> Sounds good to me. I'll try to get a rough idea about how often the
> flag, in this scenario, correctly triggers or not.
So this new logic indeed let some "unwanted" cases being instrumented,
like the ldx_patched_on_stack (testing the rnd_hi32 case, so involving
bpf_opt_subreg_zext_lo32_rnd_hi32):
0000000000000710 <ldx_patched_on_stack>:
226: bf a6 00 00 00 00 00 00 r6 = r10
227: 07 06 00 00 f0 ff ff ff r6 += -0x10
228: bf 61 00 00 00 00 00 00 r1 = r6
229: b4 02 00 00 10 00 00 00 w2 = 0x10
230: 85 10 00 00 ff ff ff ff call -0x1
231: 61 a1 f4 ff 00 00 00 00 w1 = *(u32 *)(r10 - 0xc)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Is instrumented, shouldn't
232: 63 1a f4 ff 00 00 00 00 *(u32 *)(r10 - 0xc) = w1
233: bf 61 00 00 00 00 00 00 r1 = r6
234: b4 02 00 00 10 00 00 00 w2 = 0x10
235: 85 10 00 00 ff ff ff ff call -0x1
236: b4 00 00 00 00 00 00 00 w0 = 0x0
237: 95 00 00 00 00 00 00 00 exit
While taking a further look at it, I thought about an intermediate
solution that would prevent the unwanted instrumentation, without
spreading the insn_off_in_patch value I initially proposed. What if:
- in bpf_patch_insn_data, we copy the original instruction being patched
(the one at "off") in a temp var
- we pass this saved insn to adjust_insn_aux_data
- in the loop going over the patched zone, if the current insn matches
the saved insn, we apply back the original non_stack_access flag,
otherwise we systematically mark the insn as "non stack accessing" if
this is a mem insn
Assuming that memcp'ing a struct bpf_insn is enough/correct to check
whether we are processing the original insn or a new one. Some initial
tests in my env correctly reduce the amount of instrumentation for those
stack accesses, without having to edit all bpf_patch_insn_data sites.
Alexis
--
Alexis Lothoré, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
