* [PATCH v2 bpf-next 1/2] bpf: prevent r10 register from being marked as precise
@ 2024-04-04 21:45 Andrii Nakryiko
2024-04-04 21:45 ` [PATCH v2 bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests Andrii Nakryiko
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Andrii Nakryiko @ 2024-04-04 21:45 UTC (permalink / raw)
To: bpf, ast, daniel, martin.lau
Cc: andrii, kernel-team, syzbot+148110ee7cf72f39f33e
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: syzbot+148110ee7cf72f39f33e@syzkaller.appspotmail.com
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
kernel/bpf/verifier.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
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
*/
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests
2024-04-04 21:45 [PATCH v2 bpf-next 1/2] bpf: prevent r10 register from being marked as precise Andrii Nakryiko
@ 2024-04-04 21:45 ` Andrii Nakryiko
2024-04-05 0:51 ` Yonghong Song
2024-04-05 0:34 ` [PATCH v2 bpf-next 1/2] bpf: prevent r10 register from being marked as precise Yonghong Song
2024-04-05 1:40 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 5+ messages in thread
From: Andrii Nakryiko @ 2024-04-04 21:45 UTC (permalink / raw)
To: bpf, ast, daniel, martin.lau
Cc: andrii, kernel-team, syzbot+148110ee7cf72f39f33e
Add selftests validating that BPF verifier handles precision marking
for SCALAR registers derived from r10 (fp) register correctly.
Given `r0 = (s8)r10;` syntax is not supported by older Clang compilers,
use the raw BPF instruction syntax to maximize compatibility.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
tools/include/linux/filter.h | 18 ++++
.../bpf/progs/verifier_subprog_precision.c | 89 +++++++++++++++++++
2 files changed, 107 insertions(+)
diff --git a/tools/include/linux/filter.h b/tools/include/linux/filter.h
index 736bdeccdfe4..65aa8ce142e5 100644
--- a/tools/include/linux/filter.h
+++ b/tools/include/linux/filter.h
@@ -111,6 +111,24 @@
.off = 0, \
.imm = IMM })
+/* Short form of movsx, dst_reg = (s8,s16,s32)src_reg */
+
+#define BPF_MOVSX64_REG(DST, SRC, OFF) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU64 | BPF_MOV | BPF_X, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = OFF, \
+ .imm = 0 })
+
+#define BPF_MOVSX32_REG(DST, SRC, OFF) \
+ ((struct bpf_insn) { \
+ .code = BPF_ALU | BPF_MOV | BPF_X, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = OFF, \
+ .imm = 0 })
+
/* Short form of mov based on type, BPF_X: dst_reg = src_reg, BPF_K: dst_reg = imm32 */
#define BPF_MOV64_RAW(TYPE, DST, SRC, IMM) \
diff --git a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
index 6f5d19665cf6..4a58e0398e72 100644
--- a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
+++ b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
@@ -6,6 +6,7 @@
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
#include "bpf_misc.h"
+#include <../../../tools/include/linux/filter.h>
#define ARRAY_SIZE(x) (sizeof(x) / sizeof(x[0]))
@@ -76,6 +77,94 @@ __naked int subprog_result_precise(void)
);
}
+__naked __noinline __used
+static unsigned long fp_leaking_subprog()
+{
+ asm volatile (
+ ".8byte %[r0_eq_r10_cast_s8];"
+ "exit;"
+ :: __imm_insn(r0_eq_r10_cast_s8, BPF_MOVSX64_REG(BPF_REG_0, BPF_REG_10, 8))
+ );
+}
+
+__naked __noinline __used
+static unsigned long sneaky_fp_leaking_subprog()
+{
+ asm volatile (
+ "r1 = r10;"
+ ".8byte %[r0_eq_r1_cast_s8];"
+ "exit;"
+ :: __imm_insn(r0_eq_r1_cast_s8, BPF_MOVSX64_REG(BPF_REG_0, BPF_REG_1, 8))
+ );
+}
+
+SEC("?raw_tp")
+__success __log_level(2)
+__msg("6: (0f) r1 += r0")
+__msg("mark_precise: frame0: last_idx 6 first_idx 0 subseq_idx -1")
+__msg("mark_precise: frame0: regs=r0 stack= before 5: (bf) r1 = r6")
+__msg("mark_precise: frame0: regs=r0 stack= before 4: (27) r0 *= 4")
+__msg("mark_precise: frame0: regs=r0 stack= before 3: (57) r0 &= 3")
+__msg("mark_precise: frame0: regs=r0 stack= before 10: (95) exit")
+__msg("mark_precise: frame1: regs=r0 stack= before 9: (bf) r0 = (s8)r10")
+__msg("7: R0_w=scalar")
+__naked int fp_precise_subprog_result(void)
+{
+ asm volatile (
+ "call fp_leaking_subprog;"
+ /* use subprog's returned value (which is derived from r10=fp
+ * register), as index into vals array, forcing all of that to
+ * be known precisely
+ */
+ "r0 &= 3;"
+ "r0 *= 4;"
+ "r1 = %[vals];"
+ /* force precision marking */
+ "r1 += r0;"
+ "r0 = *(u32 *)(r1 + 0);"
+ "exit;"
+ :
+ : __imm_ptr(vals)
+ : __clobber_common
+ );
+}
+
+SEC("?raw_tp")
+__success __log_level(2)
+__msg("6: (0f) r1 += r0")
+__msg("mark_precise: frame0: last_idx 6 first_idx 0 subseq_idx -1")
+__msg("mark_precise: frame0: regs=r0 stack= before 5: (bf) r1 = r6")
+__msg("mark_precise: frame0: regs=r0 stack= before 4: (27) r0 *= 4")
+__msg("mark_precise: frame0: regs=r0 stack= before 3: (57) r0 &= 3")
+__msg("mark_precise: frame0: regs=r0 stack= before 11: (95) exit")
+__msg("mark_precise: frame1: regs=r0 stack= before 10: (bf) r0 = (s8)r1")
+/* here r1 is marked precise, even though it's fp register, but that's fine
+ * because by the time we get out of subprogram it has to be derived from r10
+ * anyways, at which point we'll break precision chain
+ */
+__msg("mark_precise: frame1: regs=r1 stack= before 9: (bf) r1 = r10")
+__msg("7: R0_w=scalar")
+__naked int sneaky_fp_precise_subprog_result(void)
+{
+ asm volatile (
+ "call sneaky_fp_leaking_subprog;"
+ /* use subprog's returned value (which is derived from r10=fp
+ * register), as index into vals array, forcing all of that to
+ * be known precisely
+ */
+ "r0 &= 3;"
+ "r0 *= 4;"
+ "r1 = %[vals];"
+ /* force precision marking */
+ "r1 += r0;"
+ "r0 = *(u32 *)(r1 + 0);"
+ "exit;"
+ :
+ : __imm_ptr(vals)
+ : __clobber_common
+ );
+}
+
SEC("?raw_tp")
__success __log_level(2)
__msg("9: (0f) r1 += r0")
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 bpf-next 1/2] bpf: prevent r10 register from being marked as precise
2024-04-04 21:45 [PATCH v2 bpf-next 1/2] bpf: prevent r10 register from being marked as precise Andrii Nakryiko
2024-04-04 21:45 ` [PATCH v2 bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests Andrii Nakryiko
@ 2024-04-05 0:34 ` Yonghong Song
2024-04-05 1:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 5+ messages in thread
From: Yonghong Song @ 2024-04-05 0:34 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, ast, daniel, martin.lau
Cc: kernel-team, syzbot+148110ee7cf72f39f33e
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: syzbot+148110ee7cf72f39f33e@syzkaller.appspotmail.com
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests
2024-04-04 21:45 ` [PATCH v2 bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests Andrii Nakryiko
@ 2024-04-05 0:51 ` Yonghong Song
0 siblings, 0 replies; 5+ messages in thread
From: Yonghong Song @ 2024-04-05 0:51 UTC (permalink / raw)
To: Andrii Nakryiko, bpf, ast, daniel, martin.lau
Cc: kernel-team, syzbot+148110ee7cf72f39f33e
On 4/4/24 2:45 PM, Andrii Nakryiko wrote:
> Add selftests validating that BPF verifier handles precision marking
> for SCALAR registers derived from r10 (fp) register correctly.
>
> Given `r0 = (s8)r10;` syntax is not supported by older Clang compilers,
> use the raw BPF instruction syntax to maximize compatibility.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Yonghong Song <yonghong.song@linux.dev>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 bpf-next 1/2] bpf: prevent r10 register from being marked as precise
2024-04-04 21:45 [PATCH v2 bpf-next 1/2] bpf: prevent r10 register from being marked as precise Andrii Nakryiko
2024-04-04 21:45 ` [PATCH v2 bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests Andrii Nakryiko
2024-04-05 0:34 ` [PATCH v2 bpf-next 1/2] bpf: prevent r10 register from being marked as precise Yonghong Song
@ 2024-04-05 1:40 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-04-05 1:40 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, ast, daniel, martin.lau, kernel-team, syzbot+148110ee7cf72f39f33e
Hello:
This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:
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!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-04-05 1:40 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-04 21:45 [PATCH v2 bpf-next 1/2] bpf: prevent r10 register from being marked as precise Andrii Nakryiko
2024-04-04 21:45 ` [PATCH v2 bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests Andrii Nakryiko
2024-04-05 0:51 ` Yonghong Song
2024-04-05 0:34 ` [PATCH v2 bpf-next 1/2] bpf: prevent r10 register from being marked as precise Yonghong Song
2024-04-05 1:40 ` patchwork-bot+netdevbpf
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.