BPF List
 help / color / mirror / Atom feed
From: Gary Lin <glin@suse.com>
To: unlisted-recipients:; (no To-header on input)
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	netdev@vger.kernel.org, bpf@vger.kernel.org,
	andreas.taschner@suse.com
Subject: [PATCH RFC] bpf, x64: allow not-converged images when BPF_JIT_ALWAYS_ON is set
Date: Fri, 13 Nov 2020 16:38:52 +0800	[thread overview]
Message-ID: <20201113083852.22294-1-glin@suse.com> (raw)

The x64 bpf jit expects the bpf images converge within the given passes.
However there is a corner case:

  l0:     ldh [4]
  l1:     jeq #0x537d, l2, l40
  l2:     ld [0]
  l3:     jeq #0xfa163e0d, l4, l40
  l4:     ldh [12]
  l5:     ldx #0xe
  l6:     jeq #0x86dd, l41, l7
  l7:     jeq #0x800, l8, l41
  l8:     ld [x+16]
  l9:     ja 41

    [... repeated ja 41 ]

  l40:    ja 41
  l41:    ret #0
  l42:    ld #len
  l43:    ret a

The bpf program contains 32 "ja 41" and do_jit() only removes one "ja 41"
right before "l41:  ret #0" for offset==0 in each pass, so
bpf_int_jit_compile() needs to run do_jit() at least 32 times to
eliminate those JMP instructions. Since the current max number of passes
is 20, the bpf program couldn't converge within 20 passes and got rejected
when BPF_JIT_ALWAYS_ON is set even though it's legit as a classic socket
filter.

A not-converged image may be not optimal but at least the bpf
instructions are translated into x64 machine code. Maybe we could just
issue a warning instead so that the program is still loaded and the user
is also notified.

On the other hand, if the size convergence is mandatory, then it
deserves a document to collect the corner cases so that the user could
know the limitations and how to work around them.

Signed-off-by: Gary Lin <glin@suse.com>
---
 arch/x86/net/bpf_jit_comp.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 796506dcfc42..90814c2daaae 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -1972,6 +1972,8 @@ struct x64_jit_data {
 	struct jit_context ctx;
 };
 
+#define MAX_JIT_PASSES 20
+
 struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 {
 	struct bpf_binary_header *header = NULL;
@@ -2042,7 +2044,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 	 * may converge on the last pass. In such case do one more
 	 * pass to emit the final image.
 	 */
-	for (pass = 0; pass < 20 || image; pass++) {
+	for (pass = 0; pass < MAX_JIT_PASSES || image; pass++) {
 		proglen = do_jit(prog, addrs, image, oldproglen, &ctx);
 		if (proglen <= 0) {
 out_image:
@@ -2054,13 +2056,22 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 		}
 		if (image) {
 			if (proglen != oldproglen) {
+#ifdef CONFIG_BPF_JIT_ALWAYS_ON
+				pr_warn("bpf_jit: proglen=%d != oldproglen=%d pass=%d\n",
+					proglen, oldproglen, pass);
+#else
 				pr_err("bpf_jit: proglen=%d != oldproglen=%d\n",
 				       proglen, oldproglen);
 				goto out_image;
+#endif
 			}
 			break;
 		}
+#ifdef CONFIG_BPF_JIT_ALWAYS_ON
+		if (proglen == oldproglen || pass >= (MAX_JIT_PASSES - 1)) {
+#else
 		if (proglen == oldproglen) {
+#endif
 			/*
 			 * The number of entries in extable is the number of BPF_LDX
 			 * insns that access kernel memory via "pointer to BTF type".
-- 
2.28.0


             reply	other threads:[~2020-11-13  8:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-13  8:38 Gary Lin [this message]
2020-11-14  1:48 ` [PATCH RFC] bpf, x64: allow not-converged images when BPF_JIT_ALWAYS_ON is set Alexei Starovoitov
2020-11-16  6:31   ` Gary Lin

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=20201113083852.22294-1-glin@suse.com \
    --to=glin@suse.com \
    --cc=andreas.taschner@suse.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=netdev@vger.kernel.org \
    /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