[5.10, 5.15] New bpf kselftest failure

[5.10, 5.15] New bpf kselftest failure

[Luiz Capitulino]

Hi,

The upstream commit below is backported to 5.10.186, 5.15.120 and 6.1.36:

"""
commit ecdf985d7615356b78241fdb159c091830ed0380
Author: Eduard Zingerman <eddyz87@gmail.com>
Date:   Wed Feb 15 01:20:27 2023 +0200

     bpf: track immediate values written to stack by BPF_ST instruction
"""

This commit is causing the following bpf:test_verifier kselftest to fail:

"""
# #760/p precise: ST insn causing spi > allocated_stack FAIL
"""

Since this test didn't fail before ecdf985d76 backport, the question is
if this is a test bug or if this commit introduced a regression.

I haven't checked if this failure is present in latest Linus tree because
I was unable to build & run the bpf kselftests in an older distro.

Also, there some important details about running the bpf kselftests
in 5.10 and 5.15:

* On 5.10, bpf kselftest build is broken. The following upstream
commit needs to be cherry-picked for it to build & run:

"""
commit 4237e9f4a96228ccc8a7abe5e4b30834323cd353
Author: Gilad Reti <gilad.reti@gmail.com>
Date:   Wed Jan 13 07:38:08 2021 +0200

     selftests/bpf: Add verifier test for PTR_TO_MEM spill
"""

* On 5.15.120 there's one additional test that's failing, but I didn't
debug this one:

"""
#150/p calls: trigger reg2btf_ids[reg→type] for reg→type > __BPF_REG_TYPE_MAX FAIL
FAIL
"""

* On 5.11 onwards, building and running bpf tests is disabled by
default by commit 7a6eb7c34a78498742b5f82543b7a68c1c443329, so I wonder
if we want to backport this to 5.10 as well?

Thanks!

- Luiz

Re: [5.10, 5.15] New bpf kselftest failure

[Eduard Zingerman]

On Mon, 2023-07-17 at 09:04 -0400, Luiz Capitulino wrote:
> Hi,
> 
> The upstream commit below is backported to 5.10.186, 5.15.120 and 6.1.36:
> 
> """
> commit ecdf985d7615356b78241fdb159c091830ed0380
> Author: Eduard Zingerman <eddyz87@gmail.com>
> Date:   Wed Feb 15 01:20:27 2023 +0200
> 
>      bpf: track immediate values written to stack by BPF_ST instruction
> """
> 
> This commit is causing the following bpf:test_verifier kselftest to fail:
> 
> """
> # #760/p precise: ST insn causing spi > allocated_stack FAIL
> """
> 

I can reproduce the error on 6.1.36 but don't understand what's causing it yet.
The log is suspiciously different from master, will comment later today.

> Since this test didn't fail before ecdf985d76 backport, the question is
> if this is a test bug or if this commit introduced a regression.
> 
> I haven't checked if this failure is present in latest Linus tree because
> I was unable to build & run the bpf kselftests in an older distro.
> 
> Also, there some important details about running the bpf kselftests
> in 5.10 and 5.15:
> 
> * On 5.10, bpf kselftest build is broken. The following upstream
> commit needs to be cherry-picked for it to build & run:
> 
> """
> commit 4237e9f4a96228ccc8a7abe5e4b30834323cd353
> Author: Gilad Reti <gilad.reti@gmail.com>
> Date:   Wed Jan 13 07:38:08 2021 +0200
> 
>      selftests/bpf: Add verifier test for PTR_TO_MEM spill
> """
> 
> * On 5.15.120 there's one additional test that's failing, but I didn't
> debug this one:
> 
> """
> #150/p calls: trigger reg2btf_ids[reg→type] for reg→type > __BPF_REG_TYPE_MAX FAIL
> FAIL
> """
> 
> * On 5.11 onwards, building and running bpf tests is disabled by
> default by commit 7a6eb7c34a78498742b5f82543b7a68c1c443329, so I wonder
> if we want to backport this to 5.10 as well?
> 
> Thanks!
> 
> - Luiz
> 

Re: [5.10, 5.15] New bpf kselftest failure

[Luiz Capitulino]

On 2023-07-17 10:55, Eduard Zingerman wrote:

> 
> 
> 
> On Mon, 2023-07-17 at 09:04 -0400, Luiz Capitulino wrote:
>> Hi,
>>
>> The upstream commit below is backported to 5.10.186, 5.15.120 and 6.1.36:
>>
>> """
>> commit ecdf985d7615356b78241fdb159c091830ed0380
>> Author: Eduard Zingerman <eddyz87@gmail.com>
>> Date:   Wed Feb 15 01:20:27 2023 +0200
>>
>>       bpf: track immediate values written to stack by BPF_ST instruction
>> """
>>
>> This commit is causing the following bpf:test_verifier kselftest to fail:
>>
>> """
>> # #760/p precise: ST insn causing spi > allocated_stack FAIL
>> """
>>
> 
> I can reproduce the error on 6.1.36 but don't understand what's causing it yet.
> The log is suspiciously different from master, will comment later today.

