Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH bpf] bpf: Fix clobbering of r2 in bpf_gen_ld_abs
@ 2020-09-07 22:04 Daniel Borkmann
  2020-09-08 16:20 ` Alexei Starovoitov
  0 siblings, 1 reply; 2+ messages in thread
From: Daniel Borkmann @ 2020-09-07 22:04 UTC (permalink / raw)
  To: ast; +Cc: bryce.kahle, netdev, bpf, Daniel Borkmann

Bryce reported that he saw the following with:

  0:  r6 = r1
  1:  r1 = 12
  2:  r0 = *(u16 *)skb[r1]

The xlated sequence was incorrectly clobbering r2 with pointer
value of r6 ...

  0: (bf) r6 = r1
  1: (b7) r1 = 12
  2: (bf) r1 = r6
  3: (bf) r2 = r1
  4: (85) call bpf_skb_load_helper_16_no_cache#7692160

... and hence call to the load helper never succeeded given the
offset was too high. Fix it by reordering the load of r6 to r1.

Other than that the insn has similar calling convention than BPF
helpers, that is, r0 - r5 are scratch regs, so nothing else
affected after the insn.

Fixes: e0cea7ce988c ("bpf: implement ld_abs/ld_ind in native bpf")
Reported-by: Bryce Kahle <bryce.kahle@datadoghq.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
 net/core/filter.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/filter.c b/net/core/filter.c
index b2df52086445..2d62c25e0395 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7065,8 +7065,6 @@ static int bpf_gen_ld_abs(const struct bpf_insn *orig,
 	bool indirect = BPF_MODE(orig->code) == BPF_IND;
 	struct bpf_insn *insn = insn_buf;
 
-	/* We're guaranteed here that CTX is in R6. */
-	*insn++ = BPF_MOV64_REG(BPF_REG_1, BPF_REG_CTX);
 	if (!indirect) {
 		*insn++ = BPF_MOV64_IMM(BPF_REG_2, orig->imm);
 	} else {
@@ -7074,6 +7072,8 @@ static int bpf_gen_ld_abs(const struct bpf_insn *orig,
 		if (orig->imm)
 			*insn++ = BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, orig->imm);
 	}
+	/* We're guaranteed here that CTX is in R6. */
+	*insn++ = BPF_MOV64_REG(BPF_REG_1, BPF_REG_CTX);
 
 	switch (BPF_SIZE(orig->code)) {
 	case BPF_B:
-- 
2.21.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH bpf] bpf: Fix clobbering of r2 in bpf_gen_ld_abs
  2020-09-07 22:04 [PATCH bpf] bpf: Fix clobbering of r2 in bpf_gen_ld_abs Daniel Borkmann
@ 2020-09-08 16:20 ` Alexei Starovoitov
  0 siblings, 0 replies; 2+ messages in thread
From: Alexei Starovoitov @ 2020-09-08 16:20 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Alexei Starovoitov, bryce.kahle, Network Development, bpf

On Mon, Sep 7, 2020 at 3:04 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> Bryce reported that he saw the following with:
>
>   0:  r6 = r1
>   1:  r1 = 12
>   2:  r0 = *(u16 *)skb[r1]
>
> The xlated sequence was incorrectly clobbering r2 with pointer
> value of r6 ...
>
>   0: (bf) r6 = r1
>   1: (b7) r1 = 12
>   2: (bf) r1 = r6
>   3: (bf) r2 = r1
>   4: (85) call bpf_skb_load_helper_16_no_cache#7692160
>
> ... and hence call to the load helper never succeeded given the
> offset was too high. Fix it by reordering the load of r6 to r1.
>
> Other than that the insn has similar calling convention than BPF
> helpers, that is, r0 - r5 are scratch regs, so nothing else
> affected after the insn.
>
> Fixes: e0cea7ce988c ("bpf: implement ld_abs/ld_ind in native bpf")
> Reported-by: Bryce Kahle <bryce.kahle@datadoghq.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

Applied. Thanks

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-09-08 16:21 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-07 22:04 [PATCH bpf] bpf: Fix clobbering of r2 in bpf_gen_ld_abs Daniel Borkmann
2020-09-08 16:20 ` Alexei Starovoitov

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).