public inbox for [email protected]
help / color / mirror / Atom feedFrom: Fujii Masao <[email protected]>
To: [email protected]
To: [email protected]
Subject: Re: BUG #19508: pg_buffercache_pages() crashes the backend with an incompatible caller-supplied record definition
Date: Fri, 5 Jun 2026 12:18:49 +0900
Message-ID: <CAHGQGwGaC8RNeVP3A4_R1OcHJ-DrXgr2HH2UEYrt-Ueu516Gng@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
On Fri, Jun 5, 2026 at 12:49 AM PG Bug reporting form
<[email protected]> wrote:
>
> The following bug has been logged on the website:
>
> Bug reference: 19508
> Logged by: Nikita Kalinin
> Email address: [email protected]
> PostgreSQL version: 19beta1
> Operating system: Fedora 44
> Description:
>
> Hello,
>
> It appears that pg_buffercache_pages() trusts a caller-supplied record
> descriptor without verifying that the declared column types match the actual
> values returned by the function.
>
> The crash is reproducible on the current master branch with a fresh cluster
> after installing the extension:
Thanks for the report! I could reproduce this as well.
> git blame points to the following commit:
>
> commit 257c8231bf97a77378f6fedb826b1243f0a41612 (HEAD)
> Author: Heikki Linnakangas <[email protected]>
> Date: Tue Apr 7 16:04:48 2026 +0300
>
> Modernize and optimize pg_buffercache_pages()
Commit 257c8231bf9 changed pg_buffercache_pages() to materialize rows directly
into a tuplestore. As a result, the function started using the caller-supplied
RECORD descriptor as rsinfo->setDesc, so a mismatched column definition list
could cause tuplestore_putvalues() to interpret returned Datums with incorrect
types.
Before that change, pg_buffercache_pages() exposed its actual tuple descriptor
to the executor, allowing the executor's existing rowtype checks to reject
incompatible definitions with a normal error.
The attached patch restores that behavior while keeping the materialized-SRF
implementation. Thoughts?
Regards,
--
Fujii Masao
Attachments:
[application/octet-stream] v1-0001-pg_buffercache-restore-rowtype-verification-in-pg.patch (5.4K, 2-v1-0001-pg_buffercache-restore-rowtype-verification-in-pg.patch)
download | inline diff:
From 55320441a6862b4ee17b8af81be17a2783c021e8 Mon Sep 17 00:00:00 2001
From: "masao.fujii" <[email protected]’s-MacBook-Pro>
Date: Fri, 5 Jun 2026 10:37:12 +0900
Subject: [PATCH v1] pg_buffercache: restore rowtype verification in
pg_buffercache_pages()
Commit 257c8231bf9 changed pg_buffercache_pages() to materialize its output
directly into a tuplestore. As a result, the function ended up trusting
a caller-supplied RECORD descriptors. That could lead to crashes
if the supplied row definition did not match the actual returned values,
for example by passing bool Datums to tuplestore_putvalues() with
an incompatible descriptor.
Fix this by constructing the correct tuple descriptor for
pg_buffercache_pages() and assigning it to
rsinfo->setDesc after InitMaterializedSRF(). This restores the executor's
tupledesc_match() verification, so incompatible caller-supplied
row definitions are rejected with an error, as before commit 257c8231bf9.
---
.../expected/pg_buffercache.out | 8 +++
contrib/pg_buffercache/pg_buffercache_pages.c | 51 +++++++++++++++++++
contrib/pg_buffercache/sql/pg_buffercache.sql | 6 +++
3 files changed, 65 insertions(+)
diff --git a/contrib/pg_buffercache/expected/pg_buffercache.out b/contrib/pg_buffercache/expected/pg_buffercache.out
index 886dea770f6..c52a8491ff9 100644
--- a/contrib/pg_buffercache/expected/pg_buffercache.out
+++ b/contrib/pg_buffercache/expected/pg_buffercache.out
@@ -73,6 +73,14 @@ SELECT count(*) > 0 FROM pg_buffercache_usage_counts();
t
(1 row)
+SELECT *
+FROM pg_buffercache_pages() AS p
+ (bufferid integer, relfilenode oid, reltablespace oid, reldatabase oid,
+ relforknumber smallint, relblocknumber bigint, isdirty text,
+ usagecount smallint)
+LIMIT 1;
+ERROR: function return row and query-specified return row do not match
+DETAIL: Returned type boolean at ordinal position 7, but query expects text.
RESET role;
------
---- Test pg_buffercache_evict* and pg_buffercache_mark_dirty* functions
diff --git a/contrib/pg_buffercache/pg_buffercache_pages.c b/contrib/pg_buffercache/pg_buffercache_pages.c
index bf2e6c97220..34c9b939313 100644
--- a/contrib/pg_buffercache/pg_buffercache_pages.c
+++ b/contrib/pg_buffercache/pg_buffercache_pages.c
@@ -59,6 +59,8 @@ typedef struct
BufferCacheOsPagesRec *record;
} BufferCacheOsPagesContext;
+static TupleDesc build_buffercache_pages_tupledesc(int natts);
+
/*
* Function returning data from the shared buffer cache - buffer number,
@@ -86,6 +88,8 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
{
ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
TupleDesc expected_tupledesc;
+ TupleDesc actual_tupledesc;
+ MemoryContext oldcontext;
int i;
/*
@@ -105,6 +109,21 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
InitMaterializedSRF(fcinfo, 0);
+ oldcontext = MemoryContextSwitchTo(rsinfo->econtext->ecxt_per_query_memory);
+ actual_tupledesc = build_buffercache_pages_tupledesc(expected_tupledesc->natts);
+ MemoryContextSwitchTo(oldcontext);
+
+ /*
+ * Override the caller-supplied descriptor with the tuple descriptor that
+ * matches the values we actually return, so executor-side
+ * tupledesc_match() can verify the caller's row definition.
+ *
+ * Do not free the previous rsinfo->setDesc here: for RECORD results it
+ * can alias rsinfo->expectedDesc, which the executor still needs to
+ * reference.
+ */
+ rsinfo->setDesc = actual_tupledesc;
+
/*
* Scan through all the buffers, adding one row for each of the buffers to
* the tuplestore.
@@ -205,6 +224,38 @@ pg_buffercache_pages(PG_FUNCTION_ARGS)
return (Datum) 0;
}
+static TupleDesc
+build_buffercache_pages_tupledesc(int natts)
+{
+ TupleDesc tupledesc;
+
+ tupledesc = CreateTemplateTupleDesc(natts);
+ TupleDescInitEntry(tupledesc, (AttrNumber) 1, "bufferid",
+ INT4OID, -1, 0);
+ TupleDescInitEntry(tupledesc, (AttrNumber) 2, "relfilenode",
+ OIDOID, -1, 0);
+ TupleDescInitEntry(tupledesc, (AttrNumber) 3, "reltablespace",
+ OIDOID, -1, 0);
+ TupleDescInitEntry(tupledesc, (AttrNumber) 4, "reldatabase",
+ OIDOID, -1, 0);
+ TupleDescInitEntry(tupledesc, (AttrNumber) 5, "relforknumber",
+ INT2OID, -1, 0);
+ TupleDescInitEntry(tupledesc, (AttrNumber) 6, "relblocknumber",
+ INT8OID, -1, 0);
+ TupleDescInitEntry(tupledesc, (AttrNumber) 7, "isdirty",
+ BOOLOID, -1, 0);
+ TupleDescInitEntry(tupledesc, (AttrNumber) 8, "usage_count",
+ INT2OID, -1, 0);
+
+ if (natts == NUM_BUFFERCACHE_PAGES_ELEM)
+ TupleDescInitEntry(tupledesc, (AttrNumber) 9, "pinning_backends",
+ INT4OID, -1, 0);
+
+ TupleDescFinalize(tupledesc);
+
+ return BlessTupleDesc(tupledesc);
+}
+
/*
* Inquire about OS pages mappings for shared buffers, with NUMA information,
* optionally.
diff --git a/contrib/pg_buffercache/sql/pg_buffercache.sql b/contrib/pg_buffercache/sql/pg_buffercache.sql
index 127d604905c..be89b5f5a3a 100644
--- a/contrib/pg_buffercache/sql/pg_buffercache.sql
+++ b/contrib/pg_buffercache/sql/pg_buffercache.sql
@@ -34,6 +34,12 @@ SELECT count(*) > 0 FROM pg_buffercache;
SELECT count(*) > 0 FROM pg_buffercache_os_pages;
SELECT buffers_used + buffers_unused > 0 FROM pg_buffercache_summary();
SELECT count(*) > 0 FROM pg_buffercache_usage_counts();
+SELECT *
+FROM pg_buffercache_pages() AS p
+ (bufferid integer, relfilenode oid, reltablespace oid, reldatabase oid,
+ relforknumber smallint, relblocknumber bigint, isdirty text,
+ usagecount smallint)
+LIMIT 1;
RESET role;
--
2.53.0
view thread (10+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: BUG #19508: pg_buffercache_pages() crashes the backend with an incompatible caller-supplied record definition
In-Reply-To: <CAHGQGwGaC8RNeVP3A4_R1OcHJ-DrXgr2HH2UEYrt-Ueu516Gng@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox