linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] perf: tests: fix some mmemory leak issues
@ 2021-07-02  4:09 gushengxian
  2021-07-02 12:58 ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 4+ messages in thread
From: gushengxian @ 2021-07-02  4:09 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung,
	linux-arm-kernel
  Cc: linux-perf-users, linux-kernel, gushengxian

From: gushengxian <gushengxian@yulong.com>

Some memory leak issues should be fixed by free().
Reported by cppcheck.

Signed-off-by: gushengxian <gushengxian@yulong.com>
---
 tools/perf/arch/arm/tests/dwarf-unwind.c     | 1 +
 tools/perf/arch/arm64/tests/dwarf-unwind.c   | 1 +
 tools/perf/arch/powerpc/tests/dwarf-unwind.c | 1 +
 tools/perf/arch/x86/tests/dwarf-unwind.c     | 1 +
 4 files changed, 4 insertions(+)

diff --git a/tools/perf/arch/arm/tests/dwarf-unwind.c b/tools/perf/arch/arm/tests/dwarf-unwind.c
index ccfa87055c4a..f2541d1dbb02 100644
--- a/tools/perf/arch/arm/tests/dwarf-unwind.c
+++ b/tools/perf/arch/arm/tests/dwarf-unwind.c
@@ -39,6 +39,7 @@ static int sample_ustack(struct perf_sample *sample,
 	memcpy(buf, (void *) sp, stack_size);
 	stack->data = (char *) buf;
 	stack->size = stack_size;
+	free(buf);
 	return 0;
 }
 
diff --git a/tools/perf/arch/arm64/tests/dwarf-unwind.c b/tools/perf/arch/arm64/tests/dwarf-unwind.c
index 46147a483049..ecd25c77a71f 100644
--- a/tools/perf/arch/arm64/tests/dwarf-unwind.c
+++ b/tools/perf/arch/arm64/tests/dwarf-unwind.c
@@ -39,6 +39,7 @@ static int sample_ustack(struct perf_sample *sample,
 	memcpy(buf, (void *) sp, stack_size);
 	stack->data = (char *) buf;
 	stack->size = stack_size;
+	free(buf);
 	return 0;
 }
 
diff --git a/tools/perf/arch/powerpc/tests/dwarf-unwind.c b/tools/perf/arch/powerpc/tests/dwarf-unwind.c
index 8efd9ed9e9db..e57a8ca52ce7 100644
--- a/tools/perf/arch/powerpc/tests/dwarf-unwind.c
+++ b/tools/perf/arch/powerpc/tests/dwarf-unwind.c
@@ -40,6 +40,7 @@ static int sample_ustack(struct perf_sample *sample,
 	memcpy(buf, (void *) sp, stack_size);
 	stack->data = (char *) buf;
 	stack->size = stack_size;
+	free(buf);
 	return 0;
 }
 
diff --git a/tools/perf/arch/x86/tests/dwarf-unwind.c b/tools/perf/arch/x86/tests/dwarf-unwind.c
index 478078fb0f22..da90b33a1e28 100644
--- a/tools/perf/arch/x86/tests/dwarf-unwind.c
+++ b/tools/perf/arch/x86/tests/dwarf-unwind.c
@@ -47,6 +47,7 @@ static int sample_ustack(struct perf_sample *sample,
 #endif
 	stack->data = (char *) buf;
 	stack->size = stack_size;
+	free(buf);
 	return 0;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf: tests: fix some mmemory leak issues
  2021-07-02  4:09 [PATCH] perf: tests: fix some mmemory leak issues gushengxian
@ 2021-07-02 12:58 ` Arnaldo Carvalho de Melo
  2021-07-02 13:56   ` Leo Yan
  0 siblings, 1 reply; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-02 12:58 UTC (permalink / raw)
  To: gushengxian
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, mark.rutland,
	alexander.shishkin, jolsa, namhyung, linux-arm-kernel,
	linux-perf-users, linux-kernel, gushengxian

Em Thu, Jul 01, 2021 at 09:09:55PM -0700, gushengxian escreveu:
> From: gushengxian <gushengxian@yulong.com>
> 
> Some memory leak issues should be fixed by free().
> Reported by cppcheck.

Thanks, applied.

- Arnaldo

 
> Signed-off-by: gushengxian <gushengxian@yulong.com>
> ---
>  tools/perf/arch/arm/tests/dwarf-unwind.c     | 1 +
>  tools/perf/arch/arm64/tests/dwarf-unwind.c   | 1 +
>  tools/perf/arch/powerpc/tests/dwarf-unwind.c | 1 +
>  tools/perf/arch/x86/tests/dwarf-unwind.c     | 1 +
>  4 files changed, 4 insertions(+)
> 
> diff --git a/tools/perf/arch/arm/tests/dwarf-unwind.c b/tools/perf/arch/arm/tests/dwarf-unwind.c
> index ccfa87055c4a..f2541d1dbb02 100644
> --- a/tools/perf/arch/arm/tests/dwarf-unwind.c
> +++ b/tools/perf/arch/arm/tests/dwarf-unwind.c
> @@ -39,6 +39,7 @@ static int sample_ustack(struct perf_sample *sample,
>  	memcpy(buf, (void *) sp, stack_size);
>  	stack->data = (char *) buf;
>  	stack->size = stack_size;
> +	free(buf);
>  	return 0;
>  }
>  
> diff --git a/tools/perf/arch/arm64/tests/dwarf-unwind.c b/tools/perf/arch/arm64/tests/dwarf-unwind.c
> index 46147a483049..ecd25c77a71f 100644
> --- a/tools/perf/arch/arm64/tests/dwarf-unwind.c
> +++ b/tools/perf/arch/arm64/tests/dwarf-unwind.c
> @@ -39,6 +39,7 @@ static int sample_ustack(struct perf_sample *sample,
>  	memcpy(buf, (void *) sp, stack_size);
>  	stack->data = (char *) buf;
>  	stack->size = stack_size;
> +	free(buf);
>  	return 0;
>  }
>  
> diff --git a/tools/perf/arch/powerpc/tests/dwarf-unwind.c b/tools/perf/arch/powerpc/tests/dwarf-unwind.c
> index 8efd9ed9e9db..e57a8ca52ce7 100644
> --- a/tools/perf/arch/powerpc/tests/dwarf-unwind.c
> +++ b/tools/perf/arch/powerpc/tests/dwarf-unwind.c
> @@ -40,6 +40,7 @@ static int sample_ustack(struct perf_sample *sample,
>  	memcpy(buf, (void *) sp, stack_size);
>  	stack->data = (char *) buf;
>  	stack->size = stack_size;
> +	free(buf);
>  	return 0;
>  }
>  
> diff --git a/tools/perf/arch/x86/tests/dwarf-unwind.c b/tools/perf/arch/x86/tests/dwarf-unwind.c
> index 478078fb0f22..da90b33a1e28 100644
> --- a/tools/perf/arch/x86/tests/dwarf-unwind.c
> +++ b/tools/perf/arch/x86/tests/dwarf-unwind.c
> @@ -47,6 +47,7 @@ static int sample_ustack(struct perf_sample *sample,
>  #endif
>  	stack->data = (char *) buf;
>  	stack->size = stack_size;
> +	free(buf);
>  	return 0;
>  }
>  
> -- 
> 2.25.1
> 

-- 

- Arnaldo

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf: tests: fix some mmemory leak issues
  2021-07-02 12:58 ` Arnaldo Carvalho de Melo
@ 2021-07-02 13:56   ` Leo Yan
  2021-07-02 17:57     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 4+ messages in thread
From: Leo Yan @ 2021-07-02 13:56 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: gushengxian, will, mathieu.poirier, peterz, mingo, mark.rutland,
	alexander.shishkin, jolsa, namhyung, linux-arm-kernel,
	linux-perf-users, linux-kernel, gushengxian

Hi Arnaldo,

On Fri, Jul 02, 2021 at 09:58:52AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jul 01, 2021 at 09:09:55PM -0700, gushengxian escreveu:
> > From: gushengxian <gushengxian@yulong.com>
> > 
> > Some memory leak issues should be fixed by free().
> > Reported by cppcheck.

I don't think this patch does the right thing.  You could see that the
memory is allocated in arch specific function sample_ustack(), and the
"buf" pointer is assigned to sample->user_stack.data; and the memory
actually is released in the caller function test_dwarf_unwind__thread:

noinline int test_dwarf_unwind__thread(struct thread *thread)
{
        struct perf_sample sample;
        unsigned long cnt = 0;
        int err = -1;

        memset(&sample, 0, sizeof(sample));

        if (test__arch_unwind_sample(&sample, thread)) {
                pr_debug("failed to get unwind sample\n");
                goto out;
        } 

        [...]

 out:
        zfree(&sample.user_stack.data);
        zfree(&sample.user_regs.regs);
        return err;
}

So this patch will break the testing and doesn't fix any memory leak
issue.

Thanks,
Leo

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] perf: tests: fix some mmemory leak issues
  2021-07-02 13:56   ` Leo Yan
@ 2021-07-02 17:57     ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-07-02 17:57 UTC (permalink / raw)
  To: Leo Yan
  Cc: gushengxian, will, mathieu.poirier, peterz, mingo, mark.rutland,
	alexander.shishkin, jolsa, namhyung, linux-arm-kernel,
	linux-perf-users, linux-kernel, gushengxian

Em Fri, Jul 02, 2021 at 09:56:41PM +0800, Leo Yan escreveu:
> Hi Arnaldo,
> 
> On Fri, Jul 02, 2021 at 09:58:52AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Thu, Jul 01, 2021 at 09:09:55PM -0700, gushengxian escreveu:
> > > From: gushengxian <gushengxian@yulong.com>
> > > 
> > > Some memory leak issues should be fixed by free().
> > > Reported by cppcheck.
> 
> I don't think this patch does the right thing.  You could see that the
> memory is allocated in arch specific function sample_ustack(), and the
> "buf" pointer is assigned to sample->user_stack.data; and the memory
> actually is released in the caller function test_dwarf_unwind__thread:
> 
> noinline int test_dwarf_unwind__thread(struct thread *thread)
> {
>         struct perf_sample sample;
>         unsigned long cnt = 0;
>         int err = -1;
> 
>         memset(&sample, 0, sizeof(sample));
> 
>         if (test__arch_unwind_sample(&sample, thread)) {
>                 pr_debug("failed to get unwind sample\n");
>                 goto out;
>         } 
> 
>         [...]
> 
>  out:
>         zfree(&sample.user_stack.data);
>         zfree(&sample.user_regs.regs);
>         return err;
> }
> 
> So this patch will break the testing and doesn't fix any memory leak
> issue.

You are right, those buffers are allocated in those functions and then
_returned_ via sample->user_stack, the tool (and myself, ugh) got
fooled, I'll remove that from my local tree.

Thanks Leo!

- Arnaldo

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-07-02 17:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-02  4:09 [PATCH] perf: tests: fix some mmemory leak issues gushengxian
2021-07-02 12:58 ` Arnaldo Carvalho de Melo
2021-07-02 13:56   ` Leo Yan
2021-07-02 17:57     ` Arnaldo Carvalho de Melo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).