Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220421132317.1583867-1-asavkov@redhat.com>
In-Reply-To: <20220421132317.1583867-1-asavkov@redhat.com>
From:   Andrii Nakryiko <andrii.nakryiko@gmail.com>
Date:   Thu, 21 Apr 2022 09:51:34 -0700
Message-ID: <CAEf4Bzanrz2A==nkjV6+irPPKvGzxXQBzkLPS6spL54Sc22JkA@mail.gmail.com>
Subject: Re: [PATCH bpf-next] selftests/bpf: fix prog_tests/uprobe_autoattach
 compilation error
To:     Artem Savkov <asavkov@redhat.com>
Cc:     Alan Maguire <alan.maguire@oracle.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>, bpf <bpf@vger.kernel.org>,
        Networking <netdev@vger.kernel.org>,
        open list <linux-kernel@vger.kernel.org>,
        "open list:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Apr 21, 2022 at 6:23 AM Artem Savkov <asavkov@redhat.com> wrote:
>
> I am getting the following compilation error for prog_tests/uprobe_autoat=
tach.c
>
> tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c: In function =
=E2=80=98test_uprobe_autoattach=E2=80=99:
> ./test_progs.h:209:26: error: pointer =E2=80=98mem=E2=80=99 may be used a=
fter =E2=80=98free=E2=80=99 [-Werror=3Duse-after-free]
>
> mem variable is now used in one of the asserts so it shouldn't be freed r=
ight
> 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/ret=
urn values")
> Signed-off-by: Artem Savkov <asavkov@redhat.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 nam=
e */
>         mem =3D malloc(malloc_sz);
> -       free(mem);
>
>         ASSERT_EQ(skel->bss->uprobe_byname_parm1, trigger_val, "check_upr=
obe_byname_parm1");
>         ASSERT_EQ(skel->bss->uprobe_byname_ran, 1, "check_uprobe_byname_r=
an");
> @@ -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_b=
yname2_ran");
> +
> +       free(mem);
>  cleanup:
>         test_uprobe_autoattach__destroy(skel);
>  }
> --
> 2.35.1
>