[bpf-next] bpf: Mark bpf prog stack with kmsan_unposion_memory in interpreter mode

Commit Message

Martin KaFai Lau March 28, 2024, 6:58 p.m. UTC

From: Martin KaFai Lau <[e-mail]>

syzbot reported uninit memory usages during map_{lookup,delete}_elem.

==========
BUG: KMSAN: uninit-value in __dev_map_lookup_elem kernel/bpf/devmap.c:441 [inline]
BUG: KMSAN: uninit-value in dev_map_lookup_elem+0xf3/0x170 kernel/bpf/devmap.c:796
__dev_map_lookup_elem kernel/bpf/devmap.c:441 [inline]
dev_map_lookup_elem+0xf3/0x170 kernel/bpf/devmap.c:796
____bpf_map_lookup_elem kernel/bpf/helpers.c:42 [inline]
bpf_map_lookup_elem+0x5c/0x80 kernel/bpf/helpers.c:38
___bpf_prog_run+0x13fe/0xe0f0 kernel/bpf/core.c:1997
__bpf_prog_run256+0xb5/0xe0 kernel/bpf/core.c:2237
==========

The reproducer should be in the interpreter mode.

The C reproducer is trying to run the following bpf prog:

    0: (18) r0 = 0x0
    2: (18) r1 = map[id:49]
    4: (b7) r8 = 16777216
    5: (7b) *(u64 *)(r10 -8) = r8
    6: (bf) r2 = r10
    7: (07) r2 += -229
            ^^^^^^^^^^

    8: (b7) r3 = 8
    9: (b7) r4 = 0
   10: (85) call dev_map_lookup_elem#1543472
   11: (95) exit

It is due to the "void *key" (r2) passed to the helper. bpf allows uninit
stack memory access for bpf prog with the right privileges. This patch
uses kmsan_unpoison_memory() to mark the stack as initialized.

This should address different syzbot reports on the uninit "void *key"
argument during map_{lookup,delete}_elem.

Reported-by: [e-mail]
Closes: https://lore.kernel.org/bpf/[e-mail]/
Reported-by: [e-mail]
Closes: https://lore.kernel.org/bpf/[e-mail]/
Reported-by: [e-mail]
Closes: https://lore.kernel.org/bpf/[e-mail]/
Reported-by: [e-mail]
Closes: https://lore.kernel.org/bpf/[e-mail]/
Reported-by: [e-mail]
Closes: https://lore.kernel.org/bpf/[e-mail]/
Tested-by: [e-mail]
Suggested-by: Yonghong Song <[e-mail]>
Suggested-by: Alexei Starovoitov <[e-mail]>
Signed-off-by: Martin KaFai Lau <[e-mail]>
---
 kernel/bpf/core.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

[e-mail] March 29, 2024, 2:10 a.m. UTC | #1

Hello:

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

On Thu, 28 Mar 2024 11:58:01 -0700 you wrote:
> From: Martin KaFai Lau <[e-mail]>
> 
> syzbot reported uninit memory usages during map_{lookup,delete}_elem.
> 
> ==========
> BUG: KMSAN: uninit-value in __dev_map_lookup_elem kernel/bpf/devmap.c:441 [inline]
> BUG: KMSAN: uninit-value in dev_map_lookup_elem+0xf3/0x170 kernel/bpf/devmap.c:796
> __dev_map_lookup_elem kernel/bpf/devmap.c:441 [inline]
> dev_map_lookup_elem+0xf3/0x170 kernel/bpf/devmap.c:796
> ____bpf_map_lookup_elem kernel/bpf/helpers.c:42 [inline]
> bpf_map_lookup_elem+0x5c/0x80 kernel/bpf/helpers.c:38
> ___bpf_prog_run+0x13fe/0xe0f0 kernel/bpf/core.c:1997
> __bpf_prog_run256+0xb5/0xe0 kernel/bpf/core.c:2237
> ==========
> 
> [...]

Here is the summary with links:
  - [bpf-next] bpf: Mark bpf prog stack with kmsan_unposion_memory in interpreter mode
    https://git.kernel.org/bpf/bpf-next/c/e8742081db7d

You are awesome, thank you!

Patch

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 5aacb1d3c4cc..ab400cdd7d7a 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2218,6 +2218,7 @@  static unsigned int PROG_NAME(stack_size)(const void *ctx, const struct bpf_insn
 	u64 stack[stack_size / sizeof(u64)]; \
 	u64 regs[MAX_BPF_EXT_REG] = {}; \
 \
+	kmsan_unpoison_memory(stack, sizeof(stack)); \
 	FP = (u64) (unsigned long) &stack[ARRAY_SIZE(stack)]; \
 	ARG1 = (u64) (unsigned long) ctx; \
 	return ___bpf_prog_run(regs, insn); \
@@ -2231,6 +2232,7 @@  static u64 PROG_NAME_ARGS(stack_size)(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5, \
 	u64 stack[stack_size / sizeof(u64)]; \
 	u64 regs[MAX_BPF_EXT_REG]; \
 \
+	kmsan_unpoison_memory(stack, sizeof(stack)); \
 	FP = (u64) (unsigned long) &stack[ARRAY_SIZE(stack)]; \
 	BPF_R1 = r1; \
 	BPF_R2 = r2; \