| Message ID | 1546928219-120176-1-git-send-email-zhangxiaoxu5@huawei.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: fix shift overflow in ___bpf_prog_run | expand |
On Tue, Jan 08, 2019 at 02:16:59PM +0800, ZhangXiaoxu wrote: >From: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> > >There is a UBSAN bug as blew: >UBSAN: Undefined behaviour in kernel/bpf/core.c:1055:2 >shift exponent 511 is too large for 32-bit type 'unsigned int' > >Reproduce program: > #include <errno.h> > #include <stddef.h> > #include <stdio.h> > #include <stdlib.h> > #include <unistd.h> > #include <linux/audit.h> > #include <linux/filter.h> > #include <linux/seccomp.h> > #include <sys/prctl.h> > #include <sys/syscall.h> > > int main() { > struct sock_filter sock_filter[3] = { > BPF_JUMP(BPF_LDX|BPF_IMM, 0x1ff, 0x2, 0xfffffffffffffffd), > BPF_JUMP(BPF_ALU|BPF_LSH|BPF_X, 0x0, 0x506, 0x401), > BPF_JUMP(BPF_RET|BPF_K, 0x0, 0x0, SECCOMP_RET_KILL) > }; > > struct sock_fprog sock_fprog= { > .len = 3, > .filter = &sock_filter, > }; > > int ret = syscall(__NR_seccomp, SECCOMP_SET_MODE_FILTER, 0, &sock_fprog); > printf("%d\n", ret); > > return 0; > } > >Make sure the right operand not greater than or equal to the >width of the promoted left operand when do shift operation. Hi, I am wonder how to test the program above. Gcc and options?Just i am new to ebpf and want to test it. Thanks. > >Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com> >--- > kernel/bpf/core.c | 19 +++++++++++++++++-- > 1 file changed, 17 insertions(+), 2 deletions(-) > >diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c >index 503d421..7635557 100644 >--- a/kernel/bpf/core.c >+++ b/kernel/bpf/core.c >@@ -683,11 +683,26 @@ static unsigned int __bpf_prog_run(void *ctx, const struct bpf_insn *insn) > ALU(SUB, -) > ALU(AND, &) > ALU(OR, |) >- ALU(LSH, <<) >- ALU(RSH, >>) > ALU(XOR, ^) > ALU(MUL, *) > #undef ALU >+#define ALU_SHIFT(OPCODE, OP) \ >+ ALU64_##OPCODE##_X: \ >+ DST = DST OP (SRC & 0x3F); \ >+ CONT; \ >+ ALU_##OPCODE##_X: \ >+ DST = (u32) DST OP (SRC & 0x1F);\ >+ CONT; \ >+ ALU64_##OPCODE##_K: \ >+ DST = DST OP (IMM & 0x3F); \ >+ CONT; \ >+ ALU_##OPCODE##_K: \ >+ DST = (u32) DST OP (IMM & 0x1F);\ >+ CONT; >+ >+ ALU_SHIFT(LSH, <<) >+ ALU_SHIFT(RSH, >>) >+#undef ALU_SHIFT > ALU_NEG: > DST = (u32) -DST; > CONT; >-- >2.7.4 >
On 01/08/2019 07:16 AM, ZhangXiaoxu wrote: > From: Zhang Xiaoxu <zhangxiaoxu5@huawei.com> > > There is a UBSAN bug as blew: > UBSAN: Undefined behaviour in kernel/bpf/core.c:1055:2 > shift exponent 511 is too large for 32-bit type 'unsigned int' > > Reproduce program: > #include <errno.h> > #include <stddef.h> > #include <stdio.h> > #include <stdlib.h> > #include <unistd.h> > #include <linux/audit.h> > #include <linux/filter.h> > #include <linux/seccomp.h> > #include <sys/prctl.h> > #include <sys/syscall.h> > > int main() { > struct sock_filter sock_filter[3] = { > BPF_JUMP(BPF_LDX|BPF_IMM, 0x1ff, 0x2, 0xfffffffffffffffd), > BPF_JUMP(BPF_ALU|BPF_LSH|BPF_X, 0x0, 0x506, 0x401), > BPF_JUMP(BPF_RET|BPF_K, 0x0, 0x0, SECCOMP_RET_KILL) > }; > > struct sock_fprog sock_fprog= { > .len = 3, > .filter = &sock_filter, > }; > > int ret = syscall(__NR_seccomp, SECCOMP_SET_MODE_FILTER, 0, &sock_fprog); > printf("%d\n", ret); > > return 0; > } > > Make sure the right operand not greater than or equal to the > width of the promoted left operand when do shift operation. > > Signed-off-by: ZhangXiaoxu <zhangxiaoxu5@huawei.com> See discussion in: https://lkml.org/lkml/2015/12/4/148 Thanks, Daniel
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 503d421..7635557 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -683,11 +683,26 @@ static unsigned int __bpf_prog_run(void *ctx, const struct bpf_insn *insn) ALU(SUB, -) ALU(AND, &) ALU(OR, |) - ALU(LSH, <<) - ALU(RSH, >>) ALU(XOR, ^) ALU(MUL, *) #undef ALU +#define ALU_SHIFT(OPCODE, OP) \ + ALU64_##OPCODE##_X: \ + DST = DST OP (SRC & 0x3F); \ + CONT; \ + ALU_##OPCODE##_X: \ + DST = (u32) DST OP (SRC & 0x1F);\ + CONT; \ + ALU64_##OPCODE##_K: \ + DST = DST OP (IMM & 0x3F); \ + CONT; \ + ALU_##OPCODE##_K: \ + DST = (u32) DST OP (IMM & 0x1F);\ + CONT; + + ALU_SHIFT(LSH, <<) + ALU_SHIFT(RSH, >>) +#undef ALU_SHIFT ALU_NEG: DST = (u32) -DST; CONT;