BUG #19515: Creating a faulty BRIN operator class can cause the server to crash when used.

Started by PG Bug reporting form14 days ago2 messagesbugs
Jump to latest
#1PG Bug reporting form
noreply@postgresql.org

The following bug has been logged on the website:

Bug reference: 19515
Logged by: ji xiaohang
Email address: 1165125080@qq.com
PostgreSQL version: 18.4
Operating system: centos8
Description:

Creating a BRIN operator class with a support function that has a wrong
signature (e.g. accepting text instead of internal) succeeds at DDL time,
but crashes the server when a BRIN index using that opclass is created.

The root cause is that assignProcTypes() in opclasscmds.c validates
support function signatures for btree (amcanorder) and hash (amcanhash)
at CREATE OPERATOR CLASS / ALTER OPERATOR FAMILY time, but has no
validation branch for BRIN (amsummarizing). This allows registering
functions with incorrect signatures that then cause memory corruption
when BRIN calls them via fmgr, passing arguments that don't match what
the function expects.

Steps to reproduce:

1. Create a table with a text column:

CREATE TABLE t_sva (sva text);

2. Create a function with the wrong signature (text/text instead of
internal):

CREATE FUNCTION si_same(text, text) RETURNS boolean
AS $$ SELECT $1 = $2 $$ LANGUAGE sql;

3. Create a BRIN operator class using this wrong-signature function
as support function 1 (OPCINFO):

CREATE OPERATOR CLASS sva_special_ops FOR TYPE text USING brin AS
OPERATOR 1 =, FUNCTION 1 si_same(text, text);

This succeeds — it should have been rejected.

4. Create a BRIN index using the opclass:

CREATE INDEX idx_special1 ON t_sva USING brin(sva sva_special_ops);

The server crashes at this point.

Expected behavior:

Step 3 should fail with an error like:
ERROR: BRIN support function 1 must accept type "internal"

The server should never crash due to a DDL operation with user-defined
functions.

Analysis:

assignProcTypes() has validation for btree and hash support functions
but skips BRIN entirely, falling through to the default type-assignment
logic. BRIN support functions use "internal" argument types, so the
default logic cannot infer correct types and allows the wrong-signature
function through.

At runtime, brin_build_desc() calls the OPCINFO function via fmgr,
passing an Oid as "internal". The wrong-signature SQL function then
interprets the argument as "text", causing memory corruption.

The same gap exists for GiST, GIN, and SP-GiST — they likewise lack
validation branches in assignProcTypes(), so wrong-signature support
functions can be registered for those AMs as well. If the BRIN fix
approach is accepted, similar validation can be added for those access
methods in a follow-up patch.

Proposed fix:

Add an "else if (amroutine->amsummarizing)" validation branch in
assignProcTypes(), after the existing amcanhash block. This branch
would validate the signatures of the 4 mandatory BRIN support functions
(OPCINFO, ADDVALUE, CONSISTENT, UNION) and reject invalid proc numbers.
Optional procs (11-15) would be accepted without signature checking,
consistent with brinvalidate().

Fix the patch as follows:
diff --git a/src/backend/commands/opclasscmds.c
b/src/backend/commands/opclasscmds.c
index a6dd8ea..e5078eb 100644
--- a/src/backend/commands/opclasscmds.c
+++ b/src/backend/commands/opclasscmds.c
@@ -17,6 +17,7 @@

#include <limits.h>

