Thread: Re: Avoid possible dereference null pointer (contrib/postgres_fdw/postgres_fdw.c)
Re: Avoid possible dereference null pointer (contrib/postgres_fdw/postgres_fdw.c)
From
Fujii Masao
Date:
On 2025/06/17 4:48, Ranier Vilela wrote:
> Hi.
>
> In the function *estimate_path_cost_size* the parameter
> fpextra can be NULL.
Yes.
> It is necessary to always check its validity,
> as is already done in other parts of the source.
>
> patch attached.
adjust_foreign_grouping_path_cost(root, pathkeys,
retrieved_rows, width,
- fpextra->limit_tuples,
+ fpextra ? fpextra->limit_tuples : 0.0,
&disabled_nodes,
&startup_cost, &run_cost);
I couldn't find a query that would reach this code path with
fpextra == NULL, but I agree the current code is fragile.
So I think it's a good idea to add the check before accessing
the field.
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
Re: Avoid possible dereference null pointer (contrib/postgres_fdw/postgres_fdw.c)
From
Fujii Masao
Date:
On 2025/06/17 20:37, Ranier Vilela wrote: > Em ter., 17 de jun. de 2025 às 06:09, Etsuro Fujita <etsuro.fujita@gmail.com <mailto:etsuro.fujita@gmail.com>> escreveu: > > Hi, > > On Tue, Jun 17, 2025 at 2:38 PM Fujii Masao <masao.fujii@oss.nttdata.com <mailto:masao.fujii@oss.nttdata.com>> wrote: > > adjust_foreign_grouping_path_cost(root, pathkeys, > > retrieved_rows,width, > > - fpextra->limit_tuples, > > + fpextra ? fpextra->limit_tuples: 0.0, > > &disabled_nodes, > > &startup_cost,&run_cost); > > > > I couldn't find a query that would reach this code path with > > fpextra == NULL, but I agree the current code is fragile. > > So I think it's a good idea to add the check before accessing > > the field. > > We get here only when called from add_foreign_ordered_paths() or > add_foreign_final_paths(), in which cases fpextra is always set, so it > cannot be NULL. No? > > False. > > In the function *postgresGetForeignRelSize* there is one call, > where fpextra is NULL. I think Etsuro-san meant that the problematic code path is only reachable when estimate_path_cost_size() is called from add_foreign_ordered_paths() or add_foreign_final_paths(), and in those cases, fpextra is guaranteed to be non-NULL. In other cases, such as postgresGetForeignRelSize(), fpextra can be NULL, but the code path in question isn't reached - for example, because pathkeys is NIL. As I mentioned earlier, I haven't found a case where this actually causes a crash, so Etsuro-san's analysis seems valid. That said, I still think it's safer to guard against NULL by checking fpextra before accessing its fields, as is done elsewhere. Regards, -- Fujii Masao NTT DATA Japan Corporation