Horton

Thread

Commits

  1. pg_buffercache: restore rowtype verification in pg_buffercache_pages()

  2. Modernize and optimize pg_buffercache_pages()

  1. BUG #19508: pg_buffercache_pages() crashes the backend with an incompatible caller-supplied record definition

    The Post Office <noreply@postgresql.org> — 2026-06-04T14:35:10Z

    The following bug has been logged on the website:

Bug reference:      19508
Logged by:          Nikita Kalinin
Email address:      n.kalinin@postgrespro.ru
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:

CREATE EXTENSION pg_buffercache;

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;

postgres=# select version();
                                                   version
-------------------------------------------------------------------------------------------------------------
 PostgreSQL 19beta1 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 16.1.1
20260515 (Red Hat 16.1.1-2), 64-bit
(1 row)

The only difference from the types documented for pg_buffercache_pages() is
that isdirty is declared as text instead of boolean.

git blame points to the following commit:

commit 257c8231bf97a77378f6fedb826b1243f0a41612 (HEAD)
Author: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date:   Tue Apr 7 16:04:48 2026 +0300

    Modernize and optimize pg_buffercache_pages()


Backtrace:

#0  0x00000000004b2565 in VARATT_CAN_MAKE_SHORT (PTR=<optimized out>)
    at ../../../../src/include/varatt.h:419
#1  heap_compute_data_size (tupleDesc=tupleDesc@entry=0x3e5ba110,
    values=values@entry=0x7ffd0dc219c0, isnull=isnull@entry=0x7ffd0dc219b4)
at heaptuple.c:239
#2  0x00000000004b3bff in heap_form_minimal_tuple
(tupleDescriptor=0x3e5ba110,
    values=values@entry=0x7ffd0dc219c0, isnull=isnull@entry=0x7ffd0dc219b4,
extra=extra@entry=0)
    at heaptuple.c:1434
#3  0x0000000000a6fa09 in tuplestore_putvalues (state=0x3e5cc0d8,
tdesc=<optimized out>,
    values=values@entry=0x7ffd0dc219c0, isnull=isnull@entry=0x7ffd0dc219b4)
at tuplestore.c:791
#4  0x00007f180fa0447e in pg_buffercache_pages (fcinfo=<optimized out>)
    at pg_buffercache_pages.c:202
#5  0x00000000006b7e35 in ExecMakeTableFunctionResult (setexpr=0x3e5b9e98,
econtext=0x3e5b9d38,
    argContext=<optimized out>, expectedDesc=0x3e5ba110, randomAccess=false)
at execSRF.c:235
#6  0x00000000006ccc57 in FunctionNext (node=0x3e5b9b28) at
nodeFunctionscan.c:95
#7  0x00000000006daf22 in ExecProcNode (node=0x3e5b9b28)
    at ../../../src/include/executor/executor.h:327
#8  ExecLimit (pstate=0x3e5b97b8) at nodeLimit.c:95
#9  0x00000000006ac39a in ExecProcNode (node=0x3e5b97b8)
    at ../../../src/include/executor/executor.h:327
#10 ExecutePlan (queryDesc=0x3e5d7d80, operation=CMD_SELECT,
sendTuples=true, numberTuples=0,
    direction=<optimized out>, dest=0x3e5e11f8) at execMain.c:1736
#11 standard_ExecutorRun (queryDesc=0x3e5d7d80, direction=<optimized out>,
count=0)
    at execMain.c:377
#12 0x00000000008c61d8 in PortalRunSelect (portal=portal@entry=0x3e52f130,
    forward=forward@entry=true, count=0, count@entry=9223372036854775807,
    dest=dest@entry=0x3e5e11f8) at pquery.c:917
#13 0x00000000008c78be in PortalRun (portal=portal@entry=0x3e52f130,
    count=count@entry=9223372036854775807, isTopLevel=isTopLevel@entry=true,
    dest=dest@entry=0x3e5e11f8, altdest=altdest@entry=0x3e5e11f8,
qc=qc@entry=0x7ffd0dc21e20)
    at pquery.c:761
#14 0x00000000008c3548 in exec_simple_query (
    query_string=0x3e48b800 "SELECT *\nFROM pg_buffercache_pages() AS p(\n
