From: Tom Lane <tgl@sss.pgh.pa.us>
To: PetSerAl <petseral@gmail.com>
cc: pgsql-bugs@lists.postgresql.org
Subject: Re: Server crash when selecting from pg_cursors
In-reply-to: 
 <CAKygsHTBXLXjwV43kpZa+Cs+XTiaeeJiZdL4cPBm9f4MTdw7wg@mail.gmail.com>
References: 
 <CAKygsHTBXLXjwV43kpZa+Cs+XTiaeeJiZdL4cPBm9f4MTdw7wg@mail.gmail.com>
Comments: In-reply-to PetSerAl <petseral@gmail.com>
	message dated "Sun, 06 Oct 2024 19:15:10 +0300"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <1375361.1728238046.0@sss.pgh.pa.us>
Date: Sun, 06 Oct 2024 14:08:25 -0400
Message-ID: <1375463.1728238105@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1375463.1728238105%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1375361.1728238046.1@sss.pgh.pa.us>

PetSerAl <petseral@gmail.com> writes:
> postgres=# CREATE TABLE t (a integer, b integer);
> CREATE TABLE
> postgres=# CREATE FUNCTION f() RETURNS integer
> postgres-# STABLE STRICT LANGUAGE plpgsql
> postgres-# AS $$
> postgres$# BEGIN
> postgres$#      PERFORM FROM pg_cursors;
> postgres$#      RETURN null;
> postgres$# END
> postgres$# $$;
> CREATE FUNCTION
> postgres=# DO $$
> postgres$# DECLARE
> postgres$#      a integer;
> postgres$# BEGIN
> postgres$#      FOR a IN SELECT t.a FROM t WHERE t.b = f() LOOP
> postgres$#      END LOOP;
> postgres$# END
> postgres$# $$;
> server closed the connection unexpectedly

Huh, nice one.  It's not new in v17 though --- it seems quite ancient.
The proximate cause is that pg_cursor() is assuming portal->sourceText
can't be NULL:

        values[1] = CStringGetTextDatum(portal->sourceText);

which is not unreasonable considering that portal.h quoth

    const char *sourceText;        /* text of query (as of 8.4, never NULL) */

But there's a problem: we may be examining a portal that has been
added to the portal hashtable, but for which PortalDefineQuery
hasn't been called yet.

There are a few ways we might try to deal with this, but I think
the most reasonable one is to make pg_cursor() ignore portals
that haven't been defined yet, as attached.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="harden-pg_cursor-against-null-sourceText.patch";
	charset="us-ascii"
Content-ID: <1375361.1728238046.2@sss.pgh.pa.us>
Content-Description: harden-pg_cursor-against-null-sourceText.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/p=
ortalmem.c
index 4a24613537..93137820ac 100644
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -1150,6 +1150,9 @@ pg_cursor(PG_FUNCTION_ARGS)
 		/* report only "visible" entries */
 		if (!portal->visible)
 			continue;
+		/* also ignore it if PortalDefineQuery hasn't been called yet */
+		if (!portal->sourceText)
+			continue;
 =

 		values[0] =3D CStringGetTextDatum(portal->name);
 		values[1] =3D CStringGetTextDatum(portal->sourceText);

------- =_aaaaaaaaaa0--