On 12/12/22 10:20 AM, Greg KH wrote:
On Mon, Dec 12, 2022 at 09:52:03AM -0800, Yonghong Song wrote:
On 12/12/22 9:02 AM, Benjamin Tissoires wrote:
On Thu, Nov 3, 2022 at 4:58 PM Benjamin Tissoires
<benjamin.tissoires@xxxxxxxxxx> wrote:
Kind of a hack, but works for now:
Instead of listening for any close of eBPF program, we now
decrement the refcount when we insert it in our internal
map of fd progs.
This is safe to do because:
- we listen to any call of destructor of programs
- when a program is being destroyed, we disable it by removing
it from any RCU list used by any HID device (so it will never
be called)
- we then trigger a job to cleanup the prog fd map, but we overwrite
the removal of the elements to not do anything on the programs, just
remove the allocated space
This is better than previously because we can remove the map of known
programs and their usage count. We now rely on the refcount of
bpf, which has greater chances of being accurate.
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx>
---
So... I am a little bit embarrassed, but it turns out that this hack
is not safe enough.
If I compile the kernel with LLVM=1, the function
bpf_prog_put_deferred() is optimized in a weird way: if we are not in
irq, the function is inlined into __bpf_prog_put(), but if we are, the
function is still kept around as it is called in a scheduled work
item.
This is something I completely overlooked: I assume that if the
function would be inlined, the HID entrypoint BPF preloaded object
would not be able to bind, thus deactivating HID-BPF safely. But if a
function can be both inlined and not inlined, then I have no
guarantees that my cleanup call will be called. Meaning that a HID
device might believe there is still a bpf function to call. And things
will get messy, with kernel crashes and others.
You should not rely fentry to a static function. This is unstable
as compiler could inline it if that static function is called
directly. You could attach to a global function if it is not
compiled with lto.
But now that the kernel does support LTO, how can you be sure this will
always work properly? The code author does not know if LTO will kick in
and optimize this away or not, that's the linker's job.
Ya, that is right. So for in-kernel bpf programs, attaching to global
functions are not safe either. For other not-in-kernel bpf programs, it
may not work but that is user's responsibility to adjust properly
(to different functions based on a particular build, etc.).
thanks,
greg k-h