Thank you very much for the prompt reply, Eduard.

I'm available for further testing if needed.

- Luiz

> 
>> Since this test didn't fail before ecdf985d76 backport, the question is
>> if this is a test bug or if this commit introduced a regression.
>>
>> I haven't checked if this failure is present in latest Linus tree because
>> I was unable to build & run the bpf kselftests in an older distro.
>>
>> Also, there some important details about running the bpf kselftests
>> in 5.10 and 5.15:
>>
>> * On 5.10, bpf kselftest build is broken. The following upstream
>> commit needs to be cherry-picked for it to build & run:
>>
>> """
>> commit 4237e9f4a96228ccc8a7abe5e4b30834323cd353
>> Author: Gilad Reti <gilad.reti@gmail.com>
>> Date:   Wed Jan 13 07:38:08 2021 +0200
>>
>>       selftests/bpf: Add verifier test for PTR_TO_MEM spill
>> """
>>
>> * On 5.15.120 there's one additional test that's failing, but I didn't
>> debug this one:
>>
>> """
>> #150/p calls: trigger reg2btf_ids[reg→type] for reg→type > __BPF_REG_TYPE_MAX FAIL
>> FAIL
>> """
>>
>> * On 5.11 onwards, building and running bpf tests is disabled by
>> default by commit 7a6eb7c34a78498742b5f82543b7a68c1c443329, so I wonder
>> if we want to backport this to 5.10 as well?
>>
>> Thanks!
>>
>> - Luiz
>>
> 

Re: [5.10, 5.15] New bpf kselftest failure

[Eduard Zingerman]

On Mon, 2023-07-17 at 10:59 -0400, Luiz Capitulino wrote:
> 
> On 2023-07-17 10:55, Eduard Zingerman wrote:
> 
> > 
> > 
> > 
> > On Mon, 2023-07-17 at 09:04 -0400, Luiz Capitulino wrote:
> > > Hi,
> > > 
> > > The upstream commit below is backported to 5.10.186, 5.15.120 and 6.1.36:
> > > 
> > > """
> > > commit ecdf985d7615356b78241fdb159c091830ed0380
> > > Author: Eduard Zingerman <eddyz87@gmail.com>
> > > Date:   Wed Feb 15 01:20:27 2023 +0200
> > > 
> > >       bpf: track immediate values written to stack by BPF_ST instruction
> > > """
> > > 
> > > This commit is causing the following bpf:test_verifier kselftest to fail:
> > > 
> > > """
> > > # #760/p precise: ST insn causing spi > allocated_stack FAIL
> > > """
> > > 
> > 
> > I can reproduce the error on 6.1.36 but don't understand what's causing it yet.
> > The log is suspiciously different from master, will comment later today.
> 
> Thank you very much for the prompt reply, Eduard.
> 
> I'm available for further testing if needed.

A few observations:
- this is not a bug, precision tracking works a bit differently;
- the patches [1] and [2] (or just [1] but with a minor conflict)
  are sufficient to make behavior similar to master.

I will provided a more detailed explanation in 1-2hr, sorry.

[1] f63181b6ae79 ("bpf: stop setting precise in current state")
[2] 7a830b53c17b ("bpf: aggressively forget precise markings during state checkpointing")

> 
> - Luiz
> 
> > 
> > > Since this test didn't fail before ecdf985d76 backport, the question is
> > > if this is a test bug or if this commit introduced a regression.
> > > 
> > > I haven't checked if this failure is present in latest Linus tree because
> > > I was unable to build & run the bpf kselftests in an older distro.
> > > 
> > > Also, there some important details about running the bpf kselftests
> > > in 5.10 and 5.15:
> > > 
> > > * On 5.10, bpf kselftest build is broken. The following upstream
> > > commit needs to be cherry-picked for it to build & run:
> > > 
> > > """
> > > commit 4237e9f4a96228ccc8a7abe5e4b30834323cd353
> > > Author: Gilad Reti <gilad.reti@gmail.com>
> > > Date:   Wed Jan 13 07:38:08 2021 +0200
> > > 
> > >       selftests/bpf: Add verifier test for PTR_TO_MEM spill
> > > """
> > > 
> > > * On 5.15.120 there's one additional test that's failing, but I didn't
> > > debug this one:
> > > 
> > > """
> > > #150/p calls: trigger reg2btf_ids[reg→type] for reg→type > __BPF_REG_TYPE_MAX FAIL
> > > FAIL
> > > """
> > > 
> > > * On 5.11 onwards, building and running bpf tests is disabled by
> > > default by commit 7a6eb7c34a78498742b5f82543b7a68c1c443329, so I wonder
> > > if we want to backport this to 5.10 as well?
> > > 
> > > Thanks!
> > > 
> > > - Luiz
> > > 
> > 

Re: [5.10, 5.15] New bpf kselftest failure

[Eduard Zingerman]

