psql memory leaks

Started by Kyotaro Horiguchialmost 3 years ago3 messages
#1Kyotaro Horiguchi
horikyota.ntt@gmail.com
1 attachment(s)

I noticed that \bind is leaking memory for each option.

=# SELECT $1, $2, $3 \ bind 1 2 3 \g

The leaked memory blocks are comming from
psql_scan_slash_option(). The attached small patch resolves that
issue. I looked through the function's call sites, but I didn't find
the same mistake.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachments:

0001-Fix-memory-leak-of-psql-s-bind.patchtext/x-patch; charset=us-asciiDownload
From e67f5820455079ad6cdc1039a8d44314a360841f Mon Sep 17 00:00:00 2001
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Date: Tue, 21 Feb 2023 11:49:54 +0900
Subject: [PATCH] Fix memory leak of psql's \bind

exec_command_bind() is discarding malloc'ed memory blocks returned
from psql_scan_slash_option() after strdup'ed them. To address this
issue, use the returned memory directly instead of making a copy.
---
 src/bin/psql/command.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index b5201edf55..955397ee9d 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -480,7 +480,7 @@ exec_command_bind(PsqlScanState scan_state, bool active_branch)
 				nalloc = nalloc ? nalloc * 2 : 1;
 				pset.bind_params = pg_realloc_array(pset.bind_params, char *, nalloc);
 			}
-			pset.bind_params[nparams - 1] = pg_strdup(opt);
+			pset.bind_params[nparams - 1] = opt;
 		}
 
 		pset.bind_nparams = nparams;
-- 
2.31.1

#2Corey Huinker
corey.huinker@gmail.com
In reply to: Kyotaro Horiguchi (#1)
Re: psql memory leaks

On Mon, Feb 20, 2023 at 9:56 PM Kyotaro Horiguchi <horikyota.ntt@gmail.com>
wrote:

I noticed that \bind is leaking memory for each option.

=# SELECT $1, $2, $3 \ bind 1 2 3 \g

The leaked memory blocks are comming from
psql_scan_slash_option(). The attached small patch resolves that
issue. I looked through the function's call sites, but I didn't find
the same mistake.

regards.

Good catch. Patch passes make check-world.

#3Michael Paquier
michael@paquier.xyz
In reply to: Corey Huinker (#2)
Re: psql memory leaks

On Tue, Feb 21, 2023 at 02:03:43PM -0500, Corey Huinker wrote:

Good catch. Patch passes make check-world.

Indeed. I was reviewing the whole and there could be a point in
resetting bind_nparams at the end of SendQuery() to keep a correct
track of what's saved in the pset data for the bind parameters, but
not doing so is not a big deal either because we'd just reset it once
the full allocation of the parameters is done and bind_flag is all
about that. The code paths leading to the free seem correct, seen
from here.
--
Michael