| Message ID | 20230731204534.1975311-1-yonghong.song@linux.dev (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e99688eba2e90a600956e936bc335ece902a5d7f |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] kernel/bpf: Fix an array-index-out-of-bounds issue in disasm.c | expand |
On Mon, 2023-07-31 at 13:45 -0700, Yonghong Song wrote: > syzbot reported an array-index-out-of-bounds when printing out bpf > insns. Further investigation shows the insn is illegal but > is printed out due to log level 1 or 2 before actual insn verification > in do_check(). > > This particular illegal insn is a MOVSX insn with offset value 2. > The legal offset value for MOVSX should be 8, 16 and 32. > The disasm sign-extension-size array index is calculated as > (insn->off / 8) - 1 > and offset value 2 gives an out-of-bound index -1. > > Tighten the checking for MOVSX insn in disasm.c to avoid > array-index-out-of-bounds issue. > > Reported-by: syzbot+3758842a6c01012aa73b@syzkaller.appspotmail.com > Fixes: f835bb622299 ("bpf: Add kernel/bpftool asm support for new instructions") > Signed-off-by: Yonghong Song <yonghong.song@linux.dev> Acked-by: Eduard Zingerman <eddyz87@gmail.com> > --- > kernel/bpf/disasm.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/kernel/bpf/disasm.c b/kernel/bpf/disasm.c > index d7bff608f299..ef7c107c7b8f 100644 > --- a/kernel/bpf/disasm.c > +++ b/kernel/bpf/disasm.c > @@ -162,7 +162,8 @@ static bool is_sdiv_smod(const struct bpf_insn *insn) > > static bool is_movsx(const struct bpf_insn *insn) > { > - return BPF_OP(insn->code) == BPF_MOV && insn->off != 0; > + return BPF_OP(insn->code) == BPF_MOV && > + (insn->off == 8 || insn->off == 16 || insn->off == 32); > } > > void print_bpf_insn(const struct bpf_insn_cbs *cbs,
On Mon, Jul 31, 2023 at 1:45 PM Yonghong Song <yonghong.song@linux.dev> wrote: > > syzbot reported an array-index-out-of-bounds when printing out bpf Applied and fixed subject. "kernel/bpf:" is an usual prefix. Not sure why you used it. Just "bpf:" is enough.
On 7/31/23 5:37 PM, Alexei Starovoitov wrote: > On Mon, Jul 31, 2023 at 1:45 PM Yonghong Song <yonghong.song@linux.dev> wrote: >> >> syzbot reported an array-index-out-of-bounds when printing out bpf > > Applied and fixed subject. > "kernel/bpf:" is an usual prefix. Not sure why you used it. > Just "bpf:" is enough. I am not sure either :-( Maybe sleepy at that moment :-)
Hello: This patch was applied to bpf/bpf-next.git (master) by Alexei Starovoitov <ast@kernel.org>: On Mon, 31 Jul 2023 13:45:34 -0700 you wrote: > syzbot reported an array-index-out-of-bounds when printing out bpf > insns. Further investigation shows the insn is illegal but > is printed out due to log level 1 or 2 before actual insn verification > in do_check(). > > This particular illegal insn is a MOVSX insn with offset value 2. > The legal offset value for MOVSX should be 8, 16 and 32. > The disasm sign-extension-size array index is calculated as > (insn->off / 8) - 1 > and offset value 2 gives an out-of-bound index -1. > > [...] Here is the summary with links: - [bpf-next] kernel/bpf: Fix an array-index-out-of-bounds issue in disasm.c https://git.kernel.org/bpf/bpf-next/c/e99688eba2e9 You are awesome, thank you!
diff --git a/kernel/bpf/disasm.c b/kernel/bpf/disasm.c index d7bff608f299..ef7c107c7b8f 100644 --- a/kernel/bpf/disasm.c +++ b/kernel/bpf/disasm.c @@ -162,7 +162,8 @@ static bool is_sdiv_smod(const struct bpf_insn *insn) static bool is_movsx(const struct bpf_insn *insn) { - return BPF_OP(insn->code) == BPF_MOV && insn->off != 0; + return BPF_OP(insn->code) == BPF_MOV && + (insn->off == 8 || insn->off == 16 || insn->off == 32); } void print_bpf_insn(const struct bpf_insn_cbs *cbs,
syzbot reported an array-index-out-of-bounds when printing out bpf insns. Further investigation shows the insn is illegal but is printed out due to log level 1 or 2 before actual insn verification in do_check(). This particular illegal insn is a MOVSX insn with offset value 2. The legal offset value for MOVSX should be 8, 16 and 32. The disasm sign-extension-size array index is calculated as (insn->off / 8) - 1 and offset value 2 gives an out-of-bound index -1. Tighten the checking for MOVSX insn in disasm.c to avoid array-index-out-of-bounds issue. Reported-by: syzbot+3758842a6c01012aa73b@syzkaller.appspotmail.com Fixes: f835bb622299 ("bpf: Add kernel/bpftool asm support for new instructions") Signed-off-by: Yonghong Song <yonghong.song@linux.dev> --- kernel/bpf/disasm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)