[PATCH bpf-next 1/2] bpf: Fix combination of jit blinding and pointers to bpf subprogs.

[PATCH bpf-next 1/2] bpf: Fix combination of jit blinding and pointers to bpf subprogs.

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

The combination of jit blinding and pointers to bpf subprogs causes:
[   36.989548] BUG: unable to handle page fault for address: 0000000100000001
[   36.990342] #PF: supervisor instruction fetch in kernel mode
[   36.990968] #PF: error_code(0x0010) - not-present page
[   36.994859] RIP: 0010:0x100000001
[   36.995209] Code: Unable to access opcode bytes at RIP 0xffffffd7.
[   37.004091] Call Trace:
[   37.004351]  <TASK>
[   37.004576]  ? bpf_loop+0x4d/0x70
[   37.004932]  ? bpf_prog_3899083f75e4c5de_F+0xe3/0x13b

The jit blinding logic didn't recognize that ld_imm64 with an address
of bpf subprogram is a special instruction and proceeded to randomize it.
By itself it wouldn't have been an issue, but jit_subprogs() logic
relies on two step process to JIT all subprogs and then JIT them
again when addresses of all subprogs are known.
Blinding process in the first JIT phase caused second JIT to miss
adjustment of special ld_imm64.

Fix this issue by ignoring special ld_imm64 instructions that don't have
user controlled constants and shouldn't be blinded.

Fixes: 69c087ba6225 ("bpf: Add bpf_for_each_map_elem() helper")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/core.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 76f68d0a7ae8..9cc91f0f3115 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -1434,6 +1434,16 @@ struct bpf_prog *bpf_jit_blind_constants(struct bpf_prog *prog)
 	insn = clone->insnsi;
 
 	for (i = 0; i < insn_cnt; i++, insn++) {
+		if (bpf_pseudo_func(insn)) {
+			/* ld_imm64 with an address of bpf subprog is not
+			 * a user controlled constant. Don't randomize it,
+			 * since it will conflict with jit_subprogs() logic.
+			 */
+			insn++;
+			i++;
+			continue;
+		}
+
 		/* We temporarily need to hold the original ld64 insn
 		 * so that we can still access the first part in the
 		 * second blinding run.
-- 
2.30.2

[PATCH bpf-next 2/2] selftests/bpf: Check combination of jit blinding and pointers to bpf subprogs.

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

Check that ld_imm64 with src_reg=1 (aka BPF_PSEUDO_FUNC) works
with jit_blinding.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 tools/testing/selftests/bpf/progs/test_subprogs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/test_subprogs.c b/tools/testing/selftests/bpf/progs/test_subprogs.c
index b7c37ca09544..f8e9256cf18d 100644
--- a/tools/testing/selftests/bpf/progs/test_subprogs.c
+++ b/tools/testing/selftests/bpf/progs/test_subprogs.c
@@ -89,6 +89,11 @@ int prog2(void *ctx)
 	return 0;
 }
 
+static int empty_callback(__u32 index, void *data)
+{
+	return 0;
+}
+
 /* prog3 has the same section name as prog1 */
 SEC("raw_tp/sys_enter")
 int prog3(void *ctx)
@@ -98,6 +103,9 @@ int prog3(void *ctx)
 	if (!BPF_CORE_READ(t, pid) || !get_task_tgid((uintptr_t)t))
 		return 1;
 
+	/* test that ld_imm64 with BPF_PSEUDO_FUNC doesn't get blinded */
+	bpf_loop(1, empty_callback, NULL, 0);
+
 	res3 = sub3(5) + 6; /* (5 + 3 + (4 + 1)) + 6 = 19 */
 	return 0;
 }
-- 
2.30.2

Re: [PATCH bpf-next 2/2] selftests/bpf: Check combination of jit blinding and pointers to bpf subprogs.

[Andrii Nakryiko]

On Thu, May 12, 2022 at 6:10 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> From: Alexei Starovoitov <ast@kernel.org>
>
> Check that ld_imm64 with src_reg=1 (aka BPF_PSEUDO_FUNC) works
> with jit_blinding.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---

Acked-by: Andrii Nakryiko <andrii@kernel.org>