On Mon, 2023-07-17 at 10:59 -0400, Luiz Capitulino wrote:
> 
> On 2023-07-17 10:55, Eduard Zingerman wrote:
> 
> > 
> > 
> > 
> > On Mon, 2023-07-17 at 09:04 -0400, Luiz Capitulino wrote:
> > > Hi,
> > > 
> > > The upstream commit below is backported to 5.10.186, 5.15.120 and 6.1.36:
> > > 
> > > """
> > > commit ecdf985d7615356b78241fdb159c091830ed0380
> > > Author: Eduard Zingerman <eddyz87@gmail.com>
> > > Date:   Wed Feb 15 01:20:27 2023 +0200
> > > 
> > >       bpf: track immediate values written to stack by BPF_ST instruction
> > > """
> > > 
> > > This commit is causing the following bpf:test_verifier kselftest to fail:
> > > 
> > > """
> > > # #760/p precise: ST insn causing spi > allocated_stack FAIL
> > > """
> > > 
> > 
> > I can reproduce the error on 6.1.36 but don't understand what's causing it yet.
> > The log is suspiciously different from master, will comment later today.
> 
> Thank you very much for the prompt reply, Eduard.
> 
> I'm available for further testing if needed.

The test case in question looks as follows:

                           ; old log                 new log
0: R1=ctx(off=0,imm=0)
0: r3 = r10                ; R3=fp0 R10=fp0          R3=fp0 R10=fp0
1: if r3 != 0x7b goto pc+0 ; R3=fp0                  R3=fp0
2: *(u64 *)(r3 -8) = 0     ; R3=fp0 fp-8_w=mmmmmmmm  R3=fp0 fp-8_w=00000000
3: r4 = *(u64 *)(r10 -8)   ; R4_w=scalar() R10=fp0   R4_w=P0 R10=fp0 fp-8_w=00000000
4: r0 = -1                 ; R0=-1                   R0=-1
5: if r4 > r0 goto pc+0
6: exit

The test checks the log generated by __mark_chain_precision() at
instruction #5: registers `r4` and `r0` should be marked precise
(in that order).

The `r4` and `r0` are marked precise by check_cond_jmp_op() function
because jump #5 is predicted (because comparison is unsigned `r4` is
never greater than -1/`r0`).

Failure happens because only log for `r0` markings is present.

Patch [5] changes processing of instruction #2 ( *(u64 *)(r3 -8) = 0; ):
- before patch fp[-8] is marked as mmmmmmmm (any 64-bit scalar value);
- after patch fp[-8] is marked as 00000000.

Which in turn changes processing of instruction #3 ( r4 = *(u64 *)(r10 -8); ):
- before patch r4 is marked as unbound scalar
- after patch r4 is marked as precise zero
  (check_stack_read_fixed_off() has a special case for zero loads,
   DST registers of such loads are always marked precise).
   
If __mark_chain_precision() is called for a register that is already
marked as precise no log is generated. Thus the log looks different
when check_cond_jmp_op() calls __mark_chain_precision() for `r4` and
then for `r0`:
- before patch `r4` is not precise and log for it is generated;
- after patch `r4` is precise and log for it is not generated.

Now a question:
  why does current master has log for both `r4` and `r0`,
  given that commit [5] is present in master?

This happens because of commit [2] (not back-ported to 6.1.36).

Commit [2] adds function mark_all_scalars_imprecise() that removes
precision marks from scalar registers when new state is created in a
state chain (and for this test new states are created before #1 and #5
because of the BPF_F_TEST_STATE_FREQ flag).

Effectively, before #5 is processed by check_cond_jmp_op()
mark_all_scalars_imprecise() undoes `r4`'s precision mark assigned by
check_stack_read_fixed_off() => log for both `r4` and `r0` is present.

I need some time to convince myself that such interaction between
check_stack_read_fixed_off() and mark_all_scalars_imprecise()
is safe, but that's what we have in master.

Commit [2] is a part of a series [4] by Andrii Nakryiko.
This series had been partially back-ported already.
Commits [0,1,2,3] are not yet back-ported and have to be picked
altogether, otherwise there are numerous test failures.

Still, when I cherry-pick [0,1,2,3] `./test_progs -a setget_sockopt` is failing.
I'll investigate this failure but don't think I'll finish today.

---

Alternatively, if the goal is to minimize amount of changes, we can
disable or modify the 'precise: ST insn causing spi > allocated_stack'.

---

Commits (in chronological order):
[0] be2ef8161572 ("bpf: allow precision tracking for programs with subprogs")
[1] f63181b6ae79 ("bpf: stop setting precise in current state")
[2] 7a830b53c17b ("bpf: aggressively forget precise markings during state checkpointing")
[3] 4f999b767769 ("selftests/bpf: make test_align selftest more robust")
[4] 07d90c72efbe ("Merge branch 'BPF verifier precision tracking improvements'")
[5] ecdf985d7615 ("bpf: track immediate values written to stack by BPF_ST instruction")


