Re: [PATCH] Fix use-after-free after failed pg_checksum_init

> On 6 Nov 2026, at 03:11, Grigorev Jurij <ju(dot)grigorev(at)ftdata(dot)ru> wrote:

> Attached is v3, rebased on current master.

Thanks for the updated patch. This looks mostly ready, I just have a few comments.

if (checksum_length != 0)
pg_checksum_init(&checksum_ctx, CHECKSUM_TYPE_NONE);
- else
- pg_checksum_init(&checksum_ctx, checksum_type);
+ else if (pg_checksum_init(&checksum_ctx, checksum_type) < 0)
+ pg_fatal("could not initialize checksum of file \"%s\"",
+ ofullpath);

Nitpick, the conditionals in if and else if are unrelated, I would prefer this
to be broken up to improve readability something like the below. Also, an
assertion added to the TYPE_NONE case would be good.

if (checksum_length != 0)
{
ret = pg_checksum_init(&checksum_ctx, CHECKSUM_TYPE_NONE);
Assert(ret == 0);
}
else
{
ret = pg_checksum_init(&checksum_ctx, checksum_type);
if (ret < 0)
pg_fatal("could not initialize checksum of file \"%s\"", ofullpath);
}

This sketch might trigger a set-never-used compiler warning so it might not be
practical.

+test_checksum_helper$(X): $(TEST_OBJS) | submake-libpgport submake-libpgfeutils
+ $(CC) $(CFLAGS) $^ $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $(at)$(X)

The (X) extension should not be in the rule name.

case CHECKSUM_TYPE_CRC32C:
FIN_CRC32C(context->raw_context.c_crc32c);
retval = sizeof(pg_crc32c);
memcpy(output, &context->raw_context.c_crc32c, retval);
- break;
+ return retval;

Not the fault of this patch, and mostly a nit-pick, but I think we should clear
.c_crc32c after the memcpy and set it to zero. This would make it consistent
with the sha2 types which with this sets the raw_context member to NULL (for
different reasons of course). That would be a separate patch though.

+command_exit_is(
+ ['test_checksum_helper'],
+ 0,
+ 'checksum contexts are cleaned up after cryptohash failures');

To make the test output easier to interpret and more in line with other test,
the helper application should be called once per test case.

--
Daniel Gustafsson