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

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 9 Oct 2023 22:53:57 +0800
From:   Yicong Yang <yangyicong@...wei.com>
To:     Kuan-Wei Chiu <visitorckw@...il.com>, <yangyicong@...ilicon.com>,
        <jonathan.cameron@...wei.com>, <namhyung@...nel.org>
CC:     <peterz@...radead.org>, <mingo@...hat.com>, <acme@...nel.org>,
        <mark.rutland@....com>, <alexander.shishkin@...ux.intel.com>,
        <jolsa@...nel.org>, <irogers@...gle.com>,
        <adrian.hunter@...el.com>, <linux-kernel@...r.kernel.org>,
        <linux-perf-users@...r.kernel.org>
Subject: Re: [PATCH] perf hisi-ptt: Fix memory leak in lseek failure handling

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@...il.com>
> ---
>  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@...ilicon.com>

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);
>