Re: Possible dereference null return (src/backend/replication/logical/reorderbuffer.c) - Mailing list pgsql-hackers

From Kyotaro Horiguchi
Subject Re: Possible dereference null return (src/backend/replication/logical/reorderbuffer.c)
Date
Msg-id 20210212.155602.691574878609873242.horikyota.ntt@gmail.com
Whole thread Raw
In response to Possible dereference null return (src/backend/replication/logical/reorderbuffer.c)  (Ranier Vilela <ranier.vf@gmail.com>)
Responses Re: Possible dereference null return (src/backend/replication/logical/reorderbuffer.c)
Re: Possible dereference null return (src/backend/replication/logical/reorderbuffer.c)
List pgsql-hackers
At Wed, 10 Feb 2021 20:12:38 -0300, Ranier Vilela <ranier.vf@gmail.com> wrote in 
> Hi,
> 
> Per Coverity.
> 
> If xid is a subtransaction, the setup of base snapshot on the top-level
> transaction,
> can be not optional, otherwise a Dereference null return value
> (NULL_RETURNS) can be raised.
> 
> Patch suggestion to fix this.
> 
> diff --git a/src/backend/replication/logical/reorderbuffer.c
> b/src/backend/replication/logical/reorderbuffer.c
> index 5a62ab8bbc..3c6a81f716 100644
> --- a/src/backend/replication/logical/reorderbuffer.c
> +++ b/src/backend/replication/logical/reorderbuffer.c
> @@ -2993,8 +2993,8 @@ ReorderBufferSetBaseSnapshot(ReorderBuffer *rb,
> TransactionId xid,
>   */
>   txn = ReorderBufferTXNByXid(rb, xid, true, &is_new, lsn, true);
>   if (rbtxn_is_known_subxact(txn))
> - txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, false,
> - NULL, InvalidXLogRecPtr, false);
> + txn = ReorderBufferTXNByXid(rb, txn->toplevel_xid, true,
> + NULL, InvalidXLogRecPtr, true);
>   Assert(txn->base_snapshot == NULL);

If the return from the first call is a subtransaction, the second call
always obtain the top transaction.  If the top transaction actualy did
not exist, it's rather the correct behavior to cause SEGV, than
creating a bogus rbtxn. THus it is wrong to set create=true and
create_as_top=true.  We could change the assertion like Assert (txn &&
txn->base_snapshot) to make things clearer.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center



pgsql-hackers by date:

Previous
From: "tsunakawa.takay@fujitsu.com"
Date:
Subject: RE: Parallel INSERT (INTO ... SELECT ...)
Next
From: "osumi.takamichi@fujitsu.com"
Date:
Subject: RE: [HACKERS] logical decoding of two-phase transactions