Postgres Pro
Downloads
Home   >   mailing lists

Thread: [HACKERS] Suspicious place in heap_prepare_freeze_tuple()

[HACKERS] Suspicious place in heap_prepare_freeze_tuple()

From
Teodor Sigaev
Date:
Hi!

Playing around freezing tuple I found suspicious piece of code:

heap_prepare_freeze_tuple():
...
frz->t_infomask = tuple->t_infomask;
...
             frz->t_infomask &= ~HEAP_XMAX_BITS;
             frz->xmax = newxmax;
             if (flags & FRM_MARK_COMMITTED)
                 frz->t_infomask &= HEAP_XMAX_COMMITTED;

Seems, in last line it should be a bitwise OR instead of AND. Now this line 
cleans all bits in t_infomask which later will be copied directly in tuple.
-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
                                                    WWW: http://www.sigaev.ru/

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Attachment

Re: [HACKERS] Suspicious place in heap_prepare_freeze_tuple()

From
Alvaro Herrera
Date:
Teodor Sigaev wrote:

> Playing around freezing tuple I found suspicious piece of code:
> 
> heap_prepare_freeze_tuple():
> ...
> frz->t_infomask = tuple->t_infomask;
> ...
>             frz->t_infomask &= ~HEAP_XMAX_BITS;
>             frz->xmax = newxmax;
>             if (flags & FRM_MARK_COMMITTED)
>                 frz->t_infomask &= HEAP_XMAX_COMMITTED;
> 
> Seems, in last line it should be a bitwise OR instead of AND. Now this line
> cleans all bits in t_infomask which later will be copied directly in tuple.

I think you're right.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] Suspicious place in heap_prepare_freeze_tuple()

From
Masahiko Sawada
Date:
On Thu, Jul 6, 2017 at 1:36 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> Teodor Sigaev wrote:
>
>> Playing around freezing tuple I found suspicious piece of code:
>>
>> heap_prepare_freeze_tuple():
>> ...
>> frz->t_infomask = tuple->t_infomask;
>> ...
>>             frz->t_infomask &= ~HEAP_XMAX_BITS;
>>             frz->xmax = newxmax;
>>             if (flags & FRM_MARK_COMMITTED)
>>                 frz->t_infomask &= HEAP_XMAX_COMMITTED;
>>
>> Seems, in last line it should be a bitwise OR instead of AND. Now this line
>> cleans all bits in t_infomask which later will be copied directly in tuple.
>
> I think you're right.
>

I also think that's right. Should we back-patch it down to 9.3?

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Re: [HACKERS] Suspicious place in heap_prepare_freeze_tuple()

From
Alvaro Herrera
Date:
Masahiko Sawada wrote:
> On Thu, Jul 6, 2017 at 1:36 AM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
> > Teodor Sigaev wrote:
> >
> >> Playing around freezing tuple I found suspicious piece of code:
> >>
> >> heap_prepare_freeze_tuple():
> >> ...
> >> frz->t_infomask = tuple->t_infomask;
> >> ...
> >>             frz->t_infomask &= ~HEAP_XMAX_BITS;
> >>             frz->xmax = newxmax;
> >>             if (flags & FRM_MARK_COMMITTED)
> >>                 frz->t_infomask &= HEAP_XMAX_COMMITTED;
> >>
> >> Seems, in last line it should be a bitwise OR instead of AND. Now this line
> >> cleans all bits in t_infomask which later will be copied directly in tuple.
> >
> > I think you're right.
> 
> I also think that's right. Should we back-patch it down to 9.3?

Of course.  I think this could cause data corruption.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services