Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 12 Dec 2022 18:08:20 +0100 (CET)
From:   Jiri Kosina <jikos@kernel.org>
To:     Benjamin Tissoires <benjamin.tissoires@redhat.com>
cc:     Greg KH <gregkh@linuxfoundation.org>,
        Jonathan Corbet <corbet@lwn.net>,
        Shuah Khan <shuah@kernel.org>,
        Alexei Starovoitov <alexei.starovoitov@gmail.com>,
        Tero Kristo <tero.kristo@linux.intel.com>,
        linux-kernel@vger.kernel.org, linux-input@vger.kernel.org,
        bpf@vger.kernel.org, linux-kselftest@vger.kernel.org,
        linux-doc@vger.kernel.org
Subject: Re: [PATCH hid v12 05/15] HID: bpf jmp table: simplify the logic of
 cleaning up programs
In-Reply-To: <CAO-hwJ+fYvpD5zbDNq-f-gUEVpxsrdJ7K-ceNd37nLxzBxYL+g@mail.gmail.com>
Message-ID: <nycvar.YFH.7.76.2212121806440.9000@cbobk.fhfr.pm>
References: <20221103155756.687789-1-benjamin.tissoires@redhat.com> <20221103155756.687789-6-benjamin.tissoires@redhat.com> <CAO-hwJ+fYvpD5zbDNq-f-gUEVpxsrdJ7K-ceNd37nLxzBxYL+g@mail.gmail.com>
User-Agent: Alpine 2.21 (LSU 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

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