pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedReport some potential memory leak bugs in pg_dump.c
3+ messages / 3 participants
[nested] [flat]
* Report some potential memory leak bugs in pg_dump.c
@ 2022-02-19 05:45 wliang@stu.xidian.edu.cn
2022-02-19 06:15 ` Re: Report some potential memory leak bugs in pg_dump.c David G. Johnston <david.g.johnston@gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: wliang@stu.xidian.edu.cn @ 2022-02-19 05:45 UTC (permalink / raw)
To: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
Hi all,
I find some potential memory leaks in PostgresSQL 14.1, which are in bin/pg_dump/pg_dump.c.
The first one is in the function dumpBaseType().
Specifically, at line 10545 and line 10546, function getFormattedTypeName() is called, which allocates a chunk of memory by using pg_strdup() and returns it. The returned chunk is directly passed to appendPQExpBuffer() as its 3rd parameter. However, we find that the chunk is not freed in appendPQExpBuffer(). As a result, there is a memory leak.
10544 appendPQExpBuffer(q, ",\n ELEMENT = %s",
10545 getFormattedTypeName(fout, tyinfo->typelem,
10546 zeroIsError));
17850static const char *
17851getFormattedTypeName(Archive *fout, Oid oid, OidOptions opts)
17852{
…
17878result = pg_strdup(PQgetvalue(res, 0, 0));
…
17892return result;
17893}
Furthermore, we also find that there are a dozen of similar leaks in other functions as follow.
1) At lines 11315 to 11317, in function format_function_signature().
11315 appendPQExpBufferStr(&fn,
11316 getFormattedTypeName(fout, finfo-->argtypes[j],
11317 zeroIsError));
2) At lines 11571 to 11572, in function dumpFunc().
11571 appendPQExpBuffer(q, "FOR TYPE %s",
11572 getFormattedTypeName(fout, typeids[i], zeroAsNone));
3) At lines 11553 to 11556, in function dumpFunc().
11553 appendPQExpBuffer(q, " RETURNS %s%s",
11554 (proretset[0] == 't') ? "SETOF " : "",
11555 getFormattedTypeName(fout, finfo->prorettype,
11556 zeroIsError));
4) At lines 13125 to 13129, in function format_aggregate_signature().
13125 appendPQExpBuffer(&buf, "%s%s",
13126 (j > 0) ? ", " : "",
13127 getFormattedTypeName(fout,
13128 agginfo->aggfn.argtypes[j],
13129 zeroIsError));
5) At lines 13520 to 13521, in function dumpTSParser().
13520 appendPQExpBuffer(q, " START = %s,\n",
13521 convertTSFunction(fout, prsinfo->prsstart));
6) At lines 13522 to 13523, in function dumpTSParser().
13522 appendPQExpBuffer(q, " GETTOKEN = %s,\n",
13523 convertTSFunction(fout, prsinfo->prstoken));
7) At lines 13524 to 13525, in function dumpTSParser().
13524 appendPQExpBuffer(q, " END = %s,\n",
13525 convertTSFunction(fout, prsinfo->prsend));
8) At lines 13527 to 13528, in function dumpTSParser().
13527 appendPQExpBuffer(q, " HEADLINE = %s,\n",
13528 convertTSFunction(fout, prsinfo->prsheadline));
9) At lines 13529 to 13530, in function dumpTSParser().
13529 appendPQExpBuffer(q, " LEXTYPES = %s );\n",
13530 convertTSFunction(fout, prsinfo->prslextype));
10) At lines 13665 to 13666, in function dumpTSTemplate().
13665 appendPQExpBuffer(q, " INIT = %s,\n",
13666 convertTSFunction(fout, tmplinfo->tmplinit));
11) At lines 13667 to 13668, in function dumpTSTemplate().
13667 appendPQExpBuffer(q, " LEXIZE = %s );\n",
13668 convertTSFunction(fout, tmplinfo->tmpllexize));
12) At lines 15011 to 15013, in function dumpTableSchema().
15011 appendPQExpBuffer(q, " OF %s",
15012 getFormattedTypeName(fout, tbinfo->reloftype,
15013 zeroIsError));
We believe we can fix the problems by adding a variable to store the chunk pointer and employing pg_free() to free it after invoking appendPQExpBuffer().
I'm looking forward to your confirmation.
Best,
Wentao
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Report some potential memory leak bugs in pg_dump.c
2022-02-19 05:45 Report some potential memory leak bugs in pg_dump.c wliang@stu.xidian.edu.cn
@ 2022-02-19 06:15 ` David G. Johnston <david.g.johnston@gmail.com>
2022-02-19 07:04 ` Re: Report some potential memory leak bugs in pg_dump.c Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 3+ messages in thread
From: David G. Johnston @ 2022-02-19 06:15 UTC (permalink / raw)
To: wliang@stu.xidian.edu.cn; +Cc: pgsql-bugs <pgsql-bugs@lists.postgresql.org>
On Fri, Feb 18, 2022 at 10:59 PM <wliang@stu.xidian.edu.cn> wrote:
> Specifically, at line 10545 and line 10546, function
> getFormattedTypeName() is called, which allocates a chunk of memory by
> using pg_strdup() and returns it.
I'm not a C programmer but am operating under the assumption that you are
probably incorrect. So I took a cursory look at the code (in HEAD),
starting with the function comment. It says:
"* Note that the result is cached and must not be freed by the caller."
Then, using git blame, I found this commit:
https://github.com/postgres/postgres/commit/072e2f8a62002cb01ed6c4e161442e133509349e
That commit made it in before v14 was released and so was already in v14.1
David J.
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Report some potential memory leak bugs in pg_dump.c
2022-02-19 05:45 Report some potential memory leak bugs in pg_dump.c wliang@stu.xidian.edu.cn
2022-02-19 06:15 ` Re: Report some potential memory leak bugs in pg_dump.c David G. Johnston <david.g.johnston@gmail.com>
@ 2022-02-19 07:04 ` Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 3+ messages in thread
From: Tom Lane @ 2022-02-19 07:04 UTC (permalink / raw)
To: David G. Johnston <david.g.johnston@gmail.com>; +Cc: wliang@stu.xidian.edu.cn, pgsql-bugs <pgsql-bugs@lists.postgresql.org>
"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Fri, Feb 18, 2022 at 10:59 PM <wliang@stu.xidian.edu.cn> wrote:
>> Specifically, at line 10545 and line 10546, function
>> getFormattedTypeName() is called, which allocates a chunk of memory by
>> using pg_strdup() and returns it.
> I'm not a C programmer but am operating under the assumption that you are
> probably incorrect. So I took a cursory look at the code (in HEAD),
> starting with the function comment. It says:
> "* Note that the result is cached and must not be freed by the caller."
There's also this in the body of the function:
/*
* Cache the result for re-use in later requests, if possible. If we
* don't have a TypeInfo for the type, the string will be leaked once the
* caller is done with it ... but that case really should not happen, so
* leaking if it does seems acceptable.
*/
Since getTypes() makes a TypeInfo for every row of pg_type, "really
should not happen" is an accurate statement. You'd pretty much have to
be dealing with a catalog-corruption scenario for the non-cached path
to be taken.
regards, tom lane
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2022-02-19 07:04 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-02-19 05:45 Report some potential memory leak bugs in pg_dump.c wliang@stu.xidian.edu.cn
2022-02-19 06:15 ` David G. Johnston <david.g.johnston@gmail.com>
2022-02-19 07:04 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox