Re all: Report some potential memory leak bugs in pg_dump.c

From: wliang(at)stu(dot)xidian(dot)edu(dot)cn
To: pgsql-bugs <pgsql-bugs(at)lists(dot)postgresql(dot)org>
Cc: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>, david(dot)g(dot)johnston(at)gmail(dot)com
Subject: Re all: Report some potential memory leak bugs in pg_dump.c
Date: 2022-02-19 08:04:38
Message-ID: 1f102fb7.f70.17f1102ec92.Coremail.wliang@stu.xidian.edu.cn
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

However, there are also some potential leaks caused by convertTSFunction() rather than getFormattedTypeName(). Besides, in convertTSFunction(), there are no any comment to say that the memory should not be freed by the caller.

12262/*
12263 * Convert a function OID obtained from pg_ts_parser or pg_ts_template
12264 *
12265 * It is sufficient to use REGPROC rather than REGPROCEDURE, since the
12266 * argument lists of these functions are predetermined. Note that the
12267 * caller should ensure we are in the proper schema, because the results
12268 * are search path dependent!
12269 */
12270static char *
12271convertTSFunction(Archive *fout, Oid funcOid)
12272{
12273char *result;
12274charquery[128];
12275PGresult *res;
12276
12277snprintf(query, sizeof(query),
12278 "SELECT '%u'::pg_catalog.regproc", funcOid);
12279res = ExecuteSqlQueryForSingleRow(fout, query);
12280
12281result = pg_strdup(PQgetvalue(res, 0, 0));
12282
12283PQclear(res);
12284
12285return result;
12286}

The following seven suspects are caused by convertTSFunction().

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

>"David G. Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> writes:
>> On Fri, Feb 18, 2022 at 10:59 PM <wliang(at)stu(dot)xidian(dot)edu(dot)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

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Julien Rouhaud 2022-02-19 11:10:05 Re: Re all: Report some potential memory leak bugs in pg_dump.c
Previous Message Tom Lane 2022-02-19 07:04:14 Re: Report some potential memory leak bugs in pg_dump.c