From: Jean-Philippe Brucker <jean-philippe@linaro.org>
To: Will Deacon <will@kernel.org>
Cc: Song Liu <songliubraving@fb.com>,
Zi Shen Lim <zlim.lnx@gmail.com>,
Alexei Starovoitov <ast@kernel.org>,
ardb@kernel.org, Daniel Borkmann <daniel@iogearbox.net>,
naresh.kamboju@linaro.org,
John Fastabend <john.fastabend@gmail.com>,
Catalin Marinas <catalin.marinas@arm.com>,
Jakub Kicinski <kuba@kernel.org>,
Andrii Nakryiko <andriin@fb.com>,
Jesper Dangaard Brouer <hawk@kernel.org>,
Yonghong Song <yhs@fb.com>, KP Singh <kpsingh@chromium.org>,
linux-arm-kernel@lists.infradead.org,
Yauheni Kaliuta <yauheni.kaliuta@redhat.com>,
netdev@vger.kernel.org,
Ilias Apalodimas <ilias.apalodimas@linaro.org>,
linux-kernel@vger.kernel.org,
"David S. Miller" <davem@davemloft.net>,
Jiri Olsa <jolsa@kernel.org>,
bpf@vger.kernel.org, Martin KaFai Lau <kafai@fb.com>
Subject: Re: [PATCH v2] arm64: bpf: Fix branch offset in JIT
Date: Tue, 15 Sep 2020 15:54:19 +0200 [thread overview]
Message-ID: <20200915135419.GB1748187@myrica> (raw)
In-Reply-To: <20200915131102.GA26439@willie-the-truck>
On Tue, Sep 15, 2020 at 02:11:03PM +0100, Will Deacon wrote:
> > ret = build_insn(insn, ctx, extra_pass);
> > if (ret > 0) {
> > i++;
> > if (ctx->image == NULL)
> > - ctx->offset[i] = ctx->idx;
> > + ctx->offset[i] = ctx->offset[i - 1];
>
> Does it matter that we set the offset for both halves of a 16-byte BPF
> instruction? I think that's a change in behaviour here.
After testing this patch a bit, I think setting only the first slot should
be sufficient, and we can drop these two lines. It does make a minor
difference, because although the BPF verifier normally rejects a program
that jumps into the middle of a 16-byte instruction, it can validate it in
some cases:
BPF_LD_IMM64(BPF_REG_0, 2) // 16-byte immediate load
BPF_JMP_IMM(BPF_JLE, BPF_REG_0, 1, -2) // If r0 <= 1, branch to
BPF_EXIT_INSN() // the middle of the insn
The verifier detects that the condition is always false and doesn't follow
the branch, hands the program to the JIT. So if we don't set the second
slot, then we generate an invalid branch offset. But that doesn't really
matter as the branch is never taken.
Thanks,
Jean
_______________________________________________
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-09-15 13:56 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-14 16:03 [PATCH v2] arm64: bpf: Fix branch offset in JIT Ilias Apalodimas
2020-09-15 13:11 ` Will Deacon
2020-09-15 13:53 ` Ilias Apalodimas
2020-09-15 14:17 ` Will Deacon
2020-09-15 19:23 ` Ilias Apalodimas
2020-09-16 12:39 ` Yauheni Kaliuta
2020-09-16 13:17 ` Jean-Philippe Brucker
2020-09-16 13:45 ` Yauheni Kaliuta
2020-09-15 13:54 ` Jean-Philippe Brucker [this message]
2020-09-16 16:04 ` Ilias Apalodimas
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=20200915135419.GB1748187@myrica \
--to=jean-philippe@linaro.org \
--cc=andriin@fb.com \
--cc=ardb@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=catalin.marinas@arm.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=hawk@kernel.org \
--cc=ilias.apalodimas@linaro.org \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kafai@fb.com \
--cc=kpsingh@chromium.org \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=naresh.kamboju@linaro.org \
--cc=netdev@vger.kernel.org \
--cc=songliubraving@fb.com \
--cc=will@kernel.org \
--cc=yauheni.kaliuta@redhat.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).