On Thu, Jul 30, 2026 at 01:36:00PM -0700, David K wrote: > Thanks Matthias for the review.
Please do not top-post, see this link, in line with the Postgres mailing list style: https://en.wikipedia.org/wiki/Posting_style#Bottom-posting Yep, we are old-school around here. :p > Patch is attached with the following changes: > - keep XLogRecordMaxSize checks + Assert on reclength > - use add_size/mul_size instead of a bare size_t cast > - TYPEALIGN-equivalent roundup without an extra page when already aligned > - comment on the new oversized-length check I doubt that we want a full-fledged test module for this purpose integrated in the end result, even if it proves your point. The overall value does not seem to pile up from my perspective, just for a bunch of edge-case artistic checks to validate the API. + /* + * Cap xl_tot_len before contrecord reassembly so we never allocate or + * copy based on a garbage length from a recycled page. + */ + if (total_len > XLogRecordMaxSize) + { + report_invalid_record(state, + "invalid record length at %X/%08X: expected at most %u, got %u", + LSN_FORMAT_ARGS(RecPtr), + XLogRecordMaxSize, total_len); + goto err; + } Saying that. Andres has reminded me not so long ago that we enforce this rule on the WAL insert side but we don't do so on the xlogreader side. Now, what's the point in having the same check two times? The proposed patch does it once we have read the first bytes of the record for the total record length, and a second time when validating the record header. It feels to me that we should just do it once. Or, wait, you have done it this way to map with total_len? In which case I can buy it.. It seems to me that this should be split as a patch of its own. -- Michael
signature.asc
Description: PGP signature
