Re: [PATCH 6.1.y 2/2] bpf: Defer work in bpf_timer_cancel_and_free

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: Use-After-Free in bpf_timer_cb() due to deferred cancellation
--

commit 12eef63fe47026afa8702d0bccb135f5bc2594fe
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Defer work in bpf_timer_cancel_and_free

This commit addresses a deadlock and potential use-after-free situation in
the BPF timer subsystem by deferring timer cancellation to a workqueue when
invoked from within a timer callback context.

Link: https://lore.kernel.org/r/20240709185440.1104957-3-memxor@gmail.com

[Severity: High]
Does this deferred cancellation introduce a use-after-free when map elements
are freed synchronously?

Looking at kernel/bpf/helpers.c in bpf_timer_cancel_and_free() (around line
1476), the code defers the cancellation by calling queue_work() if
this_cpu_read(hrtimer_running) is true.

However, if a BPF program executing in a timer callback deletes a map
element containing another timer, the deletion path in kernel/bpf/hashtab.c
(in free_htab_elem) calls bpf_timer_cancel_and_free(). Because the
cancellation is now deferred, free_htab_elem() continues and synchronously
frees the map element, for example by pushing it to the PCPU freelist for
preallocated maps.

If the deferred timer is currently running on another CPU or fires before
the workqueue executes, bpf_timer_cb() in kernel/bpf/helpers.c (around line
1162) will execute. It extracts the value pointer from the timer and passes
it to the BPF program callback:

callback_fn((u64)(long)map, (u64)(long)key, (u64)(long)value, 0, 0);

Since the map element containing this value was already freed by
free_htab_elem(), could this allow the BPF program to read or write freed
memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002193020.19392-1-artem@trailofbits.com?part=2