Re: Fix possible dereference null pointer (src/backend/replication/logical/reorderbuffer.c)

Em qua., 10 de abr. de 2024 às 18:28, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
escreveu:

> On 10/04/2024 21:07, Ranier Vilela wrote:
> > Hi,
> >
> > Per Coverity.
> >
> > The function ReorderBufferTXNByXid,
> > can return NULL when the parameter *create* is false.
> >
> > In the functions ReorderBufferSetBaseSnapshot
> > and ReorderBufferXidHasBaseSnapshot,
> > the second call to ReorderBufferTXNByXid,
> > pass false to *create* argument.
> >
> > In the function ReorderBufferSetBaseSnapshot,
> > fixed passing true as argument to always return
> > a valid ReorderBufferTXN pointer.
> >
> > In the function ReorderBufferXidHasBaseSnapshot,
> > fixed by checking if the pointer is NULL.
>
> If it's a "known subxid", the top-level XID should already have its
> ReorderBufferTXN entry, so ReorderBufferTXN() should never return NULL.
>
There are several conditions to not return NULL,
I think trusting never is insecure.

It's not surprising if Coverity doesn't understand that, but setting the
> 'create' flag doesn't seem like the right fix.

ReorderBufferSetBaseSnapshot always expects that *txn* exists.
If a second call fails, the only solution is to create a new one, no?

> If we add "Assert(txn !=
> NULL)", does that silence it?
>
I think no.
I always compile it as a release to send to Coverity.

best regards,
Ranier Vilela