|
Gary Lin |
b8aeab |
From: Daniel Borkmann <daniel@iogearbox.net>
|
|
Gary Lin |
b8aeab |
Date: Fri, 5 Feb 2021 17:20:14 +0100
|
|
Gary Lin |
b8aeab |
Subject: bpf: Fix verifier jsgt branch analysis on max bound
|
|
Gary Lin |
b8aeab |
Patch-mainline: v5.11
|
|
Gary Lin |
b8aeab |
Git-commit: ee114dd64c0071500345439fc79dd5e0f9d106ed
|
|
Gary Lin |
b8aeab |
References: bsc#1155518
|
|
Gary Lin |
b8aeab |
|
|
Gary Lin |
b8aeab |
Fix incorrect is_branch{32,64}_taken() analysis for the jsgt case. The return
|
|
Gary Lin |
b8aeab |
code for both will tell the caller whether a given conditional jump is taken
|
|
Gary Lin |
b8aeab |
or not, e.g. 1 means branch will be taken [for the involved registers] and the
|
|
Gary Lin |
b8aeab |
goto target will be executed, 0 means branch will not be taken and instead we
|
|
Gary Lin |
b8aeab |
fall-through to the next insn, and last but not least a -1 denotes that it is
|
|
Gary Lin |
b8aeab |
not known at verification time whether a branch will be taken or not. Now while
|
|
Gary Lin |
b8aeab |
the jsgt has the branch-taken case correct with reg->s32_min_value > sval, the
|
|
Gary Lin |
b8aeab |
branch-not-taken case is off-by-one when testing for reg->s32_max_value < sval
|
|
Gary Lin |
b8aeab |
since the branch will also be taken for reg->s32_max_value == sval. The jgt
|
|
Gary Lin |
b8aeab |
branch analysis, for example, gets this right.
|
|
Gary Lin |
b8aeab |
|
|
Gary Lin |
b8aeab |
Fixes: 3f50f132d840 ("bpf: Verifier, do explicit ALU32 bounds tracking")
|
|
Gary Lin |
b8aeab |
Fixes: 4f7b3e82589e ("bpf: improve verifier branch analysis")
|
|
Gary Lin |
b8aeab |
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
|
|
Gary Lin |
b8aeab |
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
|
|
Gary Lin |
b8aeab |
Acked-by: Alexei Starovoitov <ast@kernel.org>
|
|
Gary Lin |
b8aeab |
Acked-by: Gary Lin <glin@suse.com>
|
|
Gary Lin |
b8aeab |
|
|
Gary Lin |
b8aeab |
NOTE from Gary:
|
|
Gary Lin |
b8aeab |
* This patch is modified for SLE15-SP2 only. Since 3f50f132d840 is not merged,
|
|
Gary Lin |
b8aeab |
the diff for is_branch32_taken() is omitted.
|
|
Gary Lin |
b8aeab |
|
|
Gary Lin |
b8aeab |
---
|
|
Gary Lin |
b8aeab |
kernel/bpf/verifier.c | 2 +-
|
|
Gary Lin |
b8aeab |
1 file changed, 1 insertion(+), 1 deletion(-)
|
|
Gary Lin |
b8aeab |
|
|
Gary Lin |
b8aeab |
--- a/kernel/bpf/verifier.c
|
|
Gary Lin |
b8aeab |
+++ b/kernel/bpf/verifier.c
|
|
Gary Lin |
b8aeab |
@@ -5499,7 +5499,7 @@ static int is_branch_taken(struct bpf_re
|
|
Gary Lin |
b8aeab |
case BPF_JSGT:
|
|
Gary Lin |
b8aeab |
if (reg->smin_value > sval)
|
|
Gary Lin |
b8aeab |
return 1;
|
|
Gary Lin |
b8aeab |
- else if (reg->smax_value < sval)
|
|
Gary Lin |
b8aeab |
+ else if (reg->smax_value <= sval)
|
|
Gary Lin |
b8aeab |
return 0;
|
|
Gary Lin |
b8aeab |
break;
|
|
Gary Lin |
b8aeab |
case BPF_JLT:
|