> 
> - Luiz
> 
> > 
> > > Since this test didn't fail before ecdf985d76 backport, the question is
> > > if this is a test bug or if this commit introduced a regression.
> > > 
> > > I haven't checked if this failure is present in latest Linus tree because
> > > I was unable to build & run the bpf kselftests in an older distro.
> > > 
> > > Also, there some important details about running the bpf kselftests
> > > in 5.10 and 5.15:
> > > 
> > > * On 5.10, bpf kselftest build is broken. The following upstream
> > > commit needs to be cherry-picked for it to build & run:
> > > 
> > > """
> > > commit 4237e9f4a96228ccc8a7abe5e4b30834323cd353
> > > Author: Gilad Reti <gilad.reti@gmail.com>
> > > Date:   Wed Jan 13 07:38:08 2021 +0200
> > > 
> > >       selftests/bpf: Add verifier test for PTR_TO_MEM spill
> > > """
> > > 
> > > * On 5.15.120 there's one additional test that's failing, but I didn't
> > > debug this one:
> > > 
> > > """
> > > #150/p calls: trigger reg2btf_ids[reg→type] for reg→type > __BPF_REG_TYPE_MAX FAIL
> > > FAIL
> > > """
> > > 
> > > * On 5.11 onwards, building and running bpf tests is disabled by
> > > default by commit 7a6eb7c34a78498742b5f82543b7a68c1c443329, so I wonder
> > > if we want to backport this to 5.10 as well?
> > > 
> > > Thanks!
> > > 
> > > - Luiz
> > > 
> > 

Re: [5.10, 5.15] New bpf kselftest failure

[Eduard Zingerman]

On Tue, 2023-07-18 at 01:57 +0300, Eduard Zingerman wrote:
> [...]
> Still, when I cherry-pick [0,1,2,3] `./test_progs -a setget_sockopt` is failing.
> I'll investigate this failure but don't think I'll finish today.
> 
> ---
> 
> Alternatively, if the goal is to minimize amount of changes, we can
> disable or modify the 'precise: ST insn causing spi > allocated_stack'.
> 
> ---
> 
> Commits (in chronological order):
> [0] be2ef8161572 ("bpf: allow precision tracking for programs with subprogs")
> [1] f63181b6ae79 ("bpf: stop setting precise in current state")
> [2] 7a830b53c17b ("bpf: aggressively forget precise markings during state checkpointing")
> [3] 4f999b767769 ("selftests/bpf: make test_align selftest more robust")
> [4] 07d90c72efbe ("Merge branch 'BPF verifier precision tracking improvements'")
> [5] ecdf985d7615 ("bpf: track immediate values written to stack by BPF_ST instruction")

I made a mistake, while resolving merge conflict for [0] yesterday.
After correction the `./test_progs -a setget_sockopt` passes.
I also noted that the following tests fail on v6.1.36:

  ./test_progs -a sk_assign,fexit_bpf2bpf

These tests are fixed by back-porting the following upstream commits:
- 7ce878ca81bc ("selftests/bpf: Fix sk_assign on s390x")
- 63d78b7e8ca2 ("selftests/bpf: Workaround verification failure for fexit_bpf2bpf/func_replace_return_code")

I pushed modified version of v6.1.36 to my github account, it has
test_verifier, test_progs, test_progs-no_alu32 and test_maps passing
(on my x86 setup):

  https://github.com/eddyz87/bpf/commits/v6.1.36-with-fixes

Do you need any additional actions from my side?

Re: [5.10, 5.15] New bpf kselftest failure

[Greg KH]

On Tue, Jul 18, 2023 at 03:31:25PM +0300, Eduard Zingerman wrote:
> On Tue, 2023-07-18 at 01:57 +0300, Eduard Zingerman wrote:
> > [...]
> > Still, when I cherry-pick [0,1,2,3] `./test_progs -a setget_sockopt` is failing.
> > I'll investigate this failure but don't think I'll finish today.
> > 
> > ---
> > 
> > Alternatively, if the goal is to minimize amount of changes, we can
> > disable or modify the 'precise: ST insn causing spi > allocated_stack'.
> > 
> > ---
> > 
> > Commits (in chronological order):
> > [0] be2ef8161572 ("bpf: allow precision tracking for programs with subprogs")
> > [1] f63181b6ae79 ("bpf: stop setting precise in current state")
> > [2] 7a830b53c17b ("bpf: aggressively forget precise markings during state checkpointing")
> > [3] 4f999b767769 ("selftests/bpf: make test_align selftest more robust")
> > [4] 07d90c72efbe ("Merge branch 'BPF verifier precision tracking improvements'")
> > [5] ecdf985d7615 ("bpf: track immediate values written to stack by BPF_ST instruction")
> 
> I made a mistake, while resolving merge conflict for [0] yesterday.
> After correction the `./test_progs -a setget_sockopt` passes.
> I also noted that the following tests fail on v6.1.36:
> 
>   ./test_progs -a sk_assign,fexit_bpf2bpf
> 
> These tests are fixed by back-porting the following upstream commits:
> - 7ce878ca81bc ("selftests/bpf: Fix sk_assign on s390x")
> - 63d78b7e8ca2 ("selftests/bpf: Workaround verification failure for fexit_bpf2bpf/func_replace_return_code")
> 
> I pushed modified version of v6.1.36 to my github account, it has
> test_verifier, test_progs, test_progs-no_alu32 and test_maps passing
> (on my x86 setup):
> 
>   https://github.com/eddyz87/bpf/commits/v6.1.36-with-fixes
> 
> Do you need any additional actions from my side?