bufferid integer,\n  relfilenode oid,\n  reltablespace oid,\n  reldatabase
oid,\n  relforknumber smallint,\n  relblocknumber bigint,\n  isdirty text,\n
usagecoun"...) at postgres.c:1290
#15 0x00000000008c5021 in PostgresMain (dbname=<optimized out>,
username=<optimized out>)
    at postgres.c:4856
#16 0x00000000008bf01d in BackendMain (startup_data=<optimized out>,
    startup_data_len=<optimized out>) at backend_startup.c:124
#17 0x00000000007fefae in postmaster_child_launch (child_type=<optimized
out>, child_slot=1,
    startup_data=startup_data@entry=0x7ffd0dc22270,
startup_data_len=startup_data_len@entry=24,
    client_sock=client_sock@entry=0x7ffd0dc22290) at launch_backend.c:268
#18 0x00000000008029b6 in BackendStartup (client_sock=0x7ffd0dc22290) at
postmaster.c:3627
#19 ServerLoop () at postmaster.c:1728
#20 0x0000000000804479 in PostmasterMain (argc=argc@entry=3,
argv=argv@entry=0x3e434fe0)
    at postmaster.c:1415
#21 0x00000000004a1c18 in main (argc=3, argv=0x3e434fe0) at main.c:231

For comparison, the same query executed on REL_18_STABLE is rejected with a
regular error:

ERROR:  function return row and query-specified return row do not match
DETAIL:  Returned type boolean at ordinal position 7, but query expects
text.

  2. Re: BUG #19508: pg_buffercache_pages() crashes the backend with an incompatible caller-supplied record definition

    Fujii Masao <masao.fujii@gmail.com> — 2026-06-05T03:18:49Z

    On Fri, Jun 5, 2026 at 12:49 AM PG Bug reporting form
<noreply@postgresql.org> wrote:
>
> The following bug has been logged on the website:
>
> Bug reference:      19508
> Logged by:          Nikita Kalinin
> Email address:      n.kalinin@postgrespro.ru
> 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 <heikki.linnakangas@iki.fi>
> 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

  3. Re: BUG #19508: pg_buffercache_pages() crashes the backend with an incompatible caller-supplied record definition

    Ayush Tiwari <ayushtiwari.slg01@gmail.com> — 2026-06-05T03:42:20Z

    Hi,

On Fri, 5 Jun 2026 at 08:49, Fujii Masao <masao.fujii@gmail.com> wrote:

>
> 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?
>

Thanks for the patch, Fujii-san!

I was looking into the bug last night, and the approach looks right to me.

This still means InitMaterializedSRF() briefly creates the caller-derived
descriptor before rsinfo->setDesc is replaced. That seems acceptable here:
the descriptor lives only in the per-query context, and avoiding a local
copy of InitMaterializedSRF() keeps the fix much smaller and less fragile.

One small nit: build_buffercache_pages_tupledesc() names attribute 8
"usage_count", while the existing pg_buffercache view and the test use
"usagecount". This probably does not affect the tupledesc_match() check,
but I think it would be better to keep the existing spelling for
consistency.

Regards,
Ayush

  4. Re: BUG #19508: pg_buffercache_pages() crashes the backend with an incompatible caller-supplied record definition

    Fujii Masao <masao.fujii@gmail.com> — 2026-06-05T07:58:07Z

    On Fri, Jun 5, 2026 at 12:42 PM Ayush Tiwari
<ayushtiwari.slg01@gmail.com> wrote:
> One small nit: build_buffercache_pages_tupledesc() names attribute 8
> "usage_count", while the existing pg_buffercache view and the test use
> "usagecount". This probably does not affect the tupledesc_match() check,
> but I think it would be better to keep the existing spelling for
> consistency.

Agreed. I've fixed that and attached an updated version of the patch.

Regards,

-- 
Fujii Masao

  5. Re: BUG #19508: pg_buffercache_pages() crashes the backend with an incompatible caller-supplied record definition

    Ayush Tiwari <ayushtiwari.slg01@gmail.com> — 2026-06-05T08:47:58Z

    Hi,

On Fri, 5 Jun, 2026, 13:28 Fujii Masao, <masao.fujii@gmail.com> wrote:

