Feedback-ID: i0fe9450f:Fastmail
Date: Thu, 1 Oct 2026 08:53:40 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Grigorev Jurij <ju.grigorev@ftdata.ru>
Cc: Rahul Yadav <rahul@rhyadav.com>,
	"pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Subject: Re: BUG #19599: RestoreBlockImage: the decode cross-checks never
 bound hole_offset + hole_length against BLCKSZ
Message-ID: <ar2hA_b1Be7VsZdP@paquier.xyz>
References: <cc0a5886d2a74f99ae58c1647ea5ed8b@localhost.localdomain>
 <CAJJjRReEjgkbm_J9avW=z+eqz6tjyjJRfBmyQvfSx04td-OEvQ@mail.gmail.com>
 <cd00a0536c674fac9e5dd9f4e27ae868@localhost.localdomain>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="mNNfTG3JkWehe1tI"
Content-Disposition: inline
In-Reply-To: <cd00a0536c674fac9e5dd9f4e27ae868@localhost.localdomain>
Archived-At: 
 <https://www.postgresql.org/message-id/ar2hA_b1Be7VsZdP%40paquier.xyz>
Precedence: bulk

--mNNfTG3JkWehe1tI
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Sep 29, 2026 at 08:37:02AM +0000, Grigorev Jurij wrote:
> Thank you very much for the thorough review and testing -- the
> crash-recovery matrix (pglz/lz4/zstd/off + consistency checking) and
> especially the crafted-record repro with pg_waldump --save-fullpage
> crashing without the patch are super convincing. And thanks for
> confirming the back-patch safety argument.
>=20
> v2 attached, addressing all your points:

XLogRecordAssemble() in xloginsert.c enforces a size policy already
when a page image needs to be included in a record (REGBUF_STANDARD
case, for both the "lower" and "upper" cases).  The argument of a
corrupted record does not stand, a CRC32 check would complain before
we ever reach this path.  The hand-made record record argument is also
something I have a hard time to buy, because WAL data is trusted.

So, I don't understand what this patch buys us at all, except more
complexity in the replay path.

There may be an argument for the xlogreader facility, but this relies
on the premise that incorrect WAL records are a thing out there.
Again here comes the CRC check in the record header and the WAL
insertion enforcing already some bounds.  This feels like test bloat
to me.
--
Michael

--mNNfTG3JkWehe1tI
Content-Type: application/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmq9oQQACgkQnvQgOdby
QH3S1hAAj/+YhVI2sxJmLuo7rD5w43r6MeQxeKSsKvPxM3cydxxivreN7o9tQz8G
Q2xBe+6lc+47uDkvaFHPU9XJalo0rd/1FeFg5p6stE3ohXLchr0WoKgrkzrG+c8/
ApDHhEOSdnl7r7VAA+EY83dod5pfVffGIFG/bY8wi6aOeNmpNB0cY8H76CwfaSWy
+CuDbf8Y1hiZbiY876oRDCHWOt31mkehbHkwWrgzDwBuKXGBOB/4UoROECzTGvzV
EiN4oTJpL3hnNgrPXC6kihiQcwR06OlAugUsuqzl7NHAXyRmuh84We+lbyZv6G5O
xLyqDHBj7TqavZPWS2ivHPKG12OiNkCthP9LeFb4JVihudGR7IjzBjeRHOhYXWyM
DRXkPSnl3nzu6z0froEKnDkiQ1GlgZ/090h6skjEhNF4G9z5VAAKxi+F4r2k+OSE
Zs1GzdZaoIEUylV9PQJvrvhrJmaBayN99Eqk1IZO0rzyuM/JYQfgcnWDkjw50+xp
VVvFd39wL998EhZHXIbnB9U6QU9vzJRXvlzLidLWpAg0dtwn6n8zmBYHiw4NRfj+
fp0og55Gx6h8neKDvdUWi17OSaxS+fHskK4nqiwsccj1hFOV+dwQPk6aZ+nFJZA0
btRBgd2Jk4eD2wWZ0DZ3LGKjTePxPdWIEeqo5a9Rt1msNNuk8Yk=
=X0TE
-----END PGP SIGNATURE-----

--mNNfTG3JkWehe1tI--