Date: Mon, 18 Feb 2019 17:27:43 -0300
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: crash in pg_identify_object_as_address
Message-ID: <20190218202743.GA12392@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="AqsLC8rIMeq19msA"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk


--AqsLC8rIMeq19msA
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Hello

I just came across a crash while debugging some corrupted system
catalogs; pg_identify_object_as_address fails to cope with some NULL
input, causing a crash.  Attached patch fixes it.  Naturally, the output
array will contain NULL members in the output, but that's better than
crashing ...

(The first hunk is purely speculative; I haven't seen anything that
requires that.  The actual fix is in the other hunks.  But seems better
to be defensive.)

The crash can be reproduced thusly

create function f() returns int language plpgsql as $$ begin return 1; end; $$;
update pg_proc set pronamespace = 9999 where proname = 'f' returning oid \gset
select * from pg_identify_object_as_address('pg_proc'::regclass, :oid, 0);

After the patch, the last line returns:

   type   | object_names | object_args 
----------+--------------+-------------
 function | {NULL,f}     | {}

where the NULL obviously corresponds to the bogus pg_namespace OID being
referenced.

The patch is on 9.6.  I checked 10 and it applies fine there.  My
intention is to apply to all branches since 9.5.

-- 
Álvaro Herrera                PostgreSQL Expert, https://www.2ndQuadrant.com/

--AqsLC8rIMeq19msA
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="pg_identify_object_as_address-crash.patch"

diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index bd55dfd08e3..479087592da 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -3576,7 +3576,10 @@ pg_identify_object_as_address(PG_FUNCTION_ARGS)
 	pfree(identity);
 
 	/* object_names */
-	values[1] = PointerGetDatum(strlist_to_textarray(names));
+	if (names != NIL)
+		values[1] = PointerGetDatum(strlist_to_textarray(names));
+	else
+		values[1] = PointerGetDatum(construct_empty_array(TEXTOID));
 	nulls[1] = false;
 
 	/* object_args */
@@ -4803,10 +4806,12 @@ strlist_to_textarray(List *list)
 {
 	ArrayType  *arr;
 	Datum	   *datums;
+	bool	   *nulls;
 	int			j = 0;
 	ListCell   *cell;
 	MemoryContext memcxt;
 	MemoryContext oldcxt;
+	int			one = 1;
 
 	memcxt = AllocSetContextCreate(CurrentMemoryContext,
 								   "strlist to array",
@@ -4814,17 +4819,24 @@ strlist_to_textarray(List *list)
 	oldcxt = MemoryContextSwitchTo(memcxt);
 
 	datums = palloc(sizeof(text *) * list_length(list));
+	nulls = palloc(sizeof(bool) * list_length(list));
 	foreach(cell, list)
 	{
 		char	   *name = lfirst(cell);
 
-		datums[j++] = CStringGetTextDatum(name);
+		if (name)
+		{
+			nulls[j] = false;
+			datums[j++] = CStringGetTextDatum(name);
+		}
+		else
+			nulls[j++] = true;
 	}
 
 	MemoryContextSwitchTo(oldcxt);
 
-	arr = construct_array(datums, list_length(list),
-						  TEXTOID, -1, false, 'i');
+	arr = construct_md_array(datums, nulls, 1, &j,
+							 &one, TEXTOID, -1, false, 'i');
 	MemoryContextDelete(memcxt);
 
 	return arr;

--AqsLC8rIMeq19msA--