Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20181123094152.21368-1-ard.biesheuvel@linaro.org>
 <20181123094152.21368-2-ard.biesheuvel@linaro.org> <a3e95c22-fcae-3550-2d3c-4d07eaad95ae@iogearbox.net>
In-Reply-To: <a3e95c22-fcae-3550-2d3c-4d07eaad95ae@iogearbox.net>
From:   Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Fri, 23 Nov 2018 22:12:21 +0100
Message-ID: <CAKv+Gu9SwbXaoBxUGaFSrBocWkjSGY3qMUqFaqK=p_DoGz4tjA@mail.gmail.com>
Subject: Re: [PATCH v3 1/2] bpf: add __weak hook for allocating executable memory
To:     Daniel Borkmann <daniel@iogearbox.net>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Alexei Starovoitov <ast@kernel.org>,
        Rick Edgecombe <rick.p.edgecombe@intel.com>,
        Eric Dumazet <eric.dumazet@gmail.com>,
        Jann Horn <jannh@google.com>,
        Kees Cook <keescook@chromium.org>,
        Jessica Yu <jeyu@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will.deacon@arm.com>,
        Mark Rutland <mark.rutland@arm.com>,
        "David S. Miller" <davem@davemloft.net>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        "<netdev@vger.kernel.org>" <netdev@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, 23 Nov 2018 at 22:07, Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 11/23/2018 10:41 AM, Ard Biesheuvel wrote:
> > By default, BPF uses module_alloc() to allocate executable memory,
> > but this is not necessary on all arches and potentially undesirable
> > on some of them.
> >
> > So break out the module_alloc() and module_memfree() calls into __weak
> > functions to allow them to be overridden in arch code.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  kernel/bpf/core.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index 1a796e0799ec..572dd74c26e3 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -609,6 +609,16 @@ static void bpf_jit_uncharge_modmem(u32 pages)
> >       atomic_long_sub(pages, &bpf_jit_current);
> >  }
> >
> > +void *__weak bpf_jit_alloc_exec(unsigned long size)
> > +{
> > +     return module_alloc(size);
> > +}
> > +
> > +void __weak bpf_jit_free_exec(const void *addr)
> > +{
> > +     module_memfree(addr);
> > +}
> > +
> >  struct bpf_binary_header *
> >  bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
> >                    unsigned int alignment,
> > @@ -626,7 +636,7 @@ bpf_jit_binary_alloc(unsigned int proglen, u8 **ima=
ge_ptr,
> >
> >       if (bpf_jit_charge_modmem(pages))
> >               return NULL;
> > -     hdr =3D module_alloc(size);
> > +     hdr =3D bpf_jit_alloc_exec(size);
> >       if (!hdr) {
> >               bpf_jit_uncharge_modmem(pages);
> >               return NULL;
> > @@ -650,7 +660,7 @@ void bpf_jit_binary_free(struct bpf_binary_header *=
hdr)
> >  {
> >       u32 pages =3D hdr->pages;
> >
> > -     module_memfree(hdr);
> > +     bpf_jit_free_exec(hdr);
> >       bpf_jit_uncharge_modmem(pages);
>
> The const needs to be removed, this generates the following warning:
>
> # make -j8 kernel/bpf/
>   DESCEND  objtool
>   CALL    scripts/checksyscalls.sh
>   CC      kernel/bpf/core.o
>   CC      kernel/bpf/syscall.o
>   CC      kernel/bpf/verifier.o
>   CC      kernel/bpf/hashtab.o
>   CC      kernel/bpf/helpers.o
>   CC      kernel/bpf/inode.o
>   CC      kernel/bpf/arraymap.o
>   CC      kernel/bpf/lpm_trie.o
> kernel/bpf/core.c: In function =E2=80=98bpf_jit_free_exec=E2=80=99:
> kernel/bpf/core.c:619:17: warning: passing argument 1 of =E2=80=98module_=
memfree=E2=80=99 discards =E2=80=98const=E2=80=99 qualifier from pointer ta=
rget type [-Wdiscarded-qualifiers]
>   module_memfree(addr);
>                  ^~~~
> In file included from kernel/bpf/core.c:28:0:
> ./include/linux/moduleloader.h:30:6: note: expected =E2=80=98void *=E2=80=
=99 but argument is of type =E2=80=98const void *=E2=80=99
>  void module_memfree(void *module_region);
>       ^~~~~~~~~~~~~~
>   CC      kernel/bpf/local_storage.o
> [...]
>
> Please respin with that fixed.
>

OK.

I'll respin the second patch as well, and move the BPF window to the
start of the vmalloc region. However, the arm64 maintainers need to
ack that as well since it modifies the layout of the kernel virtual
address space.