On Sun, Nov 7, 2021 at 8:34 AM Hengqi Chen <hengqi.chen@xxxxxxxxx> wrote:
>
> Hi, Andrii
>
> On 2021/11/7 12:03 PM, Andrii Nakryiko wrote:
> > Array holding per-cpu values wasn't freed. Fix that.
> >
> > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
> > ---
> > tools/testing/selftests/bpf/prog_tests/bpf_iter.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> > index 9454331aaf85..71c724a3f988 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> > @@ -770,6 +770,7 @@ static void test_bpf_percpu_hash_map(void)
> > bpf_link__destroy(link);
> > out:
> > bpf_iter_bpf_percpu_hash_map__destroy(skel);
> > + free(val);
> > }
> >
> > static void test_bpf_array_map(void)
> >
>
> The val is allocated at the very beginning of this function,
> when bpf_iter_bpf_percpu_hash_map__open failed, the val still
> leaked.
>
> So we should have:
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> index 9454331aaf85..ee6727389ef6 100644
> --- a/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> +++ b/tools/testing/selftests/bpf/prog_tests/bpf_iter.c
> @@ -686,7 +686,7 @@ static void test_bpf_percpu_hash_map(void)
> {
> __u32 expected_key_a = 0, expected_key_b = 0;
> DECLARE_LIBBPF_OPTS(bpf_iter_attach_opts, opts);
> - struct bpf_iter_bpf_percpu_hash_map *skel;
> + struct bpf_iter_bpf_percpu_hash_map *skel = NULL;
> int err, i, j, len, map_fd, iter_fd;
> union bpf_iter_link_info linfo;
> __u32 expected_val = 0;
> @@ -704,7 +704,7 @@ static void test_bpf_percpu_hash_map(void)
> skel = bpf_iter_bpf_percpu_hash_map__open();
> if (CHECK(!skel, "bpf_iter_bpf_percpu_hash_map__open",
> "skeleton open failed\n"))
> - return;
> + goto out;
>
I've just moved val = malloc() here and left early return intact. Same
effect, less undoing to do.
> skel->rodata->num_cpus = bpf_num_possible_cpus();
>
> @@ -770,6 +770,7 @@ static void test_bpf_percpu_hash_map(void)
> bpf_link__destroy(link);
> out:
> bpf_iter_bpf_percpu_hash_map__destroy(skel);
> + free(val);
> }
>
> static void test_bpf_array_map(void)
>
> Right?
Right, thanks for spotting!
>
> Cheers,
> --
> Hengqi
