Horton

Re: BUG #17928: Standby fails to decode WAL on termination of primary

Thomas Munro <thomas.munro@gmail.com>

View thread
From: Thomas Munro <thomas.munro@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alexander Lakhin <exclusion@gmail.com>, Sergei Kornilov <sk@zsrv.org>, Noah Misch <noah@leadboat.com>, Tom Lane <tgl@sss.pgh.pa.us>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, pgsql-bugs@lists.postgresql.org, Fujii Masao <masao.fujii@oss.nttdata.com>
Date: 2023-09-23T20:48:42Z
Lists: pgsql-bugs

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Correct assertion and comments about XLogRecordMaxSize.

  2. Fix edge-case for xl_tot_len broken by bae868ca.

  3. Don't use Perl pack('Q') in 039_end_of_wal.pl.

  4. Don't trust unvalidated xl_tot_len.

  5. Make recovery report error message when invalid page header is found.

  6. Add more protections in WAL record APIs against overflows

On Sat, Sep 23, 2023 at 4:44 PM Michael Paquier <michael@paquier.xyz> wrote:
> The stack may point out at a different issue, but perhaps this is a
> matter where we're returning now XLREAD_SUCCESS where previously we
> had XLREAD_FAIL, causing this code to fail thinking that the block was
> valid while it's not?

"grison" has a little more detail --  we see
pg_comp_crc32c_sb8(len=4294636456).  I'm wondering how to reproduce
this, but among the questions that jump out I have: why was it ever OK
that we load record->xl_tot_len into total_len, perform header
validation, determine that total_len < len (= this record is all on
one page, no reassembly loop needed, so now we're in the single-page
branch), then call ReadPageInternal() again, then call
ValidXLogRecord() which internally loads record->xl_tot_len *again*?
ReadPageInternal() might have changed xl_tot_len, no?  That seems to
be a possible pathway to reading past the end of the buffer in the CRC
check, no?

If that value didn't change underneath us, I think we'd need an
explanation for how we finished up in the single-page branch at
xlogreader.c:842 with a large xl_tot_len, which I'm not seeing yet,
though it might take more coffee.  (Possibly supporting the re-read
theory is the fact that it's only happening on a few very slow
computers, though I have no idea why it would only happen on master
[so far at least].)