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

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 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, 21 Apr 2022 09:51:34 -0700
From:   Andrii Nakryiko <andrii.nakryiko@...il.com>
To:     Artem Savkov <asavkov@...hat.com>
Cc:     Alan Maguire <alan.maguire@...cle.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>,
        Networking <netdev@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>
Subject: Re: [PATCH bpf-next] selftests/bpf: fix prog_tests/uprobe_autoattach
 compilation error

On Thu, Apr 21, 2022 at 6:23 AM Artem Savkov <asavkov@...hat.com> wrote:
>
> I am getting the following compilation error for prog_tests/uprobe_autoattach.c
>
> tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c: In function ‘test_uprobe_autoattach’:
> ./test_progs.h:209:26: error: pointer ‘mem’ may be used after ‘free’ [-Werror=use-after-free]
>
> mem variable is now used in one of the asserts so it shouldn't be freed right
> away. Move free(mem) after the assert block.

The memory is not used, we only compare the value of the pointer
itself, we don't dereference. So the compiler is being paranoid here.
But while initial version relied on free() to happen before all the
ASSERTs, now we don't, so moving free after asserts is fine.

>
> Fixes: 1717e248014c ("selftests/bpf: Uprobe tests should verify param/return values")
> Signed-off-by: Artem Savkov <asavkov@...hat.com>
> ---
>  tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> index d6003dc8cc99..35b87c7ba5be 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c
> @@ -34,7 +34,6 @@ void test_uprobe_autoattach(void)
>
>         /* trigger & validate shared library u[ret]probes attached by name */
>         mem = malloc(malloc_sz);
> -       free(mem);
>
>         ASSERT_EQ(skel->bss->uprobe_byname_parm1, trigger_val, "check_uprobe_byname_parm1");
>         ASSERT_EQ(skel->bss->uprobe_byname_ran, 1, "check_uprobe_byname_ran");
> @@ -44,6 +43,8 @@ void test_uprobe_autoattach(void)
>         ASSERT_EQ(skel->bss->uprobe_byname2_ran, 3, "check_uprobe_byname2_ran");
>         ASSERT_EQ(skel->bss->uretprobe_byname2_rc, mem, "check_uretprobe_byname2_rc");
>         ASSERT_EQ(skel->bss->uretprobe_byname2_ran, 4, "check_uretprobe_byname2_ran");
> +
> +       free(mem);
>  cleanup:
>         test_uprobe_autoattach__destroy(skel);
>  }
> --
> 2.35.1
>