Re: [PATCH hid v12 05/15] HID: bpf jmp table: simplify the logic of cleaning up programs

[Jiri Kosina <jikos@kernel.org>]

On Mon, 12 Dec 2022, Benjamin Tissoires wrote:

> 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.
> 
> An easy "fix" would be to tag bpf_prog_put_deferred() with "noinline",
> but it just feels wrong to have that for this specific reason.
> 
> AFAICT, gcc is not doing that optimisation, but nothing prevents it
> from doing it, and suddenly that will be a big whole in the kernel.

It's not doing it on this particular function, but in general it's 
actually quite common for gcc to inline a function in some callsites and 
leave it out-of-line in others (we are dealing with that in livepatching), 
so I agree it has to be taken into account irrespective of the compiler 
used.

> As much as I wish I had another option, I think for the sake of everyone 
> (and for my future holidays) I'll postpone HID-BPF to 6.3.

Thanks,

-- 
Jiri Kosina
SUSE Labs