I don't understand, what can I do with a github link?  Can you send us
the patches backported so we can apply them to the stable tree?

thanks,

greg k-h

Re: [5.10, 5.15] New bpf kselftest failure

[Eduard Zingerman]

On Tue, 2023-07-18 at 15:23 +0200, Greg KH wrote:
> On Tue, Jul 18, 2023 at 03:31:25PM +0300, Eduard Zingerman wrote:
> > On Tue, 2023-07-18 at 01:57 +0300, Eduard Zingerman wrote:
> > > [...]
> > > Still, when I cherry-pick [0,1,2,3] `./test_progs -a setget_sockopt` is failing.
> > > I'll investigate this failure but don't think I'll finish today.
> > > 
> > > ---
> > > 
> > > Alternatively, if the goal is to minimize amount of changes, we can
> > > disable or modify the 'precise: ST insn causing spi > allocated_stack'.
> > > 
> > > ---
> > > 
> > > Commits (in chronological order):
> > > [0] be2ef8161572 ("bpf: allow precision tracking for programs with subprogs")
> > > [1] f63181b6ae79 ("bpf: stop setting precise in current state")
> > > [2] 7a830b53c17b ("bpf: aggressively forget precise markings during state checkpointing")
> > > [3] 4f999b767769 ("selftests/bpf: make test_align selftest more robust")
> > > [4] 07d90c72efbe ("Merge branch 'BPF verifier precision tracking improvements'")
> > > [5] ecdf985d7615 ("bpf: track immediate values written to stack by BPF_ST instruction")
> > 
> > I made a mistake, while resolving merge conflict for [0] yesterday.
> > After correction the `./test_progs -a setget_sockopt` passes.
> > I also noted that the following tests fail on v6.1.36:
> > 
> >   ./test_progs -a sk_assign,fexit_bpf2bpf
> > 
> > These tests are fixed by back-porting the following upstream commits:
> > - 7ce878ca81bc ("selftests/bpf: Fix sk_assign on s390x")
> > - 63d78b7e8ca2 ("selftests/bpf: Workaround verification failure for fexit_bpf2bpf/func_replace_return_code")
> > 
> > I pushed modified version of v6.1.36 to my github account, it has
> > test_verifier, test_progs, test_progs-no_alu32 and test_maps passing
> > (on my x86 setup):
> > 
> >   https://github.com/eddyz87/bpf/commits/v6.1.36-with-fixes
> > 
> > Do you need any additional actions from my side?
> 
> I don't understand, what can I do with a github link?  Can you send us
> the patches backported so we can apply them to the stable tree?

Sorry, I'm not familiar with procedure for stable tree patches or
who decides what's being picked.
Looks like this situation is "Option 3" from [1], rigth?
After reading that page I'm not sure:
- can I bundle all the necessary commits as a patch-set?
- a few commits need merging, others could be cherry-picked,
  is it possible to submit all of them with [ Upstream commit ... ] marks?

Also, as I wrote above, there are two possible solutions:
- backport above mentioned patches
- adjust the test log

[1] https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html

> 
> thanks,
> 
> greg k-h

Re: [5.10, 5.15] New bpf kselftest failure

[Luiz Capitulino]

On 2023-07-18 08:31, Eduard Zingerman wrote:

> 
> 
> 
> On Tue, 2023-07-18 at 01:57 +0300, Eduard Zingerman wrote:
>> [...]
>> Still, when I cherry-pick [0,1,2,3] `./test_progs -a setget_sockopt` is failing.
>> I'll investigate this failure but don't think I'll finish today.
>>
>> ---
>>
>> Alternatively, if the goal is to minimize amount of changes, we can
>> disable or modify the 'precise: ST insn causing spi > allocated_stack'.
>>
>> ---
>>
>> Commits (in chronological order):
>> [0] be2ef8161572 ("bpf: allow precision tracking for programs with subprogs")
>> [1] f63181b6ae79 ("bpf: stop setting precise in current state")
>> [2] 7a830b53c17b ("bpf: aggressively forget precise markings during state checkpointing")
>> [3] 4f999b767769 ("selftests/bpf: make test_align selftest more robust")
>> [4] 07d90c72efbe ("Merge branch 'BPF verifier precision tracking improvements'")
>> [5] ecdf985d7615 ("bpf: track immediate values written to stack by BPF_ST instruction")
> 
> I made a mistake, while resolving merge conflict for [0] yesterday.
> After correction the `./test_progs -a setget_sockopt` passes.
> I also noted that the following tests fail on v6.1.36:
> 
>    ./test_progs -a sk_assign,fexit_bpf2bpf
> 
> These tests are fixed by back-porting the following upstream commits:
> - 7ce878ca81bc ("selftests/bpf: Fix sk_assign on s390x")
> - 63d78b7e8ca2 ("selftests/bpf: Workaround verification failure for fexit_bpf2bpf/func_replace_return_code")
> 
> I pushed modified version of v6.1.36 to my github account, it has
> test_verifier, test_progs, test_progs-no_alu32 and test_maps passing
> (on my x86 setup):
> 
>    https://github.com/eddyz87/bpf/commits/v6.1.36-with-fixes
> 
> Do you need any additional actions from my side?

