diff mbox series

bpf: fix shift overflow in ___bpf_prog_run

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

Commit Message

Zhang Xiaoxu Jan. 8, 2019, 6:16 a.m. UTC
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>
---
 kernel/bpf/core.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

Comments

Bo YU Jan. 8, 2019, 12:29 p.m. UTC | #1
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
>
Daniel Borkmann Jan. 8, 2019, 12:54 p.m. UTC | #2
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 mbox series

Patch

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;