From: Lorenz Bauer lorenz.bauer@isovalent.com
[ Upstream commit dfdd608c3b365f0fd49d7e13911ebcde06b9865b ]
Add a regression test that ensures that a VAR pointing at a modifier which follows a PTR (or STRUCT or ARRAY) is resolved correctly by the datasec validator.
Signed-off-by: Lorenz Bauer lmb@isovalent.com Link: https://lore.kernel.org/r/20230306112138.155352-3-lmb@isovalent.com Signed-off-by: Martin KaFai Lau martin.lau@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- tools/testing/selftests/bpf/prog_tests/btf.c | 28 ++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c index de1b5b9eb93a8..d8d1292e73b53 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf.c +++ b/tools/testing/selftests/bpf/prog_tests/btf.c @@ -879,6 +879,34 @@ static struct btf_raw_test raw_tests[] = { .btf_load_err = true, .err_str = "Invalid elem", }, +{ + .descr = "var after datasec, ptr followed by modifier", + .raw_types = { + /* .bss section */ /* [1] */ + BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_DATASEC, 0, 2), + sizeof(void*)+4), + BTF_VAR_SECINFO_ENC(4, 0, sizeof(void*)), + BTF_VAR_SECINFO_ENC(6, sizeof(void*), 4), + /* int */ /* [2] */ + BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), + /* int* */ /* [3] */ + BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 2), + BTF_VAR_ENC(NAME_TBD, 3, 0), /* [4] */ + /* const int */ /* [5] */ + BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 2), + BTF_VAR_ENC(NAME_TBD, 5, 0), /* [6] */ + BTF_END_RAW, + }, + .str_sec = "\0a\0b\0c\0", + .str_sec_size = sizeof("\0a\0b\0c\0"), + .map_type = BPF_MAP_TYPE_ARRAY, + .map_name = ".bss", + .key_size = sizeof(int), + .value_size = sizeof(void*)+4, + .key_type_id = 0, + .value_type_id = 1, + .max_entries = 1, +}, /* Test member exceeds the size of struct. * * struct A {
On Mon, Mar 20, 2023 at 12:53 AM Sasha Levin sashal@kernel.org wrote:
From: Lorenz Bauer lorenz.bauer@isovalent.com
[ Upstream commit dfdd608c3b365f0fd49d7e13911ebcde06b9865b ]
Add a regression test that ensures that a VAR pointing at a modifier which follows a PTR (or STRUCT or ARRAY) is resolved correctly by the datasec validator.
Signed-off-by: Lorenz Bauer lmb@isovalent.com Link: https://lore.kernel.org/r/20230306112138.155352-3-lmb@isovalent.com Signed-off-by: Martin KaFai Lau martin.lau@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org
Hi Sasha,
Can you explain why this patch was selected? I'd prefer to not backport the test, since it frequently leads to breakage when trying to build selftests/bpf on stable kernels.
Thanks Lorenz
On Mon, Mar 20, 2023 at 03:31:42PM +0000, Lorenz Bauer wrote:
On Mon, Mar 20, 2023 at 12:53 AM Sasha Levin sashal@kernel.org wrote:
From: Lorenz Bauer lorenz.bauer@isovalent.com
[ Upstream commit dfdd608c3b365f0fd49d7e13911ebcde06b9865b ]
Add a regression test that ensures that a VAR pointing at a modifier which follows a PTR (or STRUCT or ARRAY) is resolved correctly by the datasec validator.
Signed-off-by: Lorenz Bauer lmb@isovalent.com Link: https://lore.kernel.org/r/20230306112138.155352-3-lmb@isovalent.com Signed-off-by: Martin KaFai Lau martin.lau@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org
Hi Sasha,
Can you explain why this patch was selected? I'd prefer to not backport the test, since it frequently leads to breakage when trying to build selftests/bpf on stable kernels.
Why would it break? Is that because the test is buggy, or the kernel is buggy?
thanks,
greg k-h
On Mon, Mar 20, 2023 at 3:48 PM Greg KH gregkh@linuxfoundation.org wrote:
Why would it break? Is that because the test is buggy, or the kernel is buggy?
This test will be fine, but there have been several times when selftests/bpf for stable kernel releases didn't actually compile due to backported tests. This is because macros we're redefined, etc. Unless those also get picked (seems like a sisyphean task) we'll keep seeing broken selftests/bpf on stable.
Best Lorenz
On Tue, Mar 28, 2023 at 11:18 AM Lorenz Bauer lmb@isovalent.com wrote:
On Mon, Mar 20, 2023 at 3:48 PM Greg KH gregkh@linuxfoundation.org wrote:
Why would it break? Is that because the test is buggy, or the kernel is buggy?
This test will be fine, but there have been several times when selftests/bpf for stable kernel releases didn't actually compile due to backported tests. This is because macros we're redefined, etc. Unless those also get picked (seems like a sisyphean task) we'll keep seeing broken selftests/bpf on stable.
Hi Greg, Sasha,
Following up on this since it seems to have fallen through the cracks.
Lorenz
On Tue, Apr 11, 2023 at 04:08:32PM +0100, Lorenz Bauer wrote:
On Tue, Mar 28, 2023 at 11:18 AM Lorenz Bauer lmb@isovalent.com wrote:
On Mon, Mar 20, 2023 at 3:48 PM Greg KH gregkh@linuxfoundation.org wrote:
Why would it break? Is that because the test is buggy, or the kernel is buggy?
This test will be fine, but there have been several times when selftests/bpf for stable kernel releases didn't actually compile due to backported tests. This is because macros we're redefined, etc. Unless those also get picked (seems like a sisyphean task) we'll keep seeing broken selftests/bpf on stable.
Hi Greg, Sasha,
Following up on this since it seems to have fallen through the cracks.
I didn't see anything to do here.
And selftests should NOT be broken on stable releases, if so, something is wrong as no other subsystem has that happen.
confused,
greg k-h
On Tue, Apr 11, 2023 at 4:14 PM Greg KH gregkh@linuxfoundation.org wrote:
I didn't see anything to do here.
And selftests should NOT be broken on stable releases, if so, something is wrong as no other subsystem has that happen.
Sorry for the long delay in replying, I update the kernels we use for CI only infrequently. Here is an example of the build failure I'm seeing, from kernel.org 5.10 LTS:
In file included from /work/build/5.10.180/tools/testing/selftests/bpf/verifier/tests.h:59, from test_verifier.c:355: /work/build/5.10.180/tools/testing/selftests/bpf/verifier/ref_tracking.c:935:3: error: 'struct bpf_test' has no member named 'fixup_map_ringbuf'; did you mean 'fixup_map_in_map'? 935 | .fixup_map_ringbuf = { 11 }, | ^~~~~~~~~~~~~~~~~ | fixup_map_in_map
This is just doing make -C tools/testing/selftests/bpf after compiling a kernel.
Best Lorenz
On Wed, May 24, 2023 at 12:03:43PM +0100, Lorenz Bauer wrote:
On Tue, Apr 11, 2023 at 4:14 PM Greg KH gregkh@linuxfoundation.org wrote:
I didn't see anything to do here.
And selftests should NOT be broken on stable releases, if so, something is wrong as no other subsystem has that happen.
Sorry for the long delay in replying, I update the kernels we use for CI only infrequently. Here is an example of the build failure I'm seeing, from kernel.org 5.10 LTS:
In file included from /work/build/5.10.180/tools/testing/selftests/bpf/verifier/tests.h:59, from test_verifier.c:355: /work/build/5.10.180/tools/testing/selftests/bpf/verifier/ref_tracking.c:935:3: error: 'struct bpf_test' has no member named 'fixup_map_ringbuf'; did you mean 'fixup_map_in_map'? 935 | .fixup_map_ringbuf = { 11 }, | ^~~~~~~~~~~~~~~~~ | fixup_map_in_map
This is just doing make -C tools/testing/selftests/bpf after compiling a kernel.
Great, any specific commits that fix this issue would be appreciated to be pointed at so we can apply them.
thanks,
greg k-h
On Wed, May 24, 2023 at 5:04 PM Greg KH gregkh@linuxfoundation.org wrote:
Great, any specific commits that fix this issue would be appreciated to be pointed at so we can apply them.
The problem was introduced by commit f4b8c0710ab6 ("selftests/bpf: Add verifier test for release_reference()") in your tree. Seems like fixup_map_ringbuf was introduced in upstream commit 4237e9f4a962 ("selftests/bpf: Add verifier test for PTR_TO_MEM spill") but that wasn't backported.
To restate my original question: how can we avoid breaking BPF selftests? From personal experience this happens somewhat regularly.
Best Lorenz
On Wed, May 24, 2023 at 06:04:43PM +0100, Lorenz Bauer wrote:
On Wed, May 24, 2023 at 5:04 PM Greg KH gregkh@linuxfoundation.org wrote:
Great, any specific commits that fix this issue would be appreciated to be pointed at so we can apply them.
The problem was introduced by commit f4b8c0710ab6 ("selftests/bpf: Add verifier test for release_reference()") in your tree. Seems like fixup_map_ringbuf was introduced in upstream commit 4237e9f4a962 ("selftests/bpf: Add verifier test for PTR_TO_MEM spill") but that wasn't backported.
So what tree(s) does this need to be backported to? I'm confused, this is a 6.2 email thread which is long end-of-life.
To restate my original question: how can we avoid breaking BPF selftests? From personal experience this happens somewhat regularly.
It can be avoided by people testing and letting me know when things break :)
thanks,
greg k-h
On Fri, May 26, 2023 at 6:43 PM Greg KH gregkh@linuxfoundation.org wrote:
So what tree(s) does this need to be backported to? I'm confused, this is a 6.2 email thread which is long end-of-life.
That would be 5.10 from what I can tell. Other LTS kernels have working tools/testing/selftests/bpf.
I replied here because you asked for examples :)
It can be avoided by people testing and letting me know when things break :)
Fair enough. Earlier you said:
And selftests should NOT be broken on stable releases, if so, something is wrong as no other subsystem has that happen.
Why is bpf special in this regard then?
Best Lorenz
linux-kselftest-mirror@lists.linaro.org
-
Greg KH -
Lorenz Bauer -
Sasha Levin