+#include "access/brin_internal.h"
#include "access/genam.h"
#include "access/hash.h"
#include "access/htup_details.h"
@@ -1205,6 +1206,7 @@ assignProcTypes(OpFamilyMember *member, Oid amoid, Oid
typeoid,
{
HeapTuple proctup;
Form_pg_proc procform;
+ IndexAmRoutine *amroutine;

/* Fetch the procedure definition */
proctup = SearchSysCache1(PROCOID,
ObjectIdGetDatum(member->object));
@@ -1212,6 +1214,9 @@ assignProcTypes(OpFamilyMember *member, Oid amoid, Oid
typeoid,
elog(ERROR, "cache lookup failed for function %u",
member->object);
procform = (Form_pg_proc) GETSTRUCT(proctup);

+       /* Fetch the AM routine once, for use in validation below */
+       amroutine = GetIndexAmRoutineByAmId(amoid, false);
+
        /* Check the signature of the opclass options parsing function */
        if (member->number == opclassOptsProcNum)
        {
@@ -1249,7 +1254,7 @@ assignProcTypes(OpFamilyMember *member, Oid amoid, Oid
typeoid,
         * a 1-arg proc returning int4, while proc 2 must be a 2-arg proc
         * returning int8. Otherwise we don't know.
         */
-       else if (GetIndexAmRoutineByAmId(amoid, false)->amcanorder)
+       else if (amroutine->amcanorder)
        {
                if (member->number == BTORDER_PROC)
                {
@@ -1357,7 +1362,7 @@ assignProcTypes(OpFamilyMember *member, Oid amoid, Oid
typeoid,
                                                 errmsg("btree skip support
functions must not be cross-type")));
                }
        }
-       else if (GetIndexAmRoutineByAmId(amoid, false)->amcanhash)
+       else if (amroutine->amcanhash)
        {
                if (member->number == HASHSTANDARD_PROC)
                {
@@ -1390,6 +1395,92 @@ assignProcTypes(OpFamilyMember *member, Oid amoid,
Oid typeoid,
                if (!OidIsValid(member->righttype))
                        member->righttype = procform->proargtypes.values[0];
        }
+       else if (amroutine->amsummarizing)
+       {
+               /*
+                * For BRIN, support functions use "internal" argument
types, so
+                * we can't infer lefttype/righttype from them.  Validate
the
+                * function signatures per BRIN requirements.
+                */
+               if (member->number == BRIN_PROCNUM_OPCINFO)
+               {
+                       if (procform->pronargs != 1 ||
+                               procform->proargtypes.values[0] !=
INTERNALOID)
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("BRIN support
function 1 must accept type \"internal\"")));
+                       if (procform->prorettype != INTERNALOID)
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("BRIN support
function 1 must return type \"internal\"")));
+               }
+               else if (member->number == BRIN_PROCNUM_ADDVALUE)
+               {
+                       if (procform->pronargs != 4)
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("BRIN support
function 2 must have four arguments")));
+                       if (procform->proargtypes.values[0] != INTERNALOID
||
+                               procform->proargtypes.values[1] !=
INTERNALOID ||
+                               procform->proargtypes.values[2] !=
INTERNALOID ||
+                               procform->proargtypes.values[3] !=
INTERNALOID)
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("BRIN support
function 2 must accept type \"internal\"")));
+                       if (procform->prorettype != BOOLOID)
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("BRIN support
function 2 must return boolean")));
+               }
+               else if (member->number == BRIN_PROCNUM_CONSISTENT)
+               {
+                       if (procform->pronargs != 3 && procform->pronargs !=
4)
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("BRIN support
function 3 must have three or four arguments")));
+                       if (procform->proargtypes.values[0] != INTERNALOID
||
+                               procform->proargtypes.values[1] !=
INTERNALOID ||
+                               procform->proargtypes.values[2] !=
INTERNALOID)
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("BRIN support
function 3 must accept type \"internal\"")));
+                       if (procform->pronargs == 4 &&
+                               procform->proargtypes.values[3] != INT4OID)
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("BRIN support
function 3 optional fourth argument must be type \"integer\"")));
+                       if (procform->prorettype != BOOLOID)
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("BRIN support
function 3 must return boolean")));
+               }
+               else if (member->number == BRIN_PROCNUM_UNION)
+               {
+                       if (procform->pronargs != 3)
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("BRIN support
function 4 must have three arguments")));
+                       if (procform->proargtypes.values[0] != INTERNALOID
||
+                               procform->proargtypes.values[1] !=
INTERNALOID ||
+                               procform->proargtypes.values[2] !=
INTERNALOID)
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("BRIN support
function 4 must accept type \"internal\"")));
+                       if (procform->prorettype != BOOLOID)
+                               ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                                errmsg("BRIN support
function 4 must return boolean")));
+               }
+               else if (member->number < BRIN_FIRST_OPTIONAL_PROCNUM ||
+                                member->number >
BRIN_LAST_OPTIONAL_PROCNUM)
+               {
+                       ereport(ERROR,
+
(errcode(ERRCODE_INVALID_OBJECT_DEFINITION),
+                                        errmsg("invalid support function
number %d for BRIN",
+                                                       member->number)));
+               }
+               /* Optional procs (11-15): accept without signature checking
*/
+       }

/*
* The default in CREATE OPERATOR CLASS is to use the class'
opcintype as

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: PG Bug reporting form (#1)
Re: BUG #19515: Creating a faulty BRIN operator class can cause the server to crash when used.

PG Bug reporting form <noreply@postgresql.org> writes:

Creating a BRIN operator class with a support function that has a wrong
signature (e.g. accepting text instead of internal) succeeds at DDL time,
but crashes the server when a BRIN index using that opclass is created.

This is not a Postgres bug. The reason that opclass creation is
restricted to superusers is that the system trusts opclasses to
behave according to spec. We'd check that rather than just assume it
if it were practical to do so, but it's not really (see the halting
problem for starters).

We could close a small fraction of the possible crash causes by
improving assignProcTypes, but that would be a feature improvement
not a bug fix.

regards, tom lane