>  tools/testing/selftests/bpf/progs/test_subprogs.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/test_subprogs.c b/tools/testing/selftests/bpf/progs/test_subprogs.c
> index b7c37ca09544..f8e9256cf18d 100644
> --- a/tools/testing/selftests/bpf/progs/test_subprogs.c
> +++ b/tools/testing/selftests/bpf/progs/test_subprogs.c
> @@ -89,6 +89,11 @@ int prog2(void *ctx)
>         return 0;
>  }
>
> +static int empty_callback(__u32 index, void *data)
> +{
> +       return 0;
> +}
> +
>  /* prog3 has the same section name as prog1 */
>  SEC("raw_tp/sys_enter")
>  int prog3(void *ctx)
> @@ -98,6 +103,9 @@ int prog3(void *ctx)
>         if (!BPF_CORE_READ(t, pid) || !get_task_tgid((uintptr_t)t))
>                 return 1;
>
> +       /* test that ld_imm64 with BPF_PSEUDO_FUNC doesn't get blinded */
> +       bpf_loop(1, empty_callback, NULL, 0);
> +
>         res3 = sub3(5) + 6; /* (5 + 3 + (4 + 1)) + 6 = 19 */
>         return 0;
>  }
> --
> 2.30.2
>

Re: [PATCH bpf-next 2/2] selftests/bpf: Check combination of jit blinding and pointers to bpf subprogs.

[Martin KaFai Lau]

On Thu, May 12, 2022 at 06:10:25PM -0700, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> Check that ld_imm64 with src_reg=1 (aka BPF_PSEUDO_FUNC) works
> with jit_blinding.
Acked-by: Martin KaFai Lau <kafai@fb.com>

Re: [PATCH bpf-next 1/2] bpf: Fix combination of jit blinding and pointers to bpf subprogs.

[Andrii Nakryiko]

On Thu, May 12, 2022 at 6:10 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> From: Alexei Starovoitov <ast@kernel.org>
>
> The combination of jit blinding and pointers to bpf subprogs causes:
> [   36.989548] BUG: unable to handle page fault for address: 0000000100000001
> [   36.990342] #PF: supervisor instruction fetch in kernel mode
> [   36.990968] #PF: error_code(0x0010) - not-present page
> [   36.994859] RIP: 0010:0x100000001
> [   36.995209] Code: Unable to access opcode bytes at RIP 0xffffffd7.
> [   37.004091] Call Trace:
> [   37.004351]  <TASK>
> [   37.004576]  ? bpf_loop+0x4d/0x70
> [   37.004932]  ? bpf_prog_3899083f75e4c5de_F+0xe3/0x13b
>
> The jit blinding logic didn't recognize that ld_imm64 with an address
> of bpf subprogram is a special instruction and proceeded to randomize it.
> By itself it wouldn't have been an issue, but jit_subprogs() logic
> relies on two step process to JIT all subprogs and then JIT them
> again when addresses of all subprogs are known.
> Blinding process in the first JIT phase caused second JIT to miss
> adjustment of special ld_imm64.
>
> Fix this issue by ignoring special ld_imm64 instructions that don't have
> user controlled constants and shouldn't be blinded.
>
> Fixes: 69c087ba6225 ("bpf: Add bpf_for_each_map_elem() helper")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---

Thanks for the fix, LGTM.

Reported-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Andrii Nakryiko <andrii@kernel.org>


