[PATCH bpf-next] tools/bpf: check precise {func,line,jited_line}_info_rec_size in test_btf

[PATCH bpf-next] tools/bpf: check precise {func,line,jited_line}_info_rec_size in test_btf

[Yonghong Song]

Current btf func_info, line_info and jited_line are designed to be
extensible. The record sizes for {func,line}_info are passed to kernel,
and the record sizes for {func,line,jited_line}_info are returned to
userspace during bpf_prog_info query.

In bpf selftests test_btf.c, when testing whether kernel returns
a legitimate {func,line, jited_line)_info rec_size, the test only
compares to the minimum allowed size. If the returned rec_size is smaller
than the minimum allowed size, it is considered incorrect.
The minimum allowed size for these three info sizes are equal to
current value of sizeof(struct bpf_func_info), sizeof(struct bpf_line_info)
and sizeof(__u64).

The original thinking was that in the future when rec_size is increased
in kernel, the same test should run correctly. But this sacrificed
the precision of testing under the very kernel the test is shipped with,
and bpf selftest is typically run with the same repo kernel.

So this patch changed the testing of rec_size such that the
kernel returned value should be equal to the size defined by
tools uapi header bpf.h which syncs with kernel uapi header.

Martin discovered a bug in one of rec_size comparisons.
Instead of comparing to minimum func_info rec_size 8, it compares to 4.
This patch fixed that issue as well.

Fixes: 999d82cbc044 ("tools/bpf: enhance test_btf file testing to test func info")
Fixes: 05687352c600 ("bpf: Refactor and bug fix in test_func_type in test_btf.c")
Fixes: 4d6304c76355 ("bpf: Add unit tests for bpf_line_info")
Suggested-by: Martin KaFai Lau <kafai@fb.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
 tools/testing/selftests/bpf/test_btf.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_btf.c b/tools/testing/selftests/bpf/test_btf.c
index b783aa2212cc..8024b7d4c354 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -3408,7 +3408,7 @@ static int do_test_file(unsigned int test_num)
 		goto done;
 	}
 	rec_size = info.func_info_rec_size;
-	if (CHECK(rec_size < 4,
+	if (CHECK(rec_size != sizeof(struct bpf_func_info),
 		  "incorrect info.func_info_rec_size (1st) %d\n", rec_size)) {
 		err = -1;
 		goto done;
@@ -4544,7 +4544,7 @@ static int test_get_finfo(const struct prog_info_raw_test *test,
 	}
 
 	rec_size = info.func_info_rec_size;
-	if (CHECK(rec_size < 8,
+	if (CHECK(rec_size != sizeof(struct bpf_func_info),
 		  "incorrect info.func_info_rec_size (1st) %d", rec_size)) {
 		return -1;
 	}
@@ -4573,7 +4573,7 @@ static int test_get_finfo(const struct prog_info_raw_test *test,
 		err = -1;
 		goto done;
 	}
-	if (CHECK(info.func_info_rec_size < 8,
+	if (CHECK(info.func_info_rec_size != rec_size,
 		  "incorrect info.func_info_rec_size (2nd) %d",
 		  info.func_info_rec_size)) {
 		err = -1;
@@ -4649,8 +4649,8 @@ static int test_get_linfo(const struct prog_info_raw_test *test,
 		goto done;
 	}
 
-	if (CHECK(info.line_info_rec_size < 16 ||
-		  info.jited_line_info_rec_size < 8,
+	if (CHECK(info.line_info_rec_size != sizeof(struct bpf_line_info) ||
+		  info.jited_line_info_rec_size != sizeof(__u64),
 		  "info: line_info_rec_size:%u(userspace expected:%u) jited_line_info_rec_size:%u(userspace expected:%u)",
 		  info.line_info_rec_size, rec_size,
 		  info.jited_line_info_rec_size, jited_rec_size)) {
-- 
2.17.1

Re: [PATCH bpf-next] tools/bpf: check precise {func,line,jited_line}_info_rec_size in test_btf

[Daniel Borkmann]

On 12/18/2018 02:31 AM, Yonghong Song wrote:
> Current btf func_info, line_info and jited_line are designed to be
> extensible. The record sizes for {func,line}_info are passed to kernel,
> and the record sizes for {func,line,jited_line}_info are returned to
> userspace during bpf_prog_info query.
> 
> In bpf selftests test_btf.c, when testing whether kernel returns
> a legitimate {func,line, jited_line)_info rec_size, the test only
> compares to the minimum allowed size. If the returned rec_size is smaller
> than the minimum allowed size, it is considered incorrect.
> The minimum allowed size for these three info sizes are equal to
> current value of sizeof(struct bpf_func_info), sizeof(struct bpf_line_info)
> and sizeof(__u64).
> 
> The original thinking was that in the future when rec_size is increased
> in kernel, the same test should run correctly. But this sacrificed
> the precision of testing under the very kernel the test is shipped with,
> and bpf selftest is typically run with the same repo kernel.
> 
> So this patch changed the testing of rec_size such that the
> kernel returned value should be equal to the size defined by
> tools uapi header bpf.h which syncs with kernel uapi header.
> 
> Martin discovered a bug in one of rec_size comparisons.
> Instead of comparing to minimum func_info rec_size 8, it compares to 4.
> This patch fixed that issue as well.
> 
> Fixes: 999d82cbc044 ("tools/bpf: enhance test_btf file testing to test func info")
> Fixes: 05687352c600 ("bpf: Refactor and bug fix in test_func_type in test_btf.c")
> Fixes: 4d6304c76355 ("bpf: Add unit tests for bpf_line_info")
> Suggested-by: Martin KaFai Lau <kafai@fb.com>
> Acked-by: Martin KaFai Lau <kafai@fb.com>
> Signed-off-by: Yonghong Song <yhs@fb.com>

Applied to bpf-next, thanks!