netdev - Re: [PATCH bpf-next 1/5] selftests/bpf: fix btf

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 28 Feb 2019 15:52:12 -0300
From:   Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
To:     Andrii Nakryiko <andriin@...com>
Cc:     andrii.nakryiko@...il.com, kernel-team@...com, ast@...com,
        netdev@...r.kernel.org, bpf@...r.kernel.org, daniel@...earbox.net
Subject: Re: [PATCH bpf-next 1/5] selftests/bpf: fix btf_dedup testing code

Em Wed, Feb 27, 2019 at 02:46:37PM -0800, Andrii Nakryiko escreveu:
> btf_dedup testing code doesn't account for length of struct btf_header
> when calculating the start of a string section. This patch fixes this
> problem.
> 
> Fixes: 49b57e0d01db ("tools/bpf: remove btf__get_strings() superseded by raw data API")

I think this clarifies things, but a Fixes seems excessive, right? I.e.
if you missed it in both sides of the (a != b) expression, the test will
be just as valid.

I say this because Fixes tags are now tracked and generates backporting
efforts that sometimes end up causing unnecessary human exchanges when
the patches don't apply because of some other patch.o

Or is there some further use of 'test_strs' and 'expect_strs' further
down that do_test_dedup() function?

/me scratches head, probably missing something...

- Arnaldo

> Signed-off-by: Andrii Nakryiko <andriin@...com>
> ---
>  tools/testing/selftests/bpf/.gitignore | 1 +
>  tools/testing/selftests/bpf/test_btf.c | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> index e47168d1257d..3b74d23fffab 100644
> --- a/tools/testing/selftests/bpf/.gitignore
> +++ b/tools/testing/selftests/bpf/.gitignore
> @@ -14,6 +14,7 @@ feature
>  test_libbpf_open
>  test_sock
>  test_sock_addr
> +test_sock_fields
>  urandom_read
>  test_btf
>  test_sockmap
> diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
> index 02d314383a9c..1426c0a905c8 100644
> --- a/tools/testing/selftests/bpf/test_btf.c
> +++ b/tools/testing/selftests/bpf/test_btf.c
> @@ -5936,9 +5936,9 @@ static int do_test_dedup(unsigned int test_num)
>  	}
>  
>  	test_hdr = test_btf_data;
> -	test_strs = test_btf_data + test_hdr->str_off;
> +	test_strs = test_btf_data + sizeof(*test_hdr) + test_hdr->str_off;
>  	expect_hdr = expect_btf_data;
> -	expect_strs = expect_btf_data + expect_hdr->str_off;
> +	expect_strs = expect_btf_data + sizeof(*test_hdr) + expect_hdr->str_off;
>  	if (CHECK(test_hdr->str_len != expect_hdr->str_len,
>  		  "test_hdr->str_len:%u != expect_hdr->str_len:%u",
>  		  test_hdr->str_len, expect_hdr->str_len)) {
> -- 
> 2.17.1

-- 

- Arnaldo