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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 12 Dec 2022 18:08:20 +0100 (CET)
From:   Jiri Kosina <jikos@...nel.org>
To:     Benjamin Tissoires <benjamin.tissoires@...hat.com>
cc:     Greg KH <gregkh@...uxfoundation.org>,
        Jonathan Corbet <corbet@....net>,
        Shuah Khan <shuah@...nel.org>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Tero Kristo <tero.kristo@...ux.intel.com>,
        linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
        bpf@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-doc@...r.kernel.org
Subject: Re: [PATCH hid v12 05/15] HID: bpf jmp table: simplify the logic of
 cleaning up programs

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