>  kernel/bpf/core.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 76f68d0a7ae8..9cc91f0f3115 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -1434,6 +1434,16 @@ struct bpf_prog *bpf_jit_blind_constants(struct bpf_prog *prog)
>         insn = clone->insnsi;
>
>         for (i = 0; i < insn_cnt; i++, insn++) {
> +               if (bpf_pseudo_func(insn)) {
> +                       /* ld_imm64 with an address of bpf subprog is not
> +                        * a user controlled constant. Don't randomize it,
> +                        * since it will conflict with jit_subprogs() logic.
> +                        */
> +                       insn++;
> +                       i++;
> +                       continue;
> +               }
> +
>                 /* We temporarily need to hold the original ld64 insn
>                  * so that we can still access the first part in the
>                  * second blinding run.
> --
> 2.30.2
>

Re: [PATCH bpf-next 1/2] bpf: Fix combination of jit blinding and pointers to bpf subprogs.

[Alexei Starovoitov]

On Thu, May 12, 2022 at 8:44 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Thu, May 12, 2022 at 6:10 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > From: Alexei Starovoitov <ast@kernel.org>
> >
> > The combination of jit blinding and pointers to bpf subprogs causes:
> > [   36.989548] BUG: unable to handle page fault for address: 0000000100000001
> > [   36.990342] #PF: supervisor instruction fetch in kernel mode
> > [   36.990968] #PF: error_code(0x0010) - not-present page
> > [   36.994859] RIP: 0010:0x100000001
> > [   36.995209] Code: Unable to access opcode bytes at RIP 0xffffffd7.
> > [   37.004091] Call Trace:
> > [   37.004351]  <TASK>
> > [   37.004576]  ? bpf_loop+0x4d/0x70
> > [   37.004932]  ? bpf_prog_3899083f75e4c5de_F+0xe3/0x13b
> >
> > The jit blinding logic didn't recognize that ld_imm64 with an address
> > of bpf subprogram is a special instruction and proceeded to randomize it.
> > By itself it wouldn't have been an issue, but jit_subprogs() logic
> > relies on two step process to JIT all subprogs and then JIT them
> > again when addresses of all subprogs are known.
> > Blinding process in the first JIT phase caused second JIT to miss
> > adjustment of special ld_imm64.
> >
> > Fix this issue by ignoring special ld_imm64 instructions that don't have
> > user controlled constants and shouldn't be blinded.
> >
> > Fixes: 69c087ba6225 ("bpf: Add bpf_for_each_map_elem() helper")
> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> > ---
>
> Thanks for the fix, LGTM.
>
> Reported-by: Andrii Nakryiko <andrii@kernel.org>

Ohh. Sorry. Yes!

> Acked-by: Andrii Nakryiko <andrii@kernel.org>

Re: [PATCH bpf-next 1/2] bpf: Fix combination of jit blinding and pointers to bpf subprogs.

[Martin KaFai Lau]

On Thu, May 12, 2022 at 06:10:24PM -0700, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> The combination of jit blinding and pointers to bpf subprogs causes:
> [   36.989548] BUG: unable to handle page fault for address: 0000000100000001
> [   36.990342] #PF: supervisor instruction fetch in kernel mode
> [   36.990968] #PF: error_code(0x0010) - not-present page
> [   36.994859] RIP: 0010:0x100000001
> [   36.995209] Code: Unable to access opcode bytes at RIP 0xffffffd7.
> [   37.004091] Call Trace:
> [   37.004351]  <TASK>
> [   37.004576]  ? bpf_loop+0x4d/0x70
> [   37.004932]  ? bpf_prog_3899083f75e4c5de_F+0xe3/0x13b
> 
> The jit blinding logic didn't recognize that ld_imm64 with an address
> of bpf subprogram is a special instruction and proceeded to randomize it.
> By itself it wouldn't have been an issue, but jit_subprogs() logic
> relies on two step process to JIT all subprogs and then JIT them
> again when addresses of all subprogs are known.
> Blinding process in the first JIT phase caused second JIT to miss
> adjustment of special ld_imm64.
> 
> Fix this issue by ignoring special ld_imm64 instructions that don't have
> user controlled constants and shouldn't be blinded.
Acked-by: Martin KaFai Lau <kafai@fb.com>

Re: [PATCH bpf-next 1/2] bpf: Fix combination of jit blinding and pointers to bpf subprogs.

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:

On Thu, 12 May 2022 18:10:24 -0700 you wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> The combination of jit blinding and pointers to bpf subprogs causes:
> [   36.989548] BUG: unable to handle page fault for address: 0000000100000001
> [   36.990342] #PF: supervisor instruction fetch in kernel mode
> [   36.990968] #PF: error_code(0x0010) - not-present page
> [   36.994859] RIP: 0010:0x100000001
> [   36.995209] Code: Unable to access opcode bytes at RIP 0xffffffd7.
> [   37.004091] Call Trace:
> [   37.004351]  <TASK>
> [   37.004576]  ? bpf_loop+0x4d/0x70
> [   37.004932]  ? bpf_prog_3899083f75e4c5de_F+0xe3/0x13b
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/2] bpf: Fix combination of jit blinding and pointers to bpf subprogs.
    https://git.kernel.org/bpf/bpf-next/c/4b6313cf99b0
  - [bpf-next,2/2] selftests/bpf: Check combination of jit blinding and pointers to bpf subprogs.
    https://git.kernel.org/bpf/bpf-next/c/365d519923a2

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html