Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3864.400.21\))
Subject: Re: [BUG + PATCH] DSA pagemap out-of-bounds in make_new_segment
 odd-sized path
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <aaoIg049JvfqWBXN@paquier.xyz>
Date: Fri, 6 Mar 2026 08:54:35 +0800
Cc: Sami Imseih <samimseih@gmail.com>,
 paul.bunn@icloud.com,
 pgsql-hackers@postgresql.org
Content-Transfer-Encoding: quoted-printable
Message-Id: <BE7DBB95-BD78-446E-8026-68E9140B348B@gmail.com>
References: <044401dcabac$fe432490$fac96db0$@icloud.com>
 <CAA5RZ0vtF+7Oc8HAOVWGmbfWBVTAb=7-u-knU11cw2V6Bgfgog@mail.gmail.com>
 <aaoIg049JvfqWBXN@paquier.xyz>
To: Michael Paquier <michael@paquier.xyz>
Archived-At: 
 <https://www.postgresql.org/message-id/BE7DBB95-BD78-446E-8026-68E9140B348B%40gmail.com>
Precedence: bulk


> On Mar 6, 2026, at 06:49, Michael Paquier <michael@paquier.xyz> wrote:
>=20
> On Thu, Mar 05, 2026 at 04:25:57PM -0600, Sami Imseih wrote:
>> With the above assert in place, the usable_pages of 879 ars you have
>> in test_dsa.c crashes
>> due to the assertion failure.
>=20
> Yep, the assertion looks useful to have in place.
>=20

While debugging the code, I used the same Assert, which helped quickly =
trigger the bug. Without the Assert, even if an OOB happens, it might =
not immediately crash.

But now that the bug has been identified and fixed, and the function is =
self-contained, I=E2=80=99m not sure how much the Assert would still =
help. Unless we are not sure whether the fix really works, otherwise I =
don=E2=80=99t think the Assert is necessary.

However, this is not a strong objection. If we do want to add the =
Assert, I notice the expression MAXALIGN(sizeof(dsa_segment_header)) + =
MAXALIGN(sizeof(FreePageManager)) appears multiple times, and the Assert =
actually uses it as well. We could add a local variable like:
```
size_t metadata_fixed_bytes =3D
               MAXALIGN(sizeof(dsa_segment_header)) + =20
               MAXALIGN(sizeof(FreePageManager));
```

That would help reduce code redundancy.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/