Thread: Re: Avoid possible deference NULL pointer (src/backend/optimizer/path/allpaths.c)

On 05.01.2025 02:29, Ranier Vilela wrote:
> Hi.
>
> Per Coverity.
>
> All call sites of function *get_cheapest_path_for_pathkeys* checks
> for NULL returns.
>
> So, it is highly likely that the function will return NULL.
>
> IMO, the Assert in this particular call, is not fully effective.
>
> Fix removing the Assert and always check if the return is NULL.
>
> best regards,
> Ranier Vilela


Hi!

Thanks for noticing this. If this happens in the planner, it poses a 
serious risk of a segmentation fault that could crash the instance if a 
NULL pointer is dereferenced. Since checking for NULL is very cheap, I 
support this patch.

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.




Hi.

Em qua., 5 de fev. de 2025 às 13:51, Ilia Evdokimov <ilya.evdokimov@tantorlabs.com> escreveu:

On 05.01.2025 02:29, Ranier Vilela wrote:
> Hi.
>
> Per Coverity.
>
> All call sites of function *get_cheapest_path_for_pathkeys* checks
> for NULL returns.
>
> So, it is highly likely that the function will return NULL.
>
> IMO, the Assert in this particular call, is not fully effective.
>
> Fix removing the Assert and always check if the return is NULL.
>
> best regards,
> Ranier Vilela


Hi!

Thanks for noticing this. If this happens in the planner, it poses a
serious risk of a segmentation fault that could crash the instance if a
NULL pointer is dereferenced. Since checking for NULL is very cheap, I
support this patch.
Thanks for taking a look.

best regards,
Ranier Vilela
But what should we do if cheapest == NULL further? Should we return NULL 
of get_cheapest_parameterized_child_path() function?

If it is, we should write it like this:

if (cheapset == NULL || bms(PATH_REQ_OUTER(cheapset), required_outer))
     return cheapest;

I'll look into this issue further.

--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC.




Em qua., 5 de fev. de 2025 às 14:09, Ilia Evdokimov <ilya.evdokimov@tantorlabs.com> escreveu:
But what should we do if cheapest == NULL further? Should we return NULL
of get_cheapest_parameterized_child_path() function?

If it is, we should write it like this:

if (cheapset == NULL || bms(PATH_REQ_OUTER(cheapset), required_outer))
     return cheapest;
 
I think no in this case.
If cheapset is NULL, the logic is to continue the find.
What cannot happen is passing a null pointer to bms(PATH_REQ_OUTER.

best regards,
Ranier Vilela