First, thank you very much for your work on this and getting the tests
passing on 6.1.

In terms of action items, have you checked this situation in 5.10 and
5.15? For 5.10, we also need 4237e9f4a96228ccc8a7abe5e4b30834323cd353
otherwise the bpf tests don't even build there.

Also, would you know if something important is broken for users or is
this just a small behavior difference between kernels?

- Luiz

Re: [5.10, 5.15] New bpf kselftest failure

[Eduard Zingerman]

On Tue, 2023-07-18 at 10:06 -0400, Luiz Capitulino wrote:
> 
> On 2023-07-18 08:31, Eduard Zingerman wrote:
> 
> > 
> > 
> > 
> > On Tue, 2023-07-18 at 01:57 +0300, Eduard Zingerman wrote:
> > > [...]
> > > Still, when I cherry-pick [0,1,2,3] `./test_progs -a setget_sockopt` is failing.
> > > I'll investigate this failure but don't think I'll finish today.
> > > 
> > > ---
> > > 
> > > Alternatively, if the goal is to minimize amount of changes, we can
> > > disable or modify the 'precise: ST insn causing spi > allocated_stack'.
> > > 
> > > ---
> > > 
> > > Commits (in chronological order):
> > > [0] be2ef8161572 ("bpf: allow precision tracking for programs with subprogs")
> > > [1] f63181b6ae79 ("bpf: stop setting precise in current state")
> > > [2] 7a830b53c17b ("bpf: aggressively forget precise markings during state checkpointing")
> > > [3] 4f999b767769 ("selftests/bpf: make test_align selftest more robust")
> > > [4] 07d90c72efbe ("Merge branch 'BPF verifier precision tracking improvements'")
> > > [5] ecdf985d7615 ("bpf: track immediate values written to stack by BPF_ST instruction")
> > 
> > I made a mistake, while resolving merge conflict for [0] yesterday.
> > After correction the `./test_progs -a setget_sockopt` passes.
> > I also noted that the following tests fail on v6.1.36:
> > 
> >    ./test_progs -a sk_assign,fexit_bpf2bpf
> > 
> > These tests are fixed by back-porting the following upstream commits:
> > - 7ce878ca81bc ("selftests/bpf: Fix sk_assign on s390x")
> > - 63d78b7e8ca2 ("selftests/bpf: Workaround verification failure for fexit_bpf2bpf/func_replace_return_code")
> > 
> > I pushed modified version of v6.1.36 to my github account, it has
> > test_verifier, test_progs, test_progs-no_alu32 and test_maps passing
> > (on my x86 setup):
> > 
> >    https://github.com/eddyz87/bpf/commits/v6.1.36-with-fixes
> > 
> > Do you need any additional actions from my side?
> 
> First, thank you very much for your work on this and getting the tests
> passing on 6.1.

Thank you.

> In terms of action items, have you checked this situation in 5.10 and
> 5.15? For 5.10, we also need 4237e9f4a96228ccc8a7abe5e4b30834323cd353
> otherwise the bpf tests don't even build there.

