[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <94756777-b5b4-4db1-fe52-2386609f8a86@windriver.com>
Date: Tue, 23 Aug 2022 23:25:15 +0300
From: Ovidiu Panait <ovidiu.panait@...driver.com>
To: Jean-Philippe Brucker <jean-philippe@...aro.org>,
RAJESH DASARI <raajeshdasari@...il.com>
CC: Greg KH <gregkh@...uxfoundation.org>, <stable@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <alexei.starovoitov@...il.com>,
<john.fastabend@...il.com>
Subject: Re: bpf selftest failed in 5.4.210 kernel
Hi Jean-Philippe,
On 8/23/22 21:34, Jean-Philippe Brucker wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> On Tue, Aug 23, 2022 at 10:31:40AM +0300, RAJESH DASARI wrote:
>> Sorry for the confusion, results are indeed confusing to me .
>> If I try with git bisect I get
>>
>> git bisect bad
>> 9d6f67365d9cdb389fbdac2bb5b00e59e345930e is the first bad commit
> For me bisecting points to:
>
> (A) 7c1134c7da99 ("bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds()")
>
> This changes the BPF verifier output and (as expected) breaks the
> test_align selftest. That's why in the same series [1] another patch fixed
> test_align. In v5.4.y, that patch is:
>
> (B) 6a9b3f0f3bad ("selftests/bpf: Fix test_align verifier log patterns")
>
> Unfortunately commit (B) addresses multiple verifier changes, not solely
> (A). My guess is those changes were in series [1] and haven't been
> backported to v5.4. So multiple solutions:
>
> * Partially revert (B), only keeping the changes needed by (A)
> * Revert (A) and (B)
> * Add the missing commits that (B) also addresses
>
> I don't know which, I suppose it depends on the intent behind backporting
> (A). Ovidiu?
The intent behind backporting 7c1134c7da99 ("bpf: Verifer,
adjust_scalar_min_max_vals to always call update_reg_bounds()") was to
fix CVE-2021-4159.
If we revert test 11 changes brought in by 6a9b3f0f3bad ("selftests/bpf:
Fix test_align verifier log patterns") backport, all test_align
testcases pass on my side:
diff --git a/tools/testing/selftests/bpf/test_align.c
b/tools/testing/selftests/bpf/test_align.c
index c9c9bdce9d6d..4726e3eca9b2 100644
--- a/tools/testing/selftests/bpf/test_align.c
+++ b/tools/testing/selftests/bpf/test_align.c
@@ -580,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;
0xfffffffc)"},
+ {16,
"R5_w=pkt(id=2,off=0,r=0,umin_value=2,umax_value=1082,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.
*/
- {20,
"R5=pkt(id=2,off=0,r=4,umin_value=2,umax_value=1082,var_off=(0x2;
0xfffffffc)"},
+ {20,
"R5=pkt(id=2,off=0,r=4,umin_value=2,umax_value=1082,var_off=(0x2;
0x7fc))"},
},
},
};
root@...el-x86-64:~/bpf# ./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
> In any case 6098562ed9df ("selftests/bpf: Fix "dubious pointer arithmetic"
> test") can be reverted, I can send that once we figure out the rest.
In my testing, with [1] and [2] applied, but without [3], the following
test_align selftest would still fail:
Test 9: dubious pointer arithmetic ... Failed to find match 9:
R5=inv(id=0,umin_value=2,umax_value=9223372034707292158,var_off=(0x2;
0x7fffffff7ffffffc)
[1] 7c1134c7da99 ("bpf: Verifer, adjust_scalar_min_max_vals to always
call update_reg_bounds()")
[2] 6a9b3f0f3bad ("selftests/bpf: Fix test_align verifier log patterns")
[3] 6098562ed9df ("selftests/bpf: Fix "dubious pointer arithmetic" test")
Ovidiu
> Thanks,
> Jean
>
> [1] https://lore.kernel.org/bpf/158507130343.15666.8018068546764556975.stgit@john-Precision-5820-Tower/
>
>> If I try to test myself with multiple test scenarios(I have mentioned
>> in the previous mails) for the bad commits , I see that bad commits
>> are
>> bpf: Verifer, adjust_scalar_min_max_vals to always call update_reg_bounds()
>> selftests/bpf: Fix test_align verifier log patterns
>> selftests/bpf: Fix "dubious pointer arithmetic" test
>>
>> Thanks,
>> Rajesh Dasari.
>>
>> On Tue, Aug 23, 2022 at 10:04 AM Greg KH <gregkh@...uxfoundation.org> wrote:
>>> On Mon, Aug 22, 2022 at 10:23:02PM +0300, RAJESH DASARI wrote:
>>>> Hi,
>>>>
>>>> Please find the test scenarios which I have tried.
>>>>
>>>> Test 1:
>>>>
>>>> Running system Kernel version (tag/commit) : v5.4.210
>>>> Kernel source code checkout : v5.4.210
>>>> test_align test case execution status : Failure
>>>>
>>>> Test 2:
>>>>
>>>> Running system Kernel version (tag/commit) : v5.4.210
>>>> Kernel source code checkout : v5.4.209
>>>> test_align test case execution status : Failure
>>>>
>>>> Test 3:
>>>>
>>>> Running system Kernel version (tag/commit) : v5.4.209
>>>> Kernel source code checkout : v5.4.209
>>>> test_align test case execution status : Success
>>>>
>>>> Test 4:
>>>>
>>>> Running system Kernel version (tag/commit) : ACPI: APEI: Better fix to
>>>> avoid spamming the console with old error logs ( Kernel compiled at
>>>> this commit and system is booted with this change)
>>>> Kernel source code checkout : v5.4.210 but reverted selftests/bpf: Fix
>>>> test_align verifier log patterns and selftests/bpf: Fix "dubious
>>>> pointer arithmetic" test. If I revert only the Fix "dubious pointer
>>>> arithmetic" test, the testcase still fails.
>>>> test_align test case execution status : Success
>>>>
>>>> Test 5:
>>>>
>>>> Running system Kernel version (tag/commit) : v5.4.210 but reverted
>>>> commit (bpf: Verifer, adjust_scalar_min_max_vals to always call
>>>> update_reg_bounds() )
>>>> Kernel source code checkout : v5.4.210 but reverted selftests/bpf: Fix
>>>> test_align verifier log patterns and selftests/bpf: Fix "dubious
>>>> pointer arithmetic" test.
>>>> test_align test case execution status : Success
>>>>
>>>> Test 6 :
>>>>
>>>> Running system Kernel version (tag/commit) : bpf: Test_verifier, #70
>>>> error message updates for 32-bit right shift( Kernel compiled at this
>>>> commit and system is booted with this change)
>>>> Kernel source code checkout : v5.4.209 or v5.4.210
>>>> test_align test case execution status : Failure
>>> I'm sorry, but I don't know what to do with this report at all.
>>>
>>> Is there some failure somewhere? If you use 'git bisect' do you find
>>> the offending commit?
>>>
>>> confused,
>>>
>>> greg k-h
Powered by blists - more mailing lists