Re: Avoid core dump in pgstat_read_statsfile()

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Avoid core dump in pgstat_read_statsfile()
Date: 2025-04-24 00:23:58
Message-ID: aAmEngW6uTS9E7wn@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Apr 23, 2025 at 08:01:40AM +0000, Bertrand Drouvot wrote:
> I think that a check on pgstat_get_kind_info() is missing for this scenario, the
> patch adds it. Such a check already exists for PGSTAT_FILE_ENTRY_FIXED and
> for stats entry identified by name on disk, but not for PGSTAT_FILE_ENTRY_HASH.

It is, indeed. This can be reproduced with what's in core, just
disable pgstat_register_inj_fixed() to bypass the check for fixed
entries that would be read first if the module is removed from
shared_preload_libraries in injection_points.c and we are good to go.

I can see that you have copy-pasted the elog from the two other entry
types. This is incomplete here because we are dealing with an entry
in the dshash and we know its full key already: kind, database OID,
object ID. And this can provide more information to help with the
debugging. I have added this information, and applied the result.

> The v18 existing checks mentioned above as well as the new check were missing
> in pre-18 but I don't think it's worth a back-patch as the issue is unlikely to
> occur without custom stats. Adding a v18 open item then.

The check based on the kind ID should be enough, because this ensures
that we'll have the PgStat_KindInfo to rely on. So stable branches
are OK as they are.
--
Michael

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2025-04-24 00:40:48 Re: AIO v2.5
Previous Message Peter Smith 2025-04-23 23:36:57 Re: Logical Replication of sequences