Horton

RE: Is this a problem in GenericXLogFinish()?

Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>

View thread View on postgr.es
From: "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>
To: 'Alexander Lakhin' <exclusion@gmail.com>, Amit Kapila <amit.kapila16@gmail.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-30T03:09:02Z
Lists: pgsql-hackers 
Dear Alexander,

> 
> 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.ht
> ml#661
> is reached, writebuf stays uninitialized.

Good catch, thank you for reporting! I will investigate more about it and post my analysis.

Best Regards,
Hayato Kuroda
FUJITSU LIMITED

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.