| Message ID | 20220513011025.13344-1-alexei.starovoitov@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4b6313cf99b0d51b49aeaea98ec76ca8161ecb80 |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,1/2] bpf: Fix combination of jit blinding and pointers to bpf subprogs. | expand |
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 >
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>
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>
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!
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.