* [PATCH 4.14 0/3] bpf: fix CVE-2021-4159
@ 2022-09-06 15:38 Ovidiu Panait
2022-09-06 15:38 ` [PATCH 4.14 1/3] bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds() Ovidiu Panait
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Ovidiu Panait @ 2022-09-06 15:38 UTC (permalink / raw)
To: stable; +Cc: Ovidiu Panait
All test_verifier/test_align selftests pass in qemu for x86-64 with this
series applied:
root@intel-x86-64:~# ./test_verifier
...
#433/p xadd/w check unaligned pkt OK
#434/p pass unmodified ctx pointer to helper OK
#435/p pass modified ctx pointer to helper, 1 OK
#436/u pass modified ctx pointer to helper, 2 OK
#436/p pass modified ctx pointer to helper, 2 OK
#437/p pass modified ctx pointer to helper, 3 OK
Summary: 667 PASSED, 0 FAILED
root@intel-x86-64:~# ./test_align
Test 0: mov ... PASS
Test 1: shift ... PASS
Test 2: addsub ... PASS
Test 3: mul ... PASS
Test 4: unknown shift ... PASS
Test 5: unknown mul ... PASS
Test 6: packet const offset ... PASS
Test 7: packet variable offset ... PASS
Test 8: packet variable offset 2 ... PASS
Test 9: dubious pointer arithmetic ... PASS
Test 10: variable subtraction ... PASS
Test 11: pointer variable subtraction ... PASS
Results: 12 pass 0 fail
John Fastabend (1):
bpf: Verifer, adjust_scalar_min_max_vals to always call
update_reg_bounds()
Maxim Mikityanskiy (1):
bpf: Fix the off-by-two error in range markings
Stanislav Fomichev (1):
selftests/bpf: Fix test_align verifier log patterns
kernel/bpf/verifier.c | 1 +
tools/testing/selftests/bpf/test_align.c | 27 +++++++++++----------
tools/testing/selftests/bpf/test_verifier.c | 16 ++++++------
3 files changed, 23 insertions(+), 21 deletions(-)
--
2.37.2
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 4.14 1/3] bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds()
2022-09-06 15:38 [PATCH 4.14 0/3] bpf: fix CVE-2021-4159 Ovidiu Panait
@ 2022-09-06 15:38 ` Ovidiu Panait
2022-09-06 15:38 ` [PATCH 4.14 2/3] selftests/bpf: Fix test_align verifier log patterns Ovidiu Panait
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Ovidiu Panait @ 2022-09-06 15:38 UTC (permalink / raw)
To: stable; +Cc: John Fastabend, Alexei Starovoitov, Ovidiu Panait
From: John Fastabend <john.fastabend@gmail.com>
commit 294f2fc6da27620a506e6c050241655459ccd6bd upstream.
Currently, for all op verification we call __red_deduce_bounds() and
__red_bound_offset() but we only call __update_reg_bounds() in bitwise
ops. However, we could benefit from calling __update_reg_bounds() in
BPF_ADD, BPF_SUB, and BPF_MUL cases as well.
For example, a register with state 'R1_w=invP0' when we subtract from
it,
w1 -= 2
Before coerce we will now have an smin_value=S64_MIN, smax_value=U64_MAX
and unsigned bounds umin_value=0, umax_value=U64_MAX. These will then
be clamped to S32_MIN, U32_MAX values by coerce in the case of alu32 op
as done in above example. However tnum will be a constant because the
ALU op is done on a constant.
Without update_reg_bounds() we have a scenario where tnum is a const
but our unsigned bounds do not reflect this. By calling update_reg_bounds
after coerce to 32bit we further refine the umin_value to U64_MAX in the
alu64 case or U32_MAX in the alu32 case above.
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://lore.kernel.org/bpf/158507151689.15666.566796274289413203.stgit@john-Precision-5820-Tower
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
kernel/bpf/verifier.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 08f0588fa832..e8d9ddd5cb18 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2739,6 +2739,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
coerce_reg_to_size(dst_reg, 4);
}
+ __update_reg_bounds(dst_reg);
__reg_deduce_bounds(dst_reg);
__reg_bound_offset(dst_reg);
return 0;
--
2.37.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4.14 2/3] selftests/bpf: Fix test_align verifier log patterns
2022-09-06 15:38 [PATCH 4.14 0/3] bpf: fix CVE-2021-4159 Ovidiu Panait
2022-09-06 15:38 ` [PATCH 4.14 1/3] bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds() Ovidiu Panait
@ 2022-09-06 15:38 ` Ovidiu Panait
2022-09-06 15:38 ` [PATCH 4.14 3/3] bpf: Fix the off-by-two error in range markings Ovidiu Panait
2022-09-08 11:56 ` [PATCH 4.14 0/3] bpf: fix CVE-2021-4159 Greg KH
3 siblings, 0 replies; 5+ messages in thread
From: Ovidiu Panait @ 2022-09-06 15:38 UTC (permalink / raw)
To: stable; +Cc: Stanislav Fomichev, Daniel Borkmann, Ovidiu Panait
From: Stanislav Fomichev <sdf@google.com>
commit 5366d2269139ba8eb6a906d73a0819947e3e4e0a upstream.
Commit 294f2fc6da27 ("bpf: Verifer, adjust_scalar_min_max_vals to always
call update_reg_bounds()") changed the way verifier logs some of its state,
adjust the test_align accordingly. Where possible, I tried to not copy-paste
the entire log line and resorted to dropping the last closing brace instead.
Fixes: 294f2fc6da27 ("bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds()")
Signed-off-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20200515194904.229296-1-sdf@google.com
[OP: adjust for 4.14 selftests, apply only the relevant diffs]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
tools/testing/selftests/bpf/test_align.c | 27 ++++++++++++------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_align.c b/tools/testing/selftests/bpf/test_align.c
index 5d530c90779e..6004ae268a80 100644
--- a/tools/testing/selftests/bpf/test_align.c
+++ b/tools/testing/selftests/bpf/test_align.c
@@ -363,15 +363,15 @@ static struct bpf_align_test tests[] = {
* is still (4n), fixed offset is not changed.
* Also, we create a new reg->id.
*/
- {29, "R5=pkt(id=4,off=18,r=0,umax_value=2040,var_off=(0x0; 0x7fc))"},
+ {29, "R5=pkt(id=4,off=18,r=0,umax_value=2040,var_off=(0x0; 0x7fc)"},
/* At the time the word size load is performed from R5,
* its total fixed offset is NET_IP_ALIGN + reg->off (18)
* which is 20. Then the variable offset is (4n), so
* the total offset is 4-byte aligned and meets the
* load's requirements.
*/
- {33, "R4=pkt(id=4,off=22,r=22,umax_value=2040,var_off=(0x0; 0x7fc))"},
- {33, "R5=pkt(id=4,off=18,r=22,umax_value=2040,var_off=(0x0; 0x7fc))"},
+ {33, "R4=pkt(id=4,off=22,r=22,umax_value=2040,var_off=(0x0; 0x7fc)"},
+ {33, "R5=pkt(id=4,off=18,r=22,umax_value=2040,var_off=(0x0; 0x7fc)"},
},
},
{
@@ -414,15 +414,15 @@ static struct bpf_align_test tests[] = {
/* Adding 14 makes R6 be (4n+2) */
{9, "R6=inv(id=0,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc))"},
/* Packet pointer has (4n+2) offset */
- {11, "R5=pkt(id=1,off=0,r=0,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc))"},
- {13, "R4=pkt(id=1,off=4,r=0,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc))"},
+ {11, "R5=pkt(id=1,off=0,r=0,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc)"},
+ {13, "R4=pkt(id=1,off=4,r=0,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc)"},
/* At the time the word size load is performed from R5,
* its total fixed offset is NET_IP_ALIGN + reg->off (0)
* which is 2. Then the variable offset is (4n+2), so
* the total offset is 4-byte aligned and meets the
* load's requirements.
*/
- {15, "R5=pkt(id=1,off=0,r=4,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc))"},
+ {15, "R5=pkt(id=1,off=0,r=4,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc)"},
/* Newly read value in R6 was shifted left by 2, so has
* known alignment of 4.
*/
@@ -430,15 +430,15 @@ static struct bpf_align_test tests[] = {
/* Added (4n) to packet pointer's (4n+2) var_off, giving
* another (4n+2).
*/
- {19, "R5=pkt(id=2,off=0,r=0,umin_value=14,umax_value=2054,var_off=(0x2; 0xffc))"},
- {21, "R4=pkt(id=2,off=4,r=0,umin_value=14,umax_value=2054,var_off=(0x2; 0xffc))"},
+ {19, "R5=pkt(id=2,off=0,r=0,umin_value=14,umax_value=2054,var_off=(0x2; 0xffc)"},
+ {21, "R4=pkt(id=2,off=4,r=0,umin_value=14,umax_value=2054,var_off=(0x2; 0xffc)"},
/* At the time the word size load is performed from R5,
* its total fixed offset is NET_IP_ALIGN + reg->off (0)
* which is 2. Then the variable offset is (4n+2), so
* the total offset is 4-byte aligned and meets the
* load's requirements.
*/
- {23, "R5=pkt(id=2,off=0,r=4,umin_value=14,umax_value=2054,var_off=(0x2; 0xffc))"},
+ {23, "R5=pkt(id=2,off=0,r=4,umin_value=14,umax_value=2054,var_off=(0x2; 0xffc)"},
},
},
{
@@ -473,11 +473,11 @@ static struct bpf_align_test tests[] = {
.matches = {
{4, "R5=pkt_end(id=0,off=0,imm=0)"},
/* (ptr - ptr) << 2 == unknown, (4n) */
- {6, "R5=inv(id=0,smax_value=9223372036854775804,umax_value=18446744073709551612,var_off=(0x0; 0xfffffffffffffffc))"},
+ {6, "R5=inv(id=0,smax_value=9223372036854775804,umax_value=18446744073709551612,var_off=(0x0; 0xfffffffffffffffc)"},
/* (4n) + 14 == (4n+2). We blow our bounds, because
* the add could overflow.
*/
- {7, "R5=inv(id=0,var_off=(0x2; 0xfffffffffffffffc))"},
+ {7, "R5=inv(id=0,smin_value=-9223372036854775806,smax_value=9223372036854775806,umin_value=2,umax_value=18446744073709551614,var_off=(0x2; 0xfffffffffffffffc)"},
/* Checked s>=0 */
{9, "R5=inv(id=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
/* packet pointer + nonnegative (4n+2) */
@@ -532,7 +532,7 @@ static struct bpf_align_test tests[] = {
/* New unknown value in R7 is (4n) */
{11, "R7=inv(id=0,umax_value=1020,var_off=(0x0; 0x3fc))"},
/* Subtracting it from R6 blows our unsigned bounds */
- {12, "R6=inv(id=0,smin_value=-1006,smax_value=1034,var_off=(0x2; 0xfffffffffffffffc))"},
+ {12, "R6=inv(id=0,smin_value=-1006,smax_value=1034,umin_value=2,umax_value=18446744073709551614,var_off=(0x2; 0xfffffffffffffffc)"},
/* Checked s>= 0 */
{14, "R6=inv(id=0,umin_value=2,umax_value=1034,var_off=(0x2; 0x7fc))"},
/* At the time the word size load is performed from R5,
@@ -541,7 +541,8 @@ static struct bpf_align_test tests[] = {
* the total offset is 4-byte aligned and meets the
* load's requirements.
*/
- {20, "R5=pkt(id=1,off=0,r=4,umin_value=2,umax_value=1034,var_off=(0x2; 0x7fc))"},
+ {20, "R5=pkt(id=1,off=0,r=4,umin_value=2,umax_value=1034,var_off=(0x2; 0x7fc)"},
+
},
},
{
--
2.37.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 4.14 3/3] bpf: Fix the off-by-two error in range markings
2022-09-06 15:38 [PATCH 4.14 0/3] bpf: fix CVE-2021-4159 Ovidiu Panait
2022-09-06 15:38 ` [PATCH 4.14 1/3] bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds() Ovidiu Panait
2022-09-06 15:38 ` [PATCH 4.14 2/3] selftests/bpf: Fix test_align verifier log patterns Ovidiu Panait
@ 2022-09-06 15:38 ` Ovidiu Panait
2022-09-08 11:56 ` [PATCH 4.14 0/3] bpf: fix CVE-2021-4159 Greg KH
3 siblings, 0 replies; 5+ messages in thread
From: Ovidiu Panait @ 2022-09-06 15:38 UTC (permalink / raw)
To: stable; +Cc: Maxim Mikityanskiy, Daniel Borkmann, Ovidiu Panait
From: Maxim Mikityanskiy <maximmi@nvidia.com>
commit 2fa7d94afc1afbb4d702760c058dc2d7ed30f226 upstream.
The first commit cited below attempts to fix the off-by-one error that
appeared in some comparisons with an open range. Due to this error,
arithmetically equivalent pieces of code could get different verdicts
from the verifier, for example (pseudocode):
// 1. Passes the verifier:
if (data + 8 > data_end)
return early
read *(u64 *)data, i.e. [data; data+7]
// 2. Rejected by the verifier (should still pass):
if (data + 7 >= data_end)
return early
read *(u64 *)data, i.e. [data; data+7]
The attempted fix, however, shifts the range by one in a wrong
direction, so the bug not only remains, but also such piece of code
starts failing in the verifier:
// 3. Rejected by the verifier, but the check is stricter than in #1.
if (data + 8 >= data_end)
return early
read *(u64 *)data, i.e. [data; data+7]
The change performed by that fix converted an off-by-one bug into
off-by-two. The second commit cited below added the BPF selftests
written to ensure than code chunks like #3 are rejected, however,
they should be accepted.
This commit fixes the off-by-two error by adjusting new_range in the
right direction and fixes the tests by changing the range into the
one that should actually fail.
Fixes: fb2a311a31d3 ("bpf: fix off by one for range markings with L{T, E} patterns")
Fixes: b37242c773b2 ("bpf: add test cases to bpf selftests to cover all access tests")
Signed-off-by: Maxim Mikityanskiy <maximmi@nvidia.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Link: https://lore.kernel.org/bpf/20211130181607.593149-1-maximmi@nvidia.com
[OP: only cherry-pick selftest changes applicable to 4.14]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
tools/testing/selftests/bpf/test_verifier.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 0846345fe1e5..f7757f7f6d2b 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -7438,10 +7438,10 @@ static struct bpf_test tests[] = {
BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
offsetof(struct xdp_md, data_end)),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
- BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 6),
BPF_JMP_REG(BPF_JGT, BPF_REG_3, BPF_REG_1, 1),
BPF_JMP_IMM(BPF_JA, 0, 0, 1),
- BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -6),
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_EXIT_INSN(),
},
@@ -7494,10 +7494,10 @@ static struct bpf_test tests[] = {
BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
offsetof(struct xdp_md, data_end)),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
- BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 6),
BPF_JMP_REG(BPF_JLT, BPF_REG_1, BPF_REG_3, 1),
BPF_JMP_IMM(BPF_JA, 0, 0, 1),
- BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -6),
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_EXIT_INSN(),
},
@@ -7603,9 +7603,9 @@ static struct bpf_test tests[] = {
BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
offsetof(struct xdp_md, data_end)),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
- BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 6),
BPF_JMP_REG(BPF_JGE, BPF_REG_1, BPF_REG_3, 1),
- BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -6),
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_EXIT_INSN(),
},
@@ -7770,9 +7770,9 @@ static struct bpf_test tests[] = {
BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
offsetof(struct xdp_md, data_end)),
BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
- BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 8),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, 6),
BPF_JMP_REG(BPF_JLE, BPF_REG_3, BPF_REG_1, 1),
- BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -6),
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_EXIT_INSN(),
},
--
2.37.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 4.14 0/3] bpf: fix CVE-2021-4159
2022-09-06 15:38 [PATCH 4.14 0/3] bpf: fix CVE-2021-4159 Ovidiu Panait
` (2 preceding siblings ...)
2022-09-06 15:38 ` [PATCH 4.14 3/3] bpf: Fix the off-by-two error in range markings Ovidiu Panait
@ 2022-09-08 11:56 ` Greg KH
3 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2022-09-08 11:56 UTC (permalink / raw)
To: Ovidiu Panait; +Cc: stable
On Tue, Sep 06, 2022 at 06:38:52PM +0300, Ovidiu Panait wrote:
> All test_verifier/test_align selftests pass in qemu for x86-64 with this
> series applied:
> root@intel-x86-64:~# ./test_verifier
> ...
> #433/p xadd/w check unaligned pkt OK
> #434/p pass unmodified ctx pointer to helper OK
> #435/p pass modified ctx pointer to helper, 1 OK
> #436/u pass modified ctx pointer to helper, 2 OK
> #436/p pass modified ctx pointer to helper, 2 OK
> #437/p pass modified ctx pointer to helper, 3 OK
> Summary: 667 PASSED, 0 FAILED
>
> root@intel-x86-64:~# ./test_align
> Test 0: mov ... PASS
> Test 1: shift ... PASS
> Test 2: addsub ... PASS
> Test 3: mul ... PASS
> Test 4: unknown shift ... PASS
> Test 5: unknown mul ... PASS
> Test 6: packet const offset ... PASS
> Test 7: packet variable offset ... PASS
> Test 8: packet variable offset 2 ... PASS
> Test 9: dubious pointer arithmetic ... PASS
> Test 10: variable subtraction ... PASS
> Test 11: pointer variable subtraction ... PASS
> Results: 12 pass 0 fail
>
>
> John Fastabend (1):
> bpf: Verifer, adjust_scalar_min_max_vals to always call
> update_reg_bounds()
>
> Maxim Mikityanskiy (1):
> bpf: Fix the off-by-two error in range markings
>
> Stanislav Fomichev (1):
> selftests/bpf: Fix test_align verifier log patterns
>
> kernel/bpf/verifier.c | 1 +
> tools/testing/selftests/bpf/test_align.c | 27 +++++++++++----------
> tools/testing/selftests/bpf/test_verifier.c | 16 ++++++------
> 3 files changed, 23 insertions(+), 21 deletions(-)
>
> --
> 2.37.2
>
All now queued up, thanks.
greg k-h
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-09-08 11:56 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-06 15:38 [PATCH 4.14 0/3] bpf: fix CVE-2021-4159 Ovidiu Panait
2022-09-06 15:38 ` [PATCH 4.14 1/3] bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds() Ovidiu Panait
2022-09-06 15:38 ` [PATCH 4.14 2/3] selftests/bpf: Fix test_align verifier log patterns Ovidiu Panait
2022-09-06 15:38 ` [PATCH 4.14 3/3] bpf: Fix the off-by-two error in range markings Ovidiu Panait
2022-09-08 11:56 ` [PATCH 4.14 0/3] bpf: fix CVE-2021-4159 Greg KH
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.