Re: Is this a problem in GenericXLogFinish()?

Alexander Law <exclusion@gmail.com>

From: Alexander Lakhin <exclusion@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
Cc: Michael Paquier <michael@paquier.xyz>, Robert Haas <robertmhaas@gmail.com>, Jeff Davis <pgsql@j-davis.com>, Heikki Linnakangas <hlinnaka@iki.fi>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2023-11-29T15:00:01Z
Lists: pgsql-hackers
Hello,

13.11.2023 20:21, Robert Haas wrote:
> On Mon, Nov 13, 2023 at 12:47 AM Hayato Kuroda (Fujitsu)
> <kuroda.hayato@fujitsu.com> wrote:
>> Moved.
> I see that this patch was committed, but I'm not very convinced that
> the approach is correct. The comment says this:
>

I've discovered that that patch introduced a code path leading to an
uninitialized memory access.
With the following addition to hash_index.sql:
  -- Fill overflow pages by "dead" tuples.
  BEGIN;
  INSERT INTO hash_cleanup_heap SELECT 1 FROM generate_series(1, 1000) as i;
+INSERT INTO hash_cleanup_heap SELECT 1 FROM generate_series(1, 1000) as i;
  ROLLBACK;
+INSERT INTO hash_cleanup_heap SELECT 1 FROM generate_series(1, 1000) as i;

make check -C src/test/recovery/ PROVE_TESTS="t/027*"
when executed under Valgrind, triggers:
==00:00:02:30.285 97744== Conditional jump or move depends on uninitialised value(s)
==00:00:02:30.285 97744==    at 0x227585: BufferIsValid (bufmgr.h:303)
==00:00:02:30.285 97744==    by 0x227585: hash_xlog_squeeze_page (hash_xlog.c:781)
==00:00:02:30.285 97744==    by 0x228133: hash_redo (hash_xlog.c:1083)
==00:00:02:30.285 97744==    by 0x2C2801: ApplyWalRecord (xlogrecovery.c:1943)
==00:00:02:30.285 97744==    by 0x2C5C52: PerformWalRecovery (xlogrecovery.c:1774)
==00:00:02:30.285 97744==    by 0x2B63A1: StartupXLOG (xlog.c:5559)
==00:00:02:30.285 97744==    by 0x558165: StartupProcessMain (startup.c:282)
==00:00:02:30.285 97744==    by 0x54DFE8: AuxiliaryProcessMain (auxprocess.c:141)
==00:00:02:30.285 97744==    by 0x5546B0: StartChildProcess (postmaster.c:5331)
==00:00:02:30.285 97744==    by 0x557A53: PostmasterMain (postmaster.c:1458)
==00:00:02:30.285 97744==    by 0x4720C2: main (main.c:198)
==00:00:02:30.285 97744==
(in 027_stream_regress_standby_1.log)

That is, when line
https://coverage.postgresql.org/src/backend/access/hash/hash_xlog.c.gcov.html#661
is reached, writebuf stays uninitialized.

Best regards,
Alexander



Commits

  1. Fix inconsistency with replay of hash squeeze record for clean buffers

  2. Set LSN for wbuf in _hash_freeovflpage() iff wbuf is modified.

  3. Fix an uninitialized access in hash_xlog_squeeze_page().

  4. Use REGBUF_NO_CHANGE at one more place in the hash index.

  5. Assert that buffers are marked dirty before XLogRegisterBuffer().

  6. Fix bug in GenericXLogFinish().

  7. Improve hash index bucket split behavior.