netdev - Re: [PATCH bpf-next] selftests/bpf: fix trampoline

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:   Fri, 21 Feb 2020 14:04:24 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     Andrii Nakryiko <andriin@...com>
Cc:     bpf@...r.kernel.org, netdev@...r.kernel.org, ast@...com,
        daniel@...earbox.net, andrii.nakryiko@...il.com,
        kernel-team@...com, Jiri Olsa <jolsa@...nel.org>
Subject: Re: [PATCH bpf-next] selftests/bpf: fix trampoline_count clean up
 logic

On Thu, Feb 20, 2020 at 03:05:46PM -0800, Andrii Nakryiko wrote:
> Libbpf's Travis CI tests caught this issue. Ensure bpf_link and bpf_object
> clean up is performed correctly.
> 
> Fixes: d633d57902a5 ("selftest/bpf: Add test for allowed trampolines count")
> Cc: Jiri Olsa <jolsa@...nel.org>
> Signed-off-by: Andrii Nakryiko <andriin@...com>
> ---
>  .../bpf/prog_tests/trampoline_count.c         | 25 +++++++++++++------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> index 1f6ccdaed1ac..781c8d11604b 100644
> --- a/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> +++ b/tools/testing/selftests/bpf/prog_tests/trampoline_count.c
> @@ -55,31 +55,40 @@ void test_trampoline_count(void)
>  	/* attach 'allowed' 40 trampoline programs */
>  	for (i = 0; i < MAX_TRAMP_PROGS; i++) {
>  		obj = bpf_object__open_file(object, NULL);
> -		if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
> +		if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
> +			obj = NULL;
>  			goto cleanup;
> +		}
>  
>  		err = bpf_object__load(obj);
>  		if (CHECK(err, "obj_load", "err %d\n", err))
>  			goto cleanup;
>  		inst[i].obj = obj;
> +		obj = NULL;
>  
>  		if (rand() % 2) {
> -			link = load(obj, fentry_name);
> -			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link)))
> +			link = load(inst[i].obj, fentry_name);
> +			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link))) {
> +				link = NULL;
>  				goto cleanup;
> +			}
>  			inst[i].link_fentry = link;
>  		} else {
> -			link = load(obj, fexit_name);
> -			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link)))
> +			link = load(inst[i].obj, fexit_name);
> +			if (CHECK(IS_ERR(link), "attach prog", "err %ld\n", PTR_ERR(link))) {
> +				link = NULL;
>  				goto cleanup;
> +			}
>  			inst[i].link_fexit = link;
>  		}
>  	}
>  
>  	/* and try 1 extra.. */
>  	obj = bpf_object__open_file(object, NULL);
> -	if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj)))
> +	if (CHECK(IS_ERR(obj), "obj_open_file", "err %ld\n", PTR_ERR(obj))) {
> +		obj = NULL;
>  		goto cleanup;
> +	}
>  
>  	err = bpf_object__load(obj);
>  	if (CHECK(err, "obj_load", "err %d\n", err))
> @@ -104,7 +113,9 @@ void test_trampoline_count(void)
>  cleanup_extra:
>  	bpf_object__close(obj);
>  cleanup:
> -	while (--i) {
> +	if (i >= MAX_TRAMP_PROGS)
> +		i = MAX_TRAMP_PROGS - 1;
> +	for (; i >= 0; i--) {

ugh right, if we fail in first iteration, 'i' would get get -1 in here

thanks,
jirka

>  		bpf_link__destroy(inst[i].link_fentry);
>  		bpf_link__destroy(inst[i].link_fexit);
>  		bpf_object__close(inst[i].obj);
> -- 
> 2.17.1
>