* [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings"
2022-08-09 7:39 [PATCH 4.19 0/4] bpf: fix CVE-2021-4159 Ovidiu Panait
@ 2022-08-09 7:39 ` Ovidiu Panait
2022-08-13 13:10 ` Greg KH
2022-08-09 7:39 ` [PATCH 4.19 2/4] bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds() Ovidiu Panait
` (3 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Ovidiu Panait @ 2022-08-09 7:39 UTC (permalink / raw)
To: stable; +Cc: Maxim Mikityanskiy, Ovidiu Panait
From: Maxim Mikityanskiy <maximmi@nvidia.com>
The 4.19 backport of upstream commit 2fa7d94afc1a ("bpf: Fix the off-by-two
error in range markings") did not include the selftest changes, so currently
there are 8 verifier selftests that are failing:
# root@intel-x86-64:~# ./test_verifier
...
#495/p XDP pkt read, pkt_end > pkt_data', bad access 1 FAIL
#498/p XDP pkt read, pkt_data' < pkt_end, bad access 1 FAIL
#504/p XDP pkt read, pkt_data' >= pkt_end, bad access 1 FAIL
#513/p XDP pkt read, pkt_end <= pkt_data', bad access 1 FAIL
#519/p XDP pkt read, pkt_data > pkt_meta', bad access 1 FAIL
#522/p XDP pkt read, pkt_meta' < pkt_data, bad access 1 FAIL
#528/p XDP pkt read, pkt_meta' >= pkt_data, bad access 1 FAIL
#537/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 FAIL
Summary: 924 PASSED, 0 SKIPPED, 8 FAILED
Cherry-pick the selftest changes to fix these.
Fixes: c315bd962528 ("bpf: Fix the off-by-two error in range markings")
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
tools/testing/selftests/bpf/test_verifier.c | 32 ++++++++++-----------
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 858e55143233..9a103bd3542c 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -9108,10 +9108,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(),
},
@@ -9166,10 +9166,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(),
},
@@ -9279,9 +9279,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(),
},
@@ -9451,9 +9451,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(),
},
@@ -9564,10 +9564,10 @@ static struct bpf_test tests[] = {
BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
offsetof(struct xdp_md, data)),
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(),
},
@@ -9622,10 +9622,10 @@ static struct bpf_test tests[] = {
BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
offsetof(struct xdp_md, data)),
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(),
},
@@ -9735,9 +9735,9 @@ static struct bpf_test tests[] = {
BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
offsetof(struct xdp_md, data)),
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(),
},
@@ -9907,9 +9907,9 @@ static struct bpf_test tests[] = {
BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
offsetof(struct xdp_md, data)),
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.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings"
2022-08-09 7:39 ` [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings" Ovidiu Panait
@ 2022-08-13 13:10 ` Greg KH
2022-08-14 13:58 ` Ovidiu Panait
0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2022-08-13 13:10 UTC (permalink / raw)
To: Ovidiu Panait; +Cc: stable, Maxim Mikityanskiy
On Tue, Aug 09, 2022 at 10:39:44AM +0300, Ovidiu Panait wrote:
> From: Maxim Mikityanskiy <maximmi@nvidia.com>
>
> The 4.19 backport of upstream commit 2fa7d94afc1a ("bpf: Fix the off-by-two
> error in range markings") did not include the selftest changes, so currently
> there are 8 verifier selftests that are failing:
> # root@intel-x86-64:~# ./test_verifier
> ...
> #495/p XDP pkt read, pkt_end > pkt_data', bad access 1 FAIL
> #498/p XDP pkt read, pkt_data' < pkt_end, bad access 1 FAIL
> #504/p XDP pkt read, pkt_data' >= pkt_end, bad access 1 FAIL
> #513/p XDP pkt read, pkt_end <= pkt_data', bad access 1 FAIL
> #519/p XDP pkt read, pkt_data > pkt_meta', bad access 1 FAIL
> #522/p XDP pkt read, pkt_meta' < pkt_data, bad access 1 FAIL
> #528/p XDP pkt read, pkt_meta' >= pkt_data, bad access 1 FAIL
> #537/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 FAIL
> Summary: 924 PASSED, 0 SKIPPED, 8 FAILED
>
> Cherry-pick the selftest changes to fix these.
What specific "selftest changes" are you cherry-picking here? I can't
take this commit without that reference.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings"
2022-08-13 13:10 ` Greg KH
@ 2022-08-14 13:58 ` Ovidiu Panait
2022-08-14 14:44 ` Greg KH
0 siblings, 1 reply; 11+ messages in thread
From: Ovidiu Panait @ 2022-08-14 13:58 UTC (permalink / raw)
To: Greg KH; +Cc: stable, Maxim Mikityanskiy
Hi Greg,
On 8/13/22 16:10, Greg KH wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Tue, Aug 09, 2022 at 10:39:44AM +0300, Ovidiu Panait wrote:
>> From: Maxim Mikityanskiy <maximmi@nvidia.com>
>>
>> The 4.19 backport of upstream commit 2fa7d94afc1a ("bpf: Fix the off-by-two
>> error in range markings") did not include the selftest changes, so currently
>> there are 8 verifier selftests that are failing:
>> # root@intel-x86-64:~# ./test_verifier
>> ...
>> #495/p XDP pkt read, pkt_end > pkt_data', bad access 1 FAIL
>> #498/p XDP pkt read, pkt_data' < pkt_end, bad access 1 FAIL
>> #504/p XDP pkt read, pkt_data' >= pkt_end, bad access 1 FAIL
>> #513/p XDP pkt read, pkt_end <= pkt_data', bad access 1 FAIL
>> #519/p XDP pkt read, pkt_data > pkt_meta', bad access 1 FAIL
>> #522/p XDP pkt read, pkt_meta' < pkt_data, bad access 1 FAIL
>> #528/p XDP pkt read, pkt_meta' >= pkt_data, bad access 1 FAIL
>> #537/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 FAIL
>> Summary: 924 PASSED, 0 SKIPPED, 8 FAILED
>>
>> Cherry-pick the selftest changes to fix these.
> What specific "selftest changes" are you cherry-picking here? I can't
> take this commit without that reference.
This patch includes the selftest part of upstream commit:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=2fa7d94afc1a
The 4.19 backport of the above commit did not include the selftest updates:
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=c315bd962528
Ovidiu
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings"
2022-08-14 13:58 ` Ovidiu Panait
@ 2022-08-14 14:44 ` Greg KH
2022-08-14 15:08 ` Ovidiu Panait
0 siblings, 1 reply; 11+ messages in thread
From: Greg KH @ 2022-08-14 14:44 UTC (permalink / raw)
To: Ovidiu Panait; +Cc: stable, Maxim Mikityanskiy
On Sun, Aug 14, 2022 at 04:58:56PM +0300, Ovidiu Panait wrote:
> Hi Greg,
>
> On 8/13/22 16:10, Greg KH wrote:
> > [Please note: This e-mail is from an EXTERNAL e-mail address]
> >
> > On Tue, Aug 09, 2022 at 10:39:44AM +0300, Ovidiu Panait wrote:
> > > From: Maxim Mikityanskiy <maximmi@nvidia.com>
> > >
> > > The 4.19 backport of upstream commit 2fa7d94afc1a ("bpf: Fix the off-by-two
> > > error in range markings") did not include the selftest changes, so currently
> > > there are 8 verifier selftests that are failing:
> > > # root@intel-x86-64:~# ./test_verifier
> > > ...
> > > #495/p XDP pkt read, pkt_end > pkt_data', bad access 1 FAIL
> > > #498/p XDP pkt read, pkt_data' < pkt_end, bad access 1 FAIL
> > > #504/p XDP pkt read, pkt_data' >= pkt_end, bad access 1 FAIL
> > > #513/p XDP pkt read, pkt_end <= pkt_data', bad access 1 FAIL
> > > #519/p XDP pkt read, pkt_data > pkt_meta', bad access 1 FAIL
> > > #522/p XDP pkt read, pkt_meta' < pkt_data, bad access 1 FAIL
> > > #528/p XDP pkt read, pkt_meta' >= pkt_data, bad access 1 FAIL
> > > #537/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 FAIL
> > > Summary: 924 PASSED, 0 SKIPPED, 8 FAILED
> > >
> > > Cherry-pick the selftest changes to fix these.
> > What specific "selftest changes" are you cherry-picking here? I can't
> > take this commit without that reference.
>
> This patch includes the selftest part of upstream commit:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=2fa7d94afc1a
>
>
> The 4.19 backport of the above commit did not include the selftest updates:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=c315bd962528
I still do not understand, what commit is this coming from that matches
it up with what is in Linus's tree?
confused,
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings"
2022-08-14 14:44 ` Greg KH
@ 2022-08-14 15:08 ` Ovidiu Panait
2022-08-15 12:15 ` Greg KH
0 siblings, 1 reply; 11+ messages in thread
From: Ovidiu Panait @ 2022-08-14 15:08 UTC (permalink / raw)
To: Greg KH; +Cc: stable
On 8/14/22 17:44, Greg KH wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Sun, Aug 14, 2022 at 04:58:56PM +0300, Ovidiu Panait wrote:
>> Hi Greg,
>>
>> On 8/13/22 16:10, Greg KH wrote:
>>> [Please note: This e-mail is from an EXTERNAL e-mail address]
>>>
>>> On Tue, Aug 09, 2022 at 10:39:44AM +0300, Ovidiu Panait wrote:
>>>> From: Maxim Mikityanskiy <maximmi@nvidia.com>
>>>>
>>>> The 4.19 backport of upstream commit 2fa7d94afc1a ("bpf: Fix the off-by-two
>>>> error in range markings") did not include the selftest changes, so currently
>>>> there are 8 verifier selftests that are failing:
>>>> # root@intel-x86-64:~# ./test_verifier
>>>> ...
>>>> #495/p XDP pkt read, pkt_end > pkt_data', bad access 1 FAIL
>>>> #498/p XDP pkt read, pkt_data' < pkt_end, bad access 1 FAIL
>>>> #504/p XDP pkt read, pkt_data' >= pkt_end, bad access 1 FAIL
>>>> #513/p XDP pkt read, pkt_end <= pkt_data', bad access 1 FAIL
>>>> #519/p XDP pkt read, pkt_data > pkt_meta', bad access 1 FAIL
>>>> #522/p XDP pkt read, pkt_meta' < pkt_data, bad access 1 FAIL
>>>> #528/p XDP pkt read, pkt_meta' >= pkt_data, bad access 1 FAIL
>>>> #537/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 FAIL
>>>> Summary: 924 PASSED, 0 SKIPPED, 8 FAILED
>>>>
>>>> Cherry-pick the selftest changes to fix these.
>>> What specific "selftest changes" are you cherry-picking here? I can't
>>> take this commit without that reference.
>> This patch includes the selftest part of upstream commit:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=2fa7d94afc1a
>>
>>
>> The 4.19 backport of the above commit did not include the selftest updates:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=c315bd962528
> I still do not understand, what commit is this coming from that matches
> it up with what is in Linus's tree?
The changes come from commit 2fa7d94afc1a ("bpf: Fix the off-by-two
error in range markings") in Linus's tree.
Ovidiu
>
> confused,
>
> greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings"
2022-08-14 15:08 ` Ovidiu Panait
@ 2022-08-15 12:15 ` Greg KH
0 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2022-08-15 12:15 UTC (permalink / raw)
To: Ovidiu Panait; +Cc: stable
On Sun, Aug 14, 2022 at 06:08:26PM +0300, Ovidiu Panait wrote:
>
> On 8/14/22 17:44, Greg KH wrote:
> > [Please note: This e-mail is from an EXTERNAL e-mail address]
> >
> > On Sun, Aug 14, 2022 at 04:58:56PM +0300, Ovidiu Panait wrote:
> > > Hi Greg,
> > >
> > > On 8/13/22 16:10, Greg KH wrote:
> > > > [Please note: This e-mail is from an EXTERNAL e-mail address]
> > > >
> > > > On Tue, Aug 09, 2022 at 10:39:44AM +0300, Ovidiu Panait wrote:
> > > > > From: Maxim Mikityanskiy <maximmi@nvidia.com>
> > > > >
> > > > > The 4.19 backport of upstream commit 2fa7d94afc1a ("bpf: Fix the off-by-two
> > > > > error in range markings") did not include the selftest changes, so currently
> > > > > there are 8 verifier selftests that are failing:
> > > > > # root@intel-x86-64:~# ./test_verifier
> > > > > ...
> > > > > #495/p XDP pkt read, pkt_end > pkt_data', bad access 1 FAIL
> > > > > #498/p XDP pkt read, pkt_data' < pkt_end, bad access 1 FAIL
> > > > > #504/p XDP pkt read, pkt_data' >= pkt_end, bad access 1 FAIL
> > > > > #513/p XDP pkt read, pkt_end <= pkt_data', bad access 1 FAIL
> > > > > #519/p XDP pkt read, pkt_data > pkt_meta', bad access 1 FAIL
> > > > > #522/p XDP pkt read, pkt_meta' < pkt_data, bad access 1 FAIL
> > > > > #528/p XDP pkt read, pkt_meta' >= pkt_data, bad access 1 FAIL
> > > > > #537/p XDP pkt read, pkt_data <= pkt_meta', bad access 1 FAIL
> > > > > Summary: 924 PASSED, 0 SKIPPED, 8 FAILED
> > > > >
> > > > > Cherry-pick the selftest changes to fix these.
> > > > What specific "selftest changes" are you cherry-picking here? I can't
> > > > take this commit without that reference.
> > > This patch includes the selftest part of upstream commit:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=2fa7d94afc1a
> > >
> > >
> > > The 4.19 backport of the above commit did not include the selftest updates:
> > >
> > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=c315bd962528
> > I still do not understand, what commit is this coming from that matches
> > it up with what is in Linus's tree?
> The changes come from commit 2fa7d94afc1a ("bpf: Fix the off-by-two error in
> range markings") in Linus's tree.
Then can you please reword the text here to make it obvious what is
happening and resubmit?
thanks,
gre gk-h
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4.19 2/4] bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds()
2022-08-09 7:39 [PATCH 4.19 0/4] bpf: fix CVE-2021-4159 Ovidiu Panait
2022-08-09 7:39 ` [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings" Ovidiu Panait
@ 2022-08-09 7:39 ` Ovidiu Panait
2022-08-09 7:39 ` [PATCH 4.19 3/4] selftests/bpf: Fix test_align verifier log patterns Ovidiu Panait
` (2 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Ovidiu Panait @ 2022-08-09 7:39 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 30ac8ee8294c..694ee0b1fefe 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3496,6 +3496,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.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4.19 3/4] selftests/bpf: Fix test_align verifier log patterns
2022-08-09 7:39 [PATCH 4.19 0/4] bpf: fix CVE-2021-4159 Ovidiu Panait
2022-08-09 7:39 ` [PATCH 4.19 1/4] selftests/bpf: add selftest part of "bpf: Fix the off-by-two error in range markings" Ovidiu Panait
2022-08-09 7:39 ` [PATCH 4.19 2/4] bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds() Ovidiu Panait
@ 2022-08-09 7:39 ` Ovidiu Panait
2022-08-09 7:39 ` [PATCH 4.19 4/4] selftests/bpf: Fix "dubious pointer arithmetic" test Ovidiu Panait
2022-08-13 13:12 ` [PATCH 4.19 0/4] bpf: fix CVE-2021-4159 Greg KH
4 siblings, 0 replies; 11+ messages in thread
From: Ovidiu Panait @ 2022-08-09 7:39 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.19 selftests]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
tools/testing/selftests/bpf/test_align.c | 41 ++++++++++++------------
1 file changed, 21 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_align.c b/tools/testing/selftests/bpf/test_align.c
index 3c789d03b629..7e057b47b27a 100644
--- a/tools/testing/selftests/bpf/test_align.c
+++ b/tools/testing/selftests/bpf/test_align.c
@@ -359,15 +359,15 @@ static struct bpf_align_test tests[] = {
* is still (4n), fixed offset is not changed.
* Also, we create a new reg->id.
*/
- {29, "R5_w=pkt(id=4,off=18,r=0,umax_value=2040,var_off=(0x0; 0x7fc))"},
+ {29, "R5_w=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)"},
},
},
{
@@ -410,15 +410,15 @@ static struct bpf_align_test tests[] = {
/* Adding 14 makes R6 be (4n+2) */
{9, "R6_w=inv(id=0,umin_value=14,umax_value=1034,var_off=(0x2; 0x7fc))"},
/* Packet pointer has (4n+2) offset */
- {11, "R5_w=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_w=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.
*/
@@ -426,15 +426,15 @@ static struct bpf_align_test tests[] = {
/* Added (4n) to packet pointer's (4n+2) var_off, giving
* another (4n+2).
*/
- {19, "R5_w=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_w=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)"},
},
},
{
@@ -469,16 +469,16 @@ static struct bpf_align_test tests[] = {
.matches = {
{4, "R5_w=pkt_end(id=0,off=0,imm=0)"},
/* (ptr - ptr) << 2 == unknown, (4n) */
- {6, "R5_w=inv(id=0,smax_value=9223372036854775804,umax_value=18446744073709551612,var_off=(0x0; 0xfffffffffffffffc))"},
+ {6, "R5_w=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))"},
+ {9, "R5=inv(id=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
/* packet pointer + nonnegative (4n+2) */
- {11, "R6_w=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
- {13, "R4=pkt(id=1,off=4,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
+ {11, "R6_w=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
+ {13, "R4=pkt(id=1,off=4,r=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
/* NET_IP_ALIGN + (4n+2) == (4n), alignment is fine.
* We checked the bounds, but it might have been able
* to overflow if the packet pointer started in the
@@ -486,7 +486,7 @@ static struct bpf_align_test tests[] = {
* So we did not get a 'range' on R6, and the access
* attempt will fail.
*/
- {15, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc))"},
+ {15, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
}
},
{
@@ -528,7 +528,7 @@ static struct bpf_align_test tests[] = {
/* New unknown value in R7 is (4n) */
{11, "R7_w=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,
@@ -537,7 +537,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)"},
+
},
},
{
@@ -579,18 +580,18 @@ static struct bpf_align_test tests[] = {
/* Adding 14 makes R6 be (4n+2) */
{11, "R6_w=inv(id=0,umin_value=14,umax_value=74,var_off=(0x2; 0x7c))"},
/* Subtracting from packet pointer overflows ubounds */
- {13, "R5_w=pkt(id=1,off=0,r=8,umin_value=18446744073709551542,umax_value=18446744073709551602,var_off=(0xffffffffffffff82; 0x7c))"},
+ {13, "R5_w=pkt(id=1,off=0,r=8,umin_value=18446744073709551542,umax_value=18446744073709551602,var_off=(0xffffffffffffff82; 0x7c)"},
/* New unknown value in R7 is (4n), >= 76 */
{15, "R7_w=inv(id=0,umin_value=76,umax_value=1096,var_off=(0x0; 0x7fc))"},
/* Adding it to packet pointer gives nice bounds again */
- {16, "R5_w=pkt(id=2,off=0,r=0,umin_value=2,umax_value=1082,var_off=(0x2; 0x7fc))"},
+ {16, "R5_w=pkt(id=2,off=0,r=0,umin_value=2,umax_value=1082,var_off=(0x2; 0xfffffffc)"},
/* 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.
*/
- {20, "R5=pkt(id=2,off=0,r=4,umin_value=2,umax_value=1082,var_off=(0x2; 0x7fc))"},
+ {20, "R5=pkt(id=2,off=0,r=4,umin_value=2,umax_value=1082,var_off=(0x2; 0xfffffffc)"},
},
},
};
--
2.37.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 4.19 4/4] selftests/bpf: Fix "dubious pointer arithmetic" test
2022-08-09 7:39 [PATCH 4.19 0/4] bpf: fix CVE-2021-4159 Ovidiu Panait
` (2 preceding siblings ...)
2022-08-09 7:39 ` [PATCH 4.19 3/4] selftests/bpf: Fix test_align verifier log patterns Ovidiu Panait
@ 2022-08-09 7:39 ` Ovidiu Panait
2022-08-13 13:12 ` [PATCH 4.19 0/4] bpf: fix CVE-2021-4159 Greg KH
4 siblings, 0 replies; 11+ messages in thread
From: Ovidiu Panait @ 2022-08-09 7:39 UTC (permalink / raw)
To: stable
Cc: Jean-Philippe Brucker, John Fastabend, Alexei Starovoitov,
Ovidiu Panait
From: Jean-Philippe Brucker <jean-philippe@linaro.org>
commit 3615bdf6d9b19db12b1589861609b4f1c6a8d303 upstream.
The verifier trace changed following a bugfix. After checking the 64-bit
sign, only the upper bit mask is known, not bit 31. Update the test
accordingly.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
Acked-by: John Fastabend <john.fastabend@gmail.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
[OP: adjust for 4.19 selftests]
Signed-off-by: Ovidiu Panait <ovidiu.panait@windriver.com>
---
tools/testing/selftests/bpf/test_align.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_align.c b/tools/testing/selftests/bpf/test_align.c
index 7e057b47b27a..30bca0d82ad3 100644
--- a/tools/testing/selftests/bpf/test_align.c
+++ b/tools/testing/selftests/bpf/test_align.c
@@ -475,10 +475,10 @@ static struct bpf_align_test tests[] = {
*/
{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=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
+ {9, "R5=inv(id=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc)"},
/* packet pointer + nonnegative (4n+2) */
- {11, "R6_w=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
- {13, "R4=pkt(id=1,off=4,r=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
+ {11, "R6_w=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc)"},
+ {13, "R4=pkt(id=1,off=4,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc)"},
/* NET_IP_ALIGN + (4n+2) == (4n), alignment is fine.
* We checked the bounds, but it might have been able
* to overflow if the packet pointer started in the
@@ -486,7 +486,7 @@ static struct bpf_align_test tests[] = {
* So we did not get a 'range' on R6, and the access
* attempt will fail.
*/
- {15, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2; 0x7fffffff7ffffffc)"},
+ {15, "R6=pkt(id=1,off=0,r=0,umin_value=2,umax_value=9223372036854775806,var_off=(0x2; 0x7ffffffffffffffc)"},
}
},
{
--
2.37.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4.19 0/4] bpf: fix CVE-2021-4159
2022-08-09 7:39 [PATCH 4.19 0/4] bpf: fix CVE-2021-4159 Ovidiu Panait
` (3 preceding siblings ...)
2022-08-09 7:39 ` [PATCH 4.19 4/4] selftests/bpf: Fix "dubious pointer arithmetic" test Ovidiu Panait
@ 2022-08-13 13:12 ` Greg KH
4 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2022-08-13 13:12 UTC (permalink / raw)
To: Ovidiu Panait; +Cc: stable
On Tue, Aug 09, 2022 at 10:39:43AM +0300, Ovidiu Panait wrote:
> All verifier selftests pass in qemu for x86-64 with this series applied:
> root@intel-x86-64:~# ./test_verifier
> ...
> #664/p mov64 src == dst OK
> #665/p mov64 src != dst OK
> #666/u calls: ctx read at start of subprog OK
> #666/p calls: ctx read at start of subprog OK
> Summary: 932 PASSED, 0 SKIPPED, 0 FAILED
>
> Jean-Philippe Brucker (1):
> selftests/bpf: Fix "dubious pointer arithmetic" test
>
> John Fastabend (1):
> bpf: Verifer, adjust_scalar_min_max_vals to always call
> update_reg_bounds()
>
> Maxim Mikityanskiy (1):
> selftests/bpf: add selftest part of "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 | 41 +++++++++++----------
> tools/testing/selftests/bpf/test_verifier.c | 32 ++++++++--------
> 3 files changed, 38 insertions(+), 36 deletions(-)
>
> --
> 2.37.1
>
Patches 2-4 now queued up, thanks.
greg k-h
^ permalink raw reply [flat|nested] 11+ messages in thread