From a1b14d27ed0965838350f1377ff97c93ee383492 Mon Sep 17 00:00:00 2001

From: Daniel Borkmann <daniel@iogearbox.net>

Date: Wed, 10 Feb 2016 16:47:11 +0100

Subject: [PATCH] bpf: fix branch offset adjustment on backjumps after patching

 ctx expansion

When ctx access is used, the kernel often needs to expand/rewrite

instructions, so after that patching, branch offsets have to be

adjusted for both forward and backward jumps in the new eBPF program,

but for backward jumps it fails to account the delta. Meaning, for

example, if the expansion happens exactly on the insn that sits at

the jump target, it doesn't fix up the back jump offset.

Analysis on what the check in adjust_branches() is currently doing:

  /* adjust offset of jmps if necessary */

  if (i < pos && i + insn->off + 1 > pos)

    insn->off += delta;

  else if (i > pos && i + insn->off + 1 < pos)

    insn->off -= delta;

First condition (forward jumps):

  Before:                         After:

  insns[0]                        insns[0]

  insns[1] <--- i/insn            insns[1] <--- i/insn

  insns[2] <--- pos               insns[P] <--- pos

  insns[3]                        insns[P]  `------| delta

  insns[4] <--- target_X          insns[P]   `-----|

  insns[5]                        insns[3]

                                  insns[4] <--- target_X

                                  insns[5]

First case is if we cross pos-boundary and the jump instruction was

before pos. This is handeled correctly. I.e. if i == pos, then this

would mean our jump that we currently check was the patchlet itself

that we just injected. Since such patchlets are self-contained and

have no awareness of any insns before or after the patched one, the

delta is correctly not adjusted. Also, for the second condition in

case of i + insn->off + 1 == pos, means we jump to that newly patched

instruction, so no offset adjustment are needed. That part is correct.

Second condition (backward jumps):

  Before:                         After:

  insns[0]                        insns[0]

  insns[1] <--- target_X          insns[1] <--- target_X

  insns[2] <--- pos <-- target_Y  insns[P] <--- pos <-- target_Y

  insns[3]                        insns[P]  `------| delta

  insns[4] <--- i/insn            insns[P]   `-----|

  insns[5]                        insns[3]

                                  insns[4] <--- i/insn

                                  insns[5]

Second interesting case is where we cross pos-boundary and the jump

instruction was after pos. Backward jump with i == pos would be

impossible and pose a bug somewhere in the patchlet, so the first

condition checking i > pos is okay only by itself. However, i +

insn->off + 1 < pos does not always work as intended to trigger the

adjustment. It works when jump targets would be far off where the

delta wouldn't matter. But, for example, where the fixed insn->off

before pointed to pos (target_Y), it now points to pos + delta, so

that additional room needs to be taken into account for the check.

This means that i) both tests here need to be adjusted into pos + delta,

and ii) for the second condition, the test needs to be <= as pos

itself can be a target in the backjump, too.

Fixes: 9bac3d6d548e ("bpf: allow extended BPF programs access skb fields")

Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>

Signed-off-by: David S. Miller <davem@davemloft.net>

---

 kernel/bpf/verifier.c | 2 +-

 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c

index d1d3e8f57de9..2e7f7ab739e4 100644

--- a/kernel/bpf/verifier.c

+++ b/kernel/bpf/verifier.c

@@ -2082,7 +2082,7 @@ static void adjust_branches(struct bpf_prog *prog, int pos, int delta)

 		/* adjust offset of jmps if necessary */

 		if (i < pos && i + insn->off + 1 > pos)

 			insn->off += delta;

-		else if (i > pos && i + insn->off + 1 < pos)

+		else if (i > pos + delta && i + insn->off + 1 <= pos + delta)

 			insn->off -= delta;

 	}

 }

-- 

2.5.0