[v2,bpf-next,1/2] bpf: prevent r10 register from being marked as precise

Commit Message

Andrii Nakryiko April 4, 2024, 9:45 p.m. UTC

r10 is a special register that is not under BPF program's control and is
always effectively precise. The rest of precision logic assumes that
only r0-r9 SCALAR registers are marked as precise, so prevent r10 from
being marked precise.

This can happen due to signed cast instruction allowing to do something
like `r0 = (s8)r10;`, which later, if r0 needs to be precise, would lead
to an attempt to mark r10 as precise.

Prevent this with an extra check during instruction backtracking.

Fixes: 8100928c8814 ("bpf: Support new sign-extension mov insns")
Reported-by: [e-mail]
Signed-off-by: Andrii Nakryiko <[e-mail]>
---
 kernel/bpf/verifier.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Yonghong Song April 5, 2024, 12:34 a.m. UTC | #1

On 4/4/24 2:45 PM, Andrii Nakryiko wrote:
> r10 is a special register that is not under BPF program's control and is
> always effectively precise. The rest of precision logic assumes that
> only r0-r9 SCALAR registers are marked as precise, so prevent r10 from
> being marked precise.
>
> This can happen due to signed cast instruction allowing to do something
> like `r0 = (s8)r10;`, which later, if r0 needs to be precise, would lead
> to an attempt to mark r10 as precise.
>
> Prevent this with an extra check during instruction backtracking.
>
> Fixes: 8100928c8814 ("bpf: Support new sign-extension mov insns")
> Reported-by: [e-mail]
> Signed-off-by: Andrii Nakryiko <[e-mail]>

Acked-by: Yonghong Song <[e-mail]>

[e-mail] April 5, 2024, 1:40 a.m. UTC | #2

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[e-mail]>:

On Thu,  4 Apr 2024 14:45:35 -0700 you wrote:
> r10 is a special register that is not under BPF program's control and is
> always effectively precise. The rest of precision logic assumes that
> only r0-r9 SCALAR registers are marked as precise, so prevent r10 from
> being marked precise.
> 
> This can happen due to signed cast instruction allowing to do something
> like `r0 = (s8)r10;`, which later, if r0 needs to be precise, would lead
> to an attempt to mark r10 as precise.
> 
> [...]

Here is the summary with links:
  - [v2,bpf-next,1/2] bpf: prevent r10 register from being marked as precise
    https://git.kernel.org/bpf/bpf-next/c/1f2a74b41ea8
  - [v2,bpf-next,2/2] selftests/bpf: add fp-leaking precise subprog result tests
    https://git.kernel.org/bpf/bpf-next/c/343ca8131c35

You are awesome, thank you!

Patch

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index ffaa9f7f153c..41446bea8fab 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3615,7 +3615,8 @@  static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
 				 * sreg needs precision before this insn
 				 */
 				bt_clear_reg(bt, dreg);
-				bt_set_reg(bt, sreg);
+				if (sreg != BPF_REG_FP)
+					bt_set_reg(bt, sreg);
 			} else {
 				/* dreg = K
 				 * dreg needs precision after this insn.
@@ -3631,7 +3632,8 @@  static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
 				 * both dreg and sreg need precision
 				 * before this insn
 				 */
-				bt_set_reg(bt, sreg);
+				if (sreg != BPF_REG_FP)
+					bt_set_reg(bt, sreg);
 			} /* else dreg += K
 			   * dreg still needs precision before this insn
 			   */