From: Will Deacon <will@kernel.org>
To: Luke Nelson <luke.r.nels@gmail.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
Song Liu <songliubraving@fb.com>,
clang-built-linux@googlegroups.com,
Daniel Borkmann <daniel@iogearbox.net>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Marc Zyngier <maz@kernel.org>,
John Fastabend <john.fastabend@gmail.com>,
Alexei Starovoitov <ast@kernel.org>,
open list <linux-kernel@vger.kernel.org>,
Networking <netdev@vger.kernel.org>,
KP Singh <kpsingh@chromium.org>,
Luke Nelson <lukenels@cs.washington.edu>,
Thomas Gleixner <tglx@linutronix.de>,
linux-arm-kernel@lists.infradead.org,
Catalin Marinas <catalin.marinas@arm.com>,
Zi Shen Lim <zlim.lnx@gmail.com>, Yonghong Song <yhs@fb.com>,
bpf <bpf@vger.kernel.org>, Andrii Nakryiko <andriin@fb.com>,
Martin KaFai Lau <kafai@fb.com>, Xi Wang <xi.wang@gmail.com>
Subject: Re: [RFC PATCH bpf-next 1/3] arm64: insn: Fix two bugs in encoding 32-bit logical immediates
Date: Fri, 8 May 2020 12:47:10 +0100 [thread overview]
Message-ID: <20200508114709.GB16247@willie-the-truck> (raw)
In-Reply-To: <CAB-e3NRCJ_4+vkFPkMN67DwBBtO=sJwR-oL4-AozVw2bBJHOzg@mail.gmail.com>
On Thu, May 07, 2020 at 02:48:07PM -0700, Luke Nelson wrote:
> Thanks for the comments! Responses below:
>
> > It's a bit grotty spreading the checks out now. How about we tweak things
> > slightly along the lines of:
> >
> >
> > diff --git a/arch/arm64/kernel/insn.c b/arch/arm64/kernel/insn.c
> > index 4a9e773a177f..60ec788eaf33 100644
> > --- a/arch/arm64/kernel/insn.c
> > +++ b/arch/arm64/kernel/insn.c
> > [...]
>
> Agreed; this new version looks much cleaner. I re-ran all the tests /
> verification and everything seems good. Would you like me to submit a
> v2 of this series with this new code?
Yes, please! And please include Daniel's acks on the BPF changes too. It's a
public holiday here in the UK today, but I can pick this up next week.
> >> We tested the new code against llvm-mc with all 1,302 encodable 32-bit
> >> logical immediates and all 5,334 encodable 64-bit logical immediates.
> >
> > That, on its own, is awesome information. Do you have any pointer on
> > how to set this up?
>
> Sure! The process of running the tests is pretty involved, but I'll
> describe it below and give some links here.
>
> We found the bugs in insn.c while adding support for logical immediates
> to the BPF JIT and verifying the changes with our tool, Serval:
> https://github.com/uw-unsat/serval-bpf. The basic idea for how we tested /
> verified logical immediates is the following:
>
> First, we have a Python script [1] for generating every encodable
> logical immediate and the corresponding instruction fields that encode
> that immediate. The script validates the list by checking that llvm-mc
> decodes each instruction back to the expected immediate.
>
> Next, we use the list [2] from the first step to check a Racket
> translation [3] of the logical immediate encoding function in insn.c.
> We found the second mask bug by noticing that some (encodable) 32-bit
> immediates were being rejected by the encoding function.
>
> Last, we use the Racket translation of the encoding function to verify
> the correctness of the BPF JIT implementation [4], i.e., the JIT
> correctly compiles BPF_{AND,OR,XOR,JSET} BPF_K instructions to arm64
> instructions with equivalent semantics. We found the first bug as the
> verifier complained that the function was producing invalid encodings
> for 32-bit -1 immediates, and we were able to reproduce a kernel crash
> using the BPF tests.
>
> We manually translated the C code to Racket because our verifier, Serval,
> currently only works on Racket code.
Nice! Two things:
(1) I really think you should give a talk on this at a Linux conference
(2) Did you look at any instruction generation functions other than the
logical immediate encoding function?
Cheers,
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-05-08 11:47 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-07 1:05 [RFC PATCH bpf-next 0/3] arm64 BPF JIT Optimizations Luke Nelson
2020-05-07 1:05 ` [RFC PATCH bpf-next 1/3] arm64: insn: Fix two bugs in encoding 32-bit logical immediates Luke Nelson
2020-05-07 8:19 ` Marc Zyngier
2020-05-07 8:29 ` Will Deacon
2020-05-07 9:12 ` Marc Zyngier
2020-05-07 21:48 ` Luke Nelson
2020-05-08 11:47 ` Will Deacon [this message]
2020-05-08 18:12 ` Luke Nelson
2020-05-07 1:05 ` [RFC PATCH bpf-next 2/3] bpf, arm64: Optimize AND, OR, XOR, JSET BPF_K using arm64 " Luke Nelson
2020-05-07 20:19 ` [RFC PATCH bpf-next 2/3] bpf, arm64: Optimize AND,OR,XOR,JSET " Daniel Borkmann
2020-05-07 1:05 ` [RFC PATCH bpf-next 3/3] bpf, arm64: Optimize ADD, SUB, JMP BPF_K using arm64 add/sub immediates Luke Nelson
2020-05-07 20:22 ` [RFC PATCH bpf-next 3/3] bpf, arm64: Optimize ADD,SUB,JMP " Daniel Borkmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200508114709.GB16247@willie-the-truck \
--to=will@kernel.org \
--cc=andriin@fb.com \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=catalin.marinas@arm.com \
--cc=clang-built-linux@googlegroups.com \
--cc=daniel@iogearbox.net \
--cc=gregkh@linuxfoundation.org \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kpsingh@chromium.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luke.r.nels@gmail.com \
--cc=lukenels@cs.washington.edu \
--cc=mark.rutland@arm.com \
--cc=maz@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=tglx@linutronix.de \
--cc=xi.wang@gmail.com \
--cc=yhs@fb.com \
--cc=zlim.lnx@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).