All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Björn Töpel" <bjorn@kernel.org>
To: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	John Fastabend <john.fastabend@gmail.com>,
	bpf@vger.kernel.org, netdev@vger.kernel.org
Cc: "Björn Töpel" <bjorn@rivosinc.com>,
	"Ilya Leoshkevich" <iii@linux.ibm.com>,
	"Brendan Jackman" <jackmanb@google.com>
Subject: [PATCH bpf] bpf: Proper R0 zero-extension for BPF_CALL instructions
Date: Fri,  2 Dec 2022 11:36:20 +0100	[thread overview]
Message-ID: <20221202103620.1915679-1-bjorn@kernel.org> (raw)

From: Björn Töpel <bjorn@rivosinc.com>

A BPF call instruction can be, correctly, marked with zext_dst set to
true. An example of this can be found in the BPF selftests
progs/bpf_cubic.c:

  ...
  extern __u32 tcp_reno_undo_cwnd(struct sock *sk) __ksym;

  __u32 BPF_STRUCT_OPS(bpf_cubic_undo_cwnd, struct sock *sk)
  {
          return tcp_reno_undo_cwnd(sk);
  }
  ...

which compiles to:
  0:  r1 = *(u64 *)(r1 + 0x0)
  1:  call -0x1
  2:  exit

The call will be marked as zext_dst set to true, and for some backends
(bpf_jit_needs_zext() returns true) expanded to:
  0:  r1 = *(u64 *)(r1 + 0x0)
  1:  call -0x1
  2:  w0 = w0
  3:  exit

The opt_subreg_zext_lo32_rnd_hi32() function which is responsible for
the zext patching, relies on insn_def_regno() to fetch the register to
zero-extend. However, this function does not handle call instructions
correctly, and opt_subreg_zext_lo32_rnd_hi32() fails the verification.

Make sure that R0 is correctly resolved for (BPF_JMP | BPF_CALL)
instructions.

Fixes: 83a2881903f3 ("bpf: Account for BPF_FETCH in insn_has_def32()")
Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
---
I'm not super happy about the additional special case -- first
cmpxchg, and now call. :-( A more elegant/generic solution is welcome!
---
 kernel/bpf/verifier.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 264b3dc714cc..4f9660eafc72 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13386,6 +13386,9 @@ static int opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env,
 		if (!bpf_jit_needs_zext() && !is_cmpxchg_insn(&insn))
 			continue;
 
+		if (insn.code == (BPF_JMP | BPF_CALL))
+			load_reg = BPF_REG_0;
+
 		if (WARN_ON(load_reg == -1)) {
 			verbose(env, "verifier bug. zext_dst is set, but no reg is defined\n");
 			return -EFAULT;

base-commit: 01f856ae6d0ca5ad0505b79bf2d22d7ca439b2a1
-- 
2.37.2


             reply	other threads:[~2022-12-02 10:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-02 10:36 Björn Töpel [this message]
2022-12-06 13:21 ` [PATCH bpf] bpf: Proper R0 zero-extension for BPF_CALL instructions Ilya Leoshkevich
2022-12-06 13:49   ` Björn Töpel
2022-12-06 13:51     ` Ilya Leoshkevich
2022-12-06 17:47   ` Yonghong Song
2022-12-06 18:02     ` Björn Töpel
2022-12-06 18:09     ` Yonghong Song
2022-12-06 18:38       ` Björn Töpel
2022-12-07  1:01         ` Yonghong Song
2022-12-07  7:10           ` Björn Töpel

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=20221202103620.1915679-1-bjorn@kernel.org \
    --to=bjorn@kernel.org \
    --cc=ast@kernel.org \
    --cc=bjorn@rivosinc.com \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=iii@linux.ibm.com \
    --cc=jackmanb@google.com \
    --cc=john.fastabend@gmail.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.