Re: Possible dereference after null check (src/backend/executor/ExecUtils.c)

Em sex., 12 de fev. de 2021 às 13:11, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
escreveu:

> Em sex., 12 de fev. de 2021 às 03:28, Kyotaro Horiguchi <
> horikyota(dot)ntt(at)gmail(dot)com> escreveu:
>
>> At Wed, 10 Feb 2021 19:54:46 -0300, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
>> wrote in
>> > Hi,
>> >
>> > Per Coverity.
>> >
>> > The functions ExecGetInsertedCols and ExecGetUpdatedCols at ExecUtils.c
>> > only are safe to call if the variable "ri_RangeTableIndex" is != 0.
>> >
>> > Otherwise a possible Dereference after null check (FORWARD_NULL) can be
>> > raised.
>>
>> As it turns out, it's a false positive. And perhaps we don't want to
>> take action just to satisfy the static code analyzer.
>>
>>
>> The coment in ExecGetInsertedCols says:
>>
>> > /*
>> > * The columns are stored in the range table entry. If this
>> ResultRelInfo
>> > * doesn't have an entry in the range table (i.e. if it represents a
>> > * partition routing target), fetch the parent's RTE and map the columns
>> > * to the order they are in the partition.
>> > */
>> > if (relinfo->ri_RangeTableIndex != 0)
>> > {
>>
>> This means that any one of the two is always usable here. AFAICS,
>> actually, ri_RangeTableIndex is non-zero for partitioned (=leaf) and
>> non-partitoned relations and ri_RootResultRelInfo is non-null for
>> partitioned (parent or intermediate) relations (since they don't have
>> a coressponding range table entry).
>>
>> The only cases where both are 0 and NULL are trigger-use, which is
>> unrelated to the code path.
>>
> This is a case where it would be worth an assertion.
> What do you think?
>
Apparently my efforts with Coverity have been worth it.
And together we are helping to keep Postgres more secure.
Although sometimes it is not recognized for that [1].

regards,
Ranier Vilela

[1]
https://github.com/postgres/postgres/commit/54e51dcde03e5c746e8de6243c69fafdc8d0ec7a