Re: pgsql: PL/Python: Fix potential NULL pointer dereference

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Cc: pgsql-committers(at)postgresql(dot)org
Subject: Re: pgsql: PL/Python: Fix potential NULL pointer dereference
Date: 2017-12-05 19:45:59
Message-ID: 16695.1512503159@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> writes:
> On 11/28/17 13:35, Peter Eisentraut wrote:
>> OK, I'll revert and rethink.

> How about this instead?

This looks better, but I think there are a couple more things:

* Seems like you ought to s/Py_DECREF(result)/Py_XDECREF(result)/ in the
PG_CATCH block (line 451 in HEAD). As this is coded, although result
is set to NULL within the PG_TRY, it doesn't seem possible for control
to reach PG_CATCH after that ... but that's a pretty fragile assumption.

* You'd also need to declare "result" as
PLyResultObject *volatile result;
if you intend to change it within the PG_TRY block.

On the whole it seems like it might be better to dodge this whole business
of changing "result" inside the TRY. You could do that if you did
something like

result->rows = PyList_New(rows);
- if (!result->rows)
- {
- Py_DECREF(result);
- result = NULL;
- }
- else
+ if (result->rows)
{
PLy_input_setup_tuple(&ininfo, tuptable->tupdesc,

and then add after the PG_END_TRY

if (!result->rows)
{
Py_DECREF(result);
result = NULL;
}

The repeated test is a bit annoying but maybe it's better than the
alternative. You'd probably need a comment explaining why it's
like that, though.

regards, tom lane

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Thomas Munro 2017-12-05 20:05:44 Re: pgsql: Add some regression tests that exercise hash join code.
Previous Message Robert Haas 2017-12-05 19:39:29 pgsql: Fix accumulation of parallel worker instrumentation.