Re: Fix uninitialized xl_running_xacts padding

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Alexander Kuzmenkov <akuzmenkov(at)tigerdata(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Anthonin Bonnefoy <anthonin(dot)bonnefoy(at)datadoghq(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix uninitialized xl_running_xacts padding
Date: 2026-03-12 18:49:46
Message-ID: 70674869-2829-4b06-ab93-2f82ea51578c@iki.fi
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 12/03/2026 20:23, Alexander Kuzmenkov wrote:
> The functions in the "0003" patch haven't surfaced in my "make
> installcheck-parallel" runs with Valgrind, or the "make check" with
> MemorySanitizer. However, I could hit most of them with some fuzzing.
> The only exception was `xl_hash_vacuum_one_page` but that's probably
> also triggerable.

Cool. It would be nice to have test coverage for every WAL record type.
Could you add tests to the test suite to hit those cases?

> I noticed that we also use `sizeof` in some WAL functions, so probably
> the tail padding can also be written to WAL? For example, consider this:
> (gdb) ptype/o gistxlogPageSplit
> type = struct gistxlogPageSplit {
> /*      0      |       4 */    BlockNumber origrlink;
> /* XXX  4-byte hole      */
> /*      8      |       8 */    GistNSN orignsn;
> /*     16      |       1 */    _Bool origleaf;
> /* XXX  1-byte hole      */
> /*     18      |       2 */    uint16 npage;
> /*     20      |       1 */    _Bool markfollowright;
> /* XXX  3-byte padding   */
>
>                                /* total size (bytes):   24 */
>                              }
>
> And then we do  XLogRegisterData((char *) &xlrec,
> sizeof(gistxlogPageSplit));

Yep.

> In general, I'm wondering what our approach to this should be. Several
> potential improvements were mentioned, but I think for now we could
> focus on removing the Valgrind suppression. This is a meaningful
> improvement that uses the existing test tools.

+1. I think it's a good goal that no uninitialized bytes reach the WAL.
It's not a security issue or anything, but just seems like good hygiene.

> Do we want to defensively zero-initialize every case that seems to
> be potentially affected, i.e. written to WAL and has holes/tail
> padding? That sounds cheap and simple and probably even
> backportable. In the "0001" patch, there are several cases where no
> padding goes into WAL, I can remove these. For example, the use of
> xl_brin_createidx in brinbuild() does not have this problem.
Sounds good to me.

- Heikki

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2026-03-12 18:56:18 Re: Better shared data structure management and resizable shared data structures
Previous Message Marcos Pegoraro 2026-03-12 18:47:01 Re: <productname> on SGML files is used for what ?