Haven't checked 5.15/5.10, will take a look.
Are there any time-frame limitations?
(I'd like to work on this on Wednesday or Thursday)

> 
> Also, would you know if something important is broken for users or is
> this just a small behavior difference between kernels?

I think it's more like small behavior difference:
- be2ef8161572, f63181b6ae79, 7a830b53c17b are verification
  scalability optimizations, with these patches it is possible
  to load a bit more complex programs (larger programs, or more
  complex branching patterns).
- 4f999b767769, 7ce878ca81bc, 63d78b7e8ca2 - fixes for selftests,
  no new functionality.

Thanks,
Eduard

Re: [5.10, 5.15] New bpf kselftest failure

[Luiz Capitulino]

On 2023-07-18 10:35, Eduard Zingerman wrote:

> 
> 
> 
> On Tue, 2023-07-18 at 10:06 -0400, Luiz Capitulino wrote:
>>
>> On 2023-07-18 08:31, Eduard Zingerman wrote:
>>
>>>
>>>
>>>
>>> On Tue, 2023-07-18 at 01:57 +0300, Eduard Zingerman wrote:
>>>> [...]
>>>> Still, when I cherry-pick [0,1,2,3] `./test_progs -a setget_sockopt` is failing.
>>>> I'll investigate this failure but don't think I'll finish today.
>>>>
>>>> ---
>>>>
>>>> Alternatively, if the goal is to minimize amount of changes, we can
>>>> disable or modify the 'precise: ST insn causing spi > allocated_stack'.
>>>>
>>>> ---
>>>>
>>>> Commits (in chronological order):
>>>> [0] be2ef8161572 ("bpf: allow precision tracking for programs with subprogs")
>>>> [1] f63181b6ae79 ("bpf: stop setting precise in current state")
>>>> [2] 7a830b53c17b ("bpf: aggressively forget precise markings during state checkpointing")
>>>> [3] 4f999b767769 ("selftests/bpf: make test_align selftest more robust")
>>>> [4] 07d90c72efbe ("Merge branch 'BPF verifier precision tracking improvements'")
>>>> [5] ecdf985d7615 ("bpf: track immediate values written to stack by BPF_ST instruction")
>>>
>>> I made a mistake, while resolving merge conflict for [0] yesterday.
>>> After correction the `./test_progs -a setget_sockopt` passes.
>>> I also noted that the following tests fail on v6.1.36:
>>>
>>>     ./test_progs -a sk_assign,fexit_bpf2bpf
>>>
>>> These tests are fixed by back-porting the following upstream commits:
>>> - 7ce878ca81bc ("selftests/bpf: Fix sk_assign on s390x")
>>> - 63d78b7e8ca2 ("selftests/bpf: Workaround verification failure for fexit_bpf2bpf/func_replace_return_code")
>>>
>>> I pushed modified version of v6.1.36 to my github account, it has
>>> test_verifier, test_progs, test_progs-no_alu32 and test_maps passing
>>> (on my x86 setup):
>>>
>>>     https://github.com/eddyz87/bpf/commits/v6.1.36-with-fixes
>>>
>>> Do you need any additional actions from my side?
>>
>> First, thank you very much for your work on this and getting the tests
>> passing on 6.1.
> 
> Thank you.
> 
>> In terms of action items, have you checked this situation in 5.10 and
>> 5.15? For 5.10, we also need 4237e9f4a96228ccc8a7abe5e4b30834323cd353
>> otherwise the bpf tests don't even build there.
> 
> Haven't checked 5.15/5.10, will take a look.
> Are there any time-frame limitations?
> (I'd like to work on this on Wednesday or Thursday)

Since, as you explain below, this is not a impactful regression to stable
users, I think that's fine.

>> Also, would you know if something important is broken for users or is
>> this just a small behavior difference between kernels?
> 
> I think it's more like small behavior difference:
> - be2ef8161572, f63181b6ae79, 7a830b53c17b are verification
>    scalability optimizations, with these patches it is possible
>    to load a bit more complex programs (larger programs, or more
>    complex branching patterns).
> - 4f999b767769, 7ce878ca81bc, 63d78b7e8ca2 - fixes for selftests,
>    no new functionality.

Thanks for clarifying it!

- Luiz

> 
> Thanks,
> Eduard

Re: [5.10, 5.15] New bpf kselftest failure

[Luiz Capitulino]

On 2023-07-18 09:52, Eduard Zingerman wrote:

> 
> 
> 
> On Tue, 2023-07-18 at 15:23 +0200, Greg KH wrote:
>> On Tue, Jul 18, 2023 at 03:31:25PM +0300, Eduard Zingerman wrote:
>>> On Tue, 2023-07-18 at 01:57 +0300, Eduard Zingerman wrote:
>>>> [...]
>>>> Still, when I cherry-pick [0,1,2,3] `./test_progs -a setget_sockopt` is failing.
>>>> I'll investigate this failure but don't think I'll finish today.
>>>>
>>>> ---
>>>>
>>>> Alternatively, if the goal is to minimize amount of changes, we can
>>>> disable or modify the 'precise: ST insn causing spi > allocated_stack'.
>>>>
>>>> ---
>>>>
>>>> Commits (in chronological order):
>>>> [0] be2ef8161572 ("bpf: allow precision tracking for programs with subprogs")
>>>> [1] f63181b6ae79 ("bpf: stop setting precise in current state")
>>>> [2] 7a830b53c17b ("bpf: aggressively forget precise markings during state checkpointing")
>>>> [3] 4f999b767769 ("selftests/bpf: make test_align selftest more robust")
>>>> [4] 07d90c72efbe ("Merge branch 'BPF verifier precision tracking improvements'")
>>>> [5] ecdf985d7615 ("bpf: track immediate values written to stack by BPF_ST instruction")
>>>
>>> I made a mistake, while resolving merge conflict for [0] yesterday.
>>> After correction the `./test_progs -a setget_sockopt` passes.
>>> I also noted that the following tests fail on v6.1.36:
>>>
>>>    ./test_progs -a sk_assign,fexit_bpf2bpf
>>>
>>> These tests are fixed by back-porting the following upstream commits:
>>> - 7ce878ca81bc ("selftests/bpf: Fix sk_assign on s390x")
>>> - 63d78b7e8ca2 ("selftests/bpf: Workaround verification failure for fexit_bpf2bpf/func_replace_return_code")
>>>
>>> I pushed modified version of v6.1.36 to my github account, it has
>>> test_verifier, test_progs, test_progs-no_alu32 and test_maps passing
>>> (on my x86 setup):
>>>
>>>    https://github.com/eddyz87/bpf/commits/v6.1.36-with-fixes
>>>
>>> Do you need any additional actions from my side?
>>
>> I don't understand, what can I do with a github link?  Can you send us
>> the patches backported so we can apply them to the stable tree?
> 
> Sorry, I'm not familiar with procedure for stable tree patches or
> who decides what's being picked.

I'm by no means an authority here, but I'll try to help with what I would
do myself.

> Looks like this situation is "Option 3" from [1], rigth?

Right.

> After reading that page I'm not sure:
> - can I bundle all the necessary commits as a patch-set?

Yes.

> - a few commits need merging, others could be cherry-picked,
>    is it possible to submit all of them with [ Upstream commit ... ] marks?

Yes.

> Also, as I wrote above, there are two possible solutions:
> - backport above mentioned patches
> - adjust the test log

I think we want to avoid deviating from upstream (Linus tree), but I'm not
sure if there are valid exceptions.

- Luiz

> 
> [1] https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> 
>>
>> thanks,
>>
>> greg k-h
> 

Re: [5.10, 5.15] New bpf kselftest failure

[Greg KH]

On Tue, Jul 18, 2023 at 10:58:45AM -0400, Luiz Capitulino wrote:
> 
> 
> On 2023-07-18 09:52, Eduard Zingerman wrote:
> 
> > 
> > 
> > 
> > On Tue, 2023-07-18 at 15:23 +0200, Greg KH wrote:
> > > On Tue, Jul 18, 2023 at 03:31:25PM +0300, Eduard Zingerman wrote:
> > > > On Tue, 2023-07-18 at 01:57 +0300, Eduard Zingerman wrote:
> > > > > [...]
> > > > > Still, when I cherry-pick [0,1,2,3] `./test_progs -a setget_sockopt` is failing.
> > > > > I'll investigate this failure but don't think I'll finish today.
> > > > > 
> > > > > ---
> > > > > 
> > > > > Alternatively, if the goal is to minimize amount of changes, we can
> > > > > disable or modify the 'precise: ST insn causing spi > allocated_stack'.
> > > > > 
> > > > > ---
> > > > > 
> > > > > Commits (in chronological order):
> > > > > [0] be2ef8161572 ("bpf: allow precision tracking for programs with subprogs")
> > > > > [1] f63181b6ae79 ("bpf: stop setting precise in current state")
> > > > > [2] 7a830b53c17b ("bpf: aggressively forget precise markings during state checkpointing")
> > > > > [3] 4f999b767769 ("selftests/bpf: make test_align selftest more robust")
> > > > > [4] 07d90c72efbe ("Merge branch 'BPF verifier precision tracking improvements'")
> > > > > [5] ecdf985d7615 ("bpf: track immediate values written to stack by BPF_ST instruction")
> > > > 
> > > > I made a mistake, while resolving merge conflict for [0] yesterday.
> > > > After correction the `./test_progs -a setget_sockopt` passes.
> > > > I also noted that the following tests fail on v6.1.36:
> > > > 
> > > >    ./test_progs -a sk_assign,fexit_bpf2bpf
> > > > 
> > > > These tests are fixed by back-porting the following upstream commits:
> > > > - 7ce878ca81bc ("selftests/bpf: Fix sk_assign on s390x")
> > > > - 63d78b7e8ca2 ("selftests/bpf: Workaround verification failure for fexit_bpf2bpf/func_replace_return_code")
> > > > 
> > > > I pushed modified version of v6.1.36 to my github account, it has
> > > > test_verifier, test_progs, test_progs-no_alu32 and test_maps passing
> > > > (on my x86 setup):
> > > > 
> > > >    https://github.com/eddyz87/bpf/commits/v6.1.36-with-fixes
> > > > 
> > > > Do you need any additional actions from my side?
> > > 
> > > I don't understand, what can I do with a github link?  Can you send us
> > > the patches backported so we can apply them to the stable tree?
> > 
> > Sorry, I'm not familiar with procedure for stable tree patches or
> > who decides what's being picked.
> 
> I'm by no means an authority here, but I'll try to help with what I would
> do myself.
> 
> > Looks like this situation is "Option 3" from [1], rigth?
> 
> Right.
> 
> > After reading that page I'm not sure:
> > - can I bundle all the necessary commits as a patch-set?
> 
> Yes.
> 
> > - a few commits need merging, others could be cherry-picked,
> >    is it possible to submit all of them with [ Upstream commit ... ] marks?
> 
> Yes.
> 
> > Also, as I wrote above, there are two possible solutions:
> > - backport above mentioned patches
> > - adjust the test log
> 
> I think we want to avoid deviating from upstream (Linus tree), but I'm not
> sure if there are valid exceptions.

backporting the mentioned patches is best, can someone send them to us
in email so that we can apply them?

thanks,

greg k-h

Re: [5.10, 5.15] New bpf kselftest failure

[Eduard Zingerman]

On Fri, 2023-07-21 at 07:30 +0200, Greg KH wrote:
[...]
> 
> backporting the mentioned patches is best, can someone send them to us
> in email so that we can apply them?

I'm investigating 5.15 test failures at the moment.
Will send patches for linux-6.1.y by the end of the day.

Thanks,
Eduard