Re: WARNING&Oops in v6.6.37 on ppc64lea - Trying to vfree() bad address (00000000453be747)

[Michael Ellerman <mpe@ellerman.id.au>]

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Tue, Jul 09, 2024 at 03:02:13PM +0530, Naveen N Rao wrote:
>> Greg Kroah-Hartman wrote:
>> > On Mon, Jul 08, 2024 at 11:16:48PM -0400, matoro wrote:
>> > > On 2024-07-05 16:34, Vitaly Chikunov wrote:
>> > > > Hi,
>> > > > > There is new WARNING and Oops on ppc64le in v6.6.37 when running
>> > > LTP tests:
>> > > > bpf_prog01, bpf_prog02, bpf_prog04, bpf_prog05, prctl04. Logs excerpt
>> > > > below. I
>> > > > see there is 1 commit in v6.6.36..v6.6.37 with call to
>> > > > bpf_jit_binary_pack_finalize, backported from 5 patch mainline patchset:
>> > > > >   f99feda5684a powerpc/bpf: use
>> > > bpf_jit_binary_pack_[alloc|finalize|free]
>> > > >
>> 
>> <snip>
>> 
>> > > > > And so on. Temporary build/test log is at
>> > > > https://git.altlinux.org/tasks/352218/build/100/ppc64le/log
>> > > > > Other stable/longterm branches or other architectures does not
>> > > exhibit this.
>> > > > > Thanks,
>> > > 
>> > > Hi all - this just took down a production server for me, on POWER9 bare
>> > > metal.  Not running tests, just booting normally, before services even came
>> > > up.  Had to perform manual restoration, reverting to 6.6.36 worked.  Also
>> > > running 64k kernel, unsure if it's better on 4k kernel.
>> > > 
>> > > In case it's helpful, here's the log from my boot:
>> > > https://dpaste.org/Gyxxg/raw
>> > 
>> > Ok, this isn't good, something went wrong with my backports here.  Let
>> > me go revert them all and push out a new 6.6.y release right away.
>> 
>> I think the problem is that the series adding support for bpf prog_pack was
>> partially backported. In particular, the below patches are missing from
>> stable v6.6:
>> 465cabc97b42 powerpc/code-patching: introduce patch_instructions()
>> 033ffaf0af1f powerpc/bpf: implement bpf_arch_text_invalidate for bpf_prog_pack
>> 6efc1675acb8 powerpc/bpf: implement bpf_arch_text_copy
>> 
>> It should be sufficient to revert commit f99feda5684a (powerpc/bpf: use
>> bpf_jit_binary_pack_[alloc|finalize|free]) to allow the above to apply
>> cleanly, followed by cherry picking commit 90d862f370b6 (powerpc/bpf: use
>> bpf_jit_binary_pack_[alloc|finalize|free]) from upstream.
>> 
>> Alternately, commit f99feda5684a (powerpc/bpf: use
>> bpf_jit_binary_pack_[alloc|finalize|free]) can be reverted.
>
> I'm dropping them all now, if you want to submit a working series for
> this, I'll be glad to queue them all up.

Thanks, revert is good for now.

With the revert there will be a build warning/error, only in stable,
which I think can be fixed with the diff below. I'll get it tested and
submit it properly.

cheers

diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 5f57a8ba3cc8..cdd9db8f8684 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -205,7 +205,10 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)

        bpf_flush_icache(bpf_hdr, (u8 *)bpf_hdr + bpf_hdr->size);
        if (!fp->is_func || extra_pass) {
-               bpf_jit_binary_lock_ro(bpf_hdr);
+               if (bpf_jit_binary_lock_ro(bpf_hdr)) {
+                       fp = org_fp;
+                       goto out_addrs;
+               }
                bpf_prog_fill_jited_linfo(fp, addrs);
 out_addrs:
                kfree(addrs);