Thread: BUG #8453: uninitialized memory access in pg_receivexlog and other bugs

The following bug has been logged on the website:

Bug reference:      8453
Logged by:          Andrew Gierth
Email address:      andrew@tao11.riddles.org.uk
PostgreSQL version: 9.3.0
Operating system:   any
Description:

The first snprintf in writeTimeLineHistoryFile in receivelog.c accesses
uninitialized data in the "path" variable, thus creating the .tmp file in a
random place (usually the current dir, leading to unexpected EXDEV errors on
the rename).


Also, receivexlog is ignoring .partial and .history files when determining
which timeline to start streaming from, which means that if there are two
timeline changes that are not separated by a WAL segment switch, it will
fail to operate due to attempting to start from a too-old timeline (for
which xlogs are not available on the server).


Found from my analysis of a report from irc.

On 15.09.2013 15:02, andrew@tao11.riddles.org.uk wrote:
> The following bug has been logged on the website:
>
> Bug reference:      8453
> Logged by:          Andrew Gierth
> Email address:      andrew@tao11.riddles.org.uk
> PostgreSQL version: 9.3.0
> Operating system:   any
> Description:
>
> The first snprintf in writeTimeLineHistoryFile in receivelog.c accesses
> uninitialized data in the "path" variable, thus creating the .tmp file in a
> random place (usually the current dir, leading to unexpected EXDEV errors on
> the rename).

Ouch, that was a silly bug!

> Also, receivexlog is ignoring .partial and .history files when determining
> which timeline to start streaming from, which means that if there are two
> timeline changes that are not separated by a WAL segment switch, it will
> fail to operate due to attempting to start from a too-old timeline (for
> which xlogs are not available on the server).

There's nothing we can do with .history files here. The point is to find
out how far we have already received WAL, and the presence of a .history
file doesn't tell you anything about that.

There is a comment about .partial files though:

>         /*
>          * Check if the filename looks like an xlog file, or a .partial file.
>          * Xlog files are always 24 characters, and .partial files are 32
>          * characters.
>          */
>         if (strlen(dirent->d_name) != 24 ||
>             strspn(dirent->d_name, "0123456789ABCDEF") != 24)
>             continue;

The comment says that .partial files are taken into account, but the
code doesn't match the comment.

Attached is a patch to fix both of these issues. I'm too tired right now
to thoroughly test it and commit, so I'll get back to this tomorrow.
Meanwhile, please take a look and let me know if you can see something
wrong.

- Heikki

>>>>> "Heikki" == Heikki Linnakangas <hlinnakangas@vmware.com> writes:

 >> Also, receivexlog is ignoring .partial and .history files when
 >> determining which timeline to start streaming from, which means
 >> that if there are two timeline changes that are not separated by a
 >> WAL segment switch, it will fail to operate due to attempting to
 >> start from a too-old timeline (for which xlogs are not available
 >> on the server).

 Heikki> There's nothing we can do with .history files here. The point
 Heikki> is to find out how far we have already received WAL, and the
 Heikki> presence of a .history file doesn't tell you anything about
 Heikki> that.

I was thinking that the presence of a history file might at least be
useful for setting a bound on what TLI we're going to use; but if a
possible .partial file is taken into account the issue should be moot
anyway I guess.

 Heikki> Attached is a patch to fix both of these issues. I'm too
 Heikki> tired right now to thoroughly test it and commit, so I'll get
 Heikki> back to this tomorrow. Meanwhile, please take a look and let
 Heikki> me know if you can see something wrong.

A quick eyeball check looks ok; I'll see about reproducing the
original scenario with this patch applied.

--
Andrew (irc:RhodiumToad)

On 16.09.2013 22:59, Andrew Gierth wrote:
>>>>>> "Heikki" == Heikki Linnakangas<hlinnakangas@vmware.com>  writes:
>   Heikki>  Attached is a patch to fix both of these issues. I'm too
>   Heikki>  tired right now to thoroughly test it and commit, so I'll get
>   Heikki>  back to this tomorrow. Meanwhile, please take a look and let
>   Heikki>  me know if you can see something wrong.
>
> A quick eyeball check looks ok; I'll see about reproducing the
> original scenario with this patch applied.

Committed, thanks for the report! If you still have a chance to try it
with the original scenario, please do.

- Heikki