| From: | Alexander Kuzmenkov <akuzmenkov(at)tigerdata(dot)com> |
|---|---|
| To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
| 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:23:31 |
| Message-ID: | CALzhyqy27==NkjnXYysF1zJj_K2rX5kncJZ9fEdTRVszTjJisg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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.
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));
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. 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.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Alvaro Herrera | 2026-03-12 18:25:54 | Re: Adding REPACK [concurrently] |
| Previous Message | Robert Treat | 2026-03-12 18:23:27 | Re: <productname> on SGML files is used for what ? |