> On Fri, Jun 5, 2026 at 12:42 PM Ayush Tiwari
> <ayushtiwari.slg01@gmail.com> wrote:
> > One small nit: build_buffercache_pages_tupledesc() names attribute 8
> > "usage_count", while the existing pg_buffercache view and the test use
> > "usagecount". This probably does not affect the tupledesc_match() check,
> > but I think it would be better to keep the existing spelling for
> > consistency.
>
> Agreed. I've fixed that and attached an updated version of the patch.
>

LGTM.

Regards,
Ayush

>

  6. Re: BUG #19508: pg_buffercache_pages() crashes the backend with an incompatible caller-supplied record definition

    Ashutosh Sharma <ashu.coek88@gmail.com> — 2026-06-05T15:29:23Z

    Hi Fujii-san,

On Fri, Jun 5, 2026 at 8:49 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Fri, Jun 5, 2026 at 12:49 AM PG Bug reporting form
> <noreply@postgresql.org> wrote:
> >
> > The following bug has been logged on the website:
> >
> > Bug reference:      19508
> > Logged by:          Nikita Kalinin
> > Email address:      n.kalinin@postgrespro.ru
> > 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 <heikki.linnakangas@iki.fi>
> > 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?
>

pg_buffercache_pages uses RETURNS SETOF RECORD whereas other
extensions like pgstattuple define explicit IN/OUT parameters at the
SQL level. Is there a specific reason this pattern was kept, or is it
simply a legacy design that hasn't been modernized? Had we followed
the IN/OUT parameter style, this sort of issue could have been
avoided, no?

--
With Regards,
Ashutosh Sharma.

  7. Re: BUG #19508: pg_buffercache_pages() crashes the backend with an incompatible caller-supplied record definition

    Fujii Masao <masao.fujii@gmail.com> — 2026-06-05T15:38:15Z

    On Sat, Jun 6, 2026 at 12:29 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> pg_buffercache_pages uses RETURNS SETOF RECORD whereas other
> extensions like pgstattuple define explicit IN/OUT parameters at the
> SQL level. Is there a specific reason this pattern was kept, or is it
> simply a legacy design that hasn't been modernized? Had we followed
> the IN/OUT parameter style, this sort of issue could have been
> avoided, no?

Probably yes. But if we do that, we would likely need to bump pg_buffercache
version. I'm not sure that's worthwhile just for this change.

Regards,

-- 
Fujii Masao

  8. Re: BUG #19508: pg_buffercache_pages() crashes the backend with an incompatible caller-supplied record definition

    Fujii Masao <masao.fujii@gmail.com> — 2026-06-08T23:44:50Z

    On Fri, Jun 5, 2026 at 5:48 PM Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote:
>> Agreed. I've fixed that and attached an updated version of the patch.
>
>
> LGTM.

Barring any objections, I will commit the patch.

Regards,

-- 
Fujii Masao

  9. Re: BUG #19508: pg_buffercache_pages() crashes the backend with an incompatible caller-supplied record definition

    Ashutosh Sharma <ashu.coek88@gmail.com> — 2026-06-09T04:38:58Z

    On Fri, Jun 5, 2026 at 9:08 PM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Sat, Jun 6, 2026 at 12:29 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> > pg_buffercache_pages uses RETURNS SETOF RECORD whereas other
> > extensions like pgstattuple define explicit IN/OUT parameters at the
> > SQL level. Is there a specific reason this pattern was kept, or is it
> > simply a legacy design that hasn't been modernized? Had we followed
> > the IN/OUT parameter style, this sort of issue could have been
> > avoided, no?
>
> Probably yes. But if we do that, we would likely need to bump pg_buffercache
> version. I'm not sure that's worthwhile just for this change.
>

Okay, that makes perfect sense, thanks for the confirmation.

--
With Regards,
Ashutosh Sharma.

  10. Re: BUG #19508: pg_buffercache_pages() crashes the backend with an incompatible caller-supplied record definition

    Fujii Masao <masao.fujii@gmail.com> — 2026-06-10T03:34:34Z

    On Tue, Jun 9, 2026 at 8:44 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Fri, Jun 5, 2026 at 5:48 PM Ayush Tiwari <ayushtiwari.slg01@gmail.com> wrote:
> >> Agreed. I've fixed that and attached an updated version of the patch.
> >
> >
> > LGTM.
>
> Barring any objections, I will commit the patch.

I've pushed the patch. Thanks!

Regards,

-- 
Fujii Masao