Re: [PATCH] perf hisi-ptt: Fix memory leak in lseek failure handling

[Yicong Yang]

On 2023/9/30 15:27, Kuan-Wei Chiu wrote:
> In the previous code, there was a memory leak issue where the previously
> allocated memory was not freed upon a failed lseek operation. This patch
> addresses the problem by releasing the old memory before returning -errno
> in case of a lseek failure. This ensures that memory is properly managed
> and avoids potential memory leaks.
> 
> Signed-off-by: Kuan-Wei Chiu <visitorckw@xxxxxxxxx>
> ---
>  tools/perf/util/hisi-ptt.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/hisi-ptt.c b/tools/perf/util/hisi-ptt.c
> index 45b614bb73bf..43bd1ca62d58 100644
> --- a/tools/perf/util/hisi-ptt.c
> +++ b/tools/perf/util/hisi-ptt.c
> @@ -108,8 +108,10 @@ static int hisi_ptt_process_auxtrace_event(struct perf_session *session,
>  		data_offset = 0;
>  	} else {
>  		data_offset = lseek(fd, 0, SEEK_CUR);
> -		if (data_offset == -1)
> +		if (data_offset == -1) {
> +			free(data);
>  			return -errno;
> +		}

Sorry for the late reply, this looks correct to me:

Acked-by: Yicong Yang <yangyicong@xxxxxxxxxxxxx>

This patch makes me think whether data_offset is necessary for PTT.
The PTT only supports dump raw trace data and since this is an
uncore PMU we only have one single data source so we are not using
the auxtrace_queues for sorting or ordering the buffers. It seems
there's no need to record the data_offset for later mmap(),
just read trace data out and dump them is enough. I'll further check
and have some tests on this.

I see Namhyung has already taken this. Thanks for the fix.

>  	}
>  
>  	err = readn(fd, data, size);
>