Re: BUG #14928: Unchecked SearchSysCacheCopy1() return value

On 2017/11/28 9:25, Tom Lane wrote:
> Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> writes:
>> On 2017/11/27 18:13, bianpan2016@163.com wrote:
>>> Function SearchSysCacheCopy1() may return a NULL pointer if there is no
>>> enough memory. But in function ATExecDetachPartition(), its return value is
>>> not checked, which may result in NULL dereference (see line 13818).
> 
>> Thanks for the report.  Attached a patch that adds a check that tuple is
>> valid before trying to dereference it.
> 
> Pushed.

Thank you.

> Checking other SearchSysCache calls in these two files, I noted
> a third instance of the same problem in StorePartitionKey().  However,
> looking closer, StorePartitionKey never does anything at all with the
> result of its SearchSysCache1(PARTRELID, ...) lookup, including never
> releasing the syscache reference.  How is it that we don't get refcount
> leak warnings?  I guess that must prove that that lookup always fails,
> which is not too surprising since it seems to be against the partition
> key info that we haven't stored yet.  Anyway, I just diked that one
> out, since it's clearly useless.

Thank you, too.  I guess that SearchSysCache was from one of the earliest
versions of the patch, whereby we'd error out if the tuple we got out was
valid; that is, error for trying to set the partition of key of a table
that already had one.

Thanks,
Amit