Potential race condition in modern_bpf/auxmap

[darsto2]

The auxmap in driver/modern_bpf/helpers/store/auxmap_store_params.h assumes an ebpf program runs exclusively on a CPU without preemption.

But I only see ebpfs run with migrate_disable().
So an ebpf could be preempted after it started writing to the auxmap.
Then another program scheduled on that CPU overwrites the auxmap.
Then the original program is scheduled again and with bpf_ringbuf_output() it copies a malformed event from the auxmap.

Is there some other place where preemption gets disabled? I don't see anything i.e. in sys_exit.

I see also that historically epbf used preempt_disable().
In linux 5.7 it changed to migrate_disable() [1], although at the time migrate_disable() was just an alias for preempt_disable().
In 5.11 migrate_disable() moved away from preempt_disable() [2].

There was an bpf_preempt_disable() added in linux 6.10, quite late.
For 5.1+ there's bpf_spin_lock() that disables preemption.

[1] torvalds/linux@02ad059
[2] torvalds/linux@74d862b

[Andreagit97]

Hi! You raised a good point here, thank you!

You are right, a program can be preempted during execution, and auxmap can be overridden. While theoretically possible, I think it's really hard to see it happen in our scenario. Usually, when a program is preempted, there is an interrupt, and this means that the execution will continue kernel side, so it's hard for the sys_enter/sys_exit tracepoints to be called starting from the interrupt, but probably not impossible. It's also true that we instrument other points like sched_proc_fork/exec/exit, so I cannot exclude that the program interleaving can happen in some strange scenario.

That said, these are all speculations, and I believe we should take care of this possible scenario. One option would be to add an atomic bool for each CPU inside the auxmap structure. Each program sets it to true before starting to write, so that if another program is scheduled on the same CPU, it will not overwrite the content and just terminate. These drops should be monitored with an ad-hoc metric (preemption_drops), so that we can notify the user about drops, and we can keep an eye on how frequent this situation is. If we realize this is a common case, we should probably consider alternative solutions that don't involve dropping events

[poiana]

Issues go stale after 90d of inactivity.

Mark the issue as fresh with /remove-lifecycle stale.

Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Provide feedback via https://github.com/falcosecurity/community.

/lifecycle stale