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;
MIME-Version: 1.0
References: <00000000000076f3a305e97e9229@google.com> <a68d118d-ee03-399c-df02-82848e2197a2@intel.com>
In-Reply-To: <a68d118d-ee03-399c-df02-82848e2197a2@intel.com>
From:   Alexei Starovoitov <alexei.starovoitov@gmail.com>
Date:   Sun, 25 Sep 2022 14:55:46 -0700
Message-ID: <CAADnVQ+SpNuUSRFte2Lm13QZiTXcWfn2eZw5Q+MP0SKwuJEXFg@mail.gmail.com>
Subject: Re: [syzbot] WARNING in __change_page_attr_set_clr
To:     Dave Hansen <dave.hansen@intel.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        tech-board@lists.linuxfoundation.org,
        Song Liu <songliubraving@fb.com>,
        Kernel Team <Kernel-team@fb.com>
Cc:     Borislav Petkov <bp@alien8.de>, brijesh.singh@amd.com,
        Dan Williams <dan.j.williams@intel.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        "H. Peter Anvin" <hpa@zytor.com>, jane.chu@oracle.com,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Andy Lutomirski <luto@kernel.org>,
        Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>, seanjc@google.com,
        syzkaller-bugs <syzkaller-bugs@googlegroups.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        X86 ML <x86@kernel.org>, Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        John Fastabend <john.fastabend@gmail.com>,
        "open list:BPF (Safe dynamic programs and tools)" 
        <bpf@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sun, Sep 25, 2022 at 9:44 AM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 9/25/22 04:18, syzbot wrote:
> > ------------[ cut here ]------------
> > CPA refuse W^X violation: 8000000000000163 -> 0000000000000163 range: 0xffffffffa0401000 - 0xffffffffa0401fff PFN 7d8d5
> > WARNING: CPU: 0 PID: 3607 at arch/x86/mm/pat/set_memory.c:600 verify_rwx arch/x86/mm/pat/set_memory.c:600 [inline]
> > WARNING: CPU: 0 PID: 3607 at arch/x86/mm/pat/set_memory.c:600 __change_page_attr arch/x86/mm/pat/set_memory.c:1569 [inline]
> > WARNING: CPU: 0 PID: 3607 at arch/x86/mm/pat/set_memory.c:600 __change_page_attr_set_clr+0x1f40/0x2020 arch/x86/mm/pat/set_memory.c:1691
> > Modules linked in:
>
> Yay, one of these that isn't due to wonky 32-bit kernels!
>
> This one looks to be naughty intentionally:
>
> > void *bpf_jit_alloc_exec_page(void)
> > {
> ...
> >         /* Keep image as writeable. The alternative is to keep flipping ro/rw
> >          * every time new program is attached or detached.
> >          */
> >         set_memory_x((long)image, 1);
> >         return image;
> > }
>
> For STRICT_KERNEL_RWX kernels, I think we would really rather that this
> code *did* flip ro/rw every time a new BPF program is attached or detached.

Steven Rostedt noticed that comment around the middle of August
and told you and Peter about it.
Then Peter added a WARN_ONCE in commit
https://lore.kernel.org/all/YwySW3ROc21hN7g9@hirez.programming.kicks-ass.net/
to explicitly trigger that known issue.
Sure enough the fedora fails to boot on linux-next since then,
because systemd is loading bpf programs that use bpf trampoline.
The boot issue was was reported 3 days ago:
https://lore.kernel.org/bpf/c84cc27c1a5031a003039748c3c099732a718aec.camel@kernel.org/T/#u
Now we're trying to urgently address it with:
https://lore.kernel.org/bpf/20220923211837.3044723-1-song@kernel.org/

So instead of pinging us with your w^x concern you've decided
to fail hard in -next to force the issue and
now acting like this is something surprising to you?!

This is Code of Conduct "worthy" behavior demonstrated
by a newly elected member of the Technical Advisory Board.
Please consider resigning.
A TAB member should be better than this.

As far as this w^x issue...
we've communicated back in May 2022 (sorry I cannot find the link
to that discussion) that bpf_prog_pack is targeting to be used
by everything bpf. Currently by bpf progs only.
bpf trampoline and bpf dispatcher were next on the list.
But then folks expressed the desire to generalize bpf_prog_pack for
everything: bpf, modules and all other trampolines.
We've posted multiple revisions and kept pinging for feedback.
The last one on Aug 24:
https://lkml.org/lkml/2022/8/24/857
If/when the generic vmalloc_exec lands we can finally close
the issue for modules, bpf progs, and trampolines of all kinds.
Make them fast by using large pages and w^x compliant.

And, sorry, "flip ro/rw every time" is not a good idea from
security pov.
There is a much better solution that stalled on the code review.
In the meantime we'll land a quick fix to re-enable boot in -next
in the coming days.