pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Dmitry Koval <d.koval@postgrespro.ru>
To: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17815: Server crashes on executing gist_page_items() in pageinspect extension
Date: Wed, 1 Mar 2023 12:31:47 +0300
Message-ID: <38ce17fb-37d8-252b-f11d-a8fcec944086@postgrespro.ru> (raw)
In-Reply-To: <17815-fc4a2d3b74705703@postgresql.org>
References: <17815-fc4a2d3b74705703@postgresql.org>

Hi,

Attached fix for "master" branch.

-- 
With best regards,
Dmitry Koval

Postgres Professional: http://postgrespro.com
diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c
index 100697814d..2b252f85bd 100644
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -32,13 +32,14 @@ PG_FUNCTION_INFO_V1(gist_page_items_bytea);
 #define IS_GIST(r) ((r)->rd_rel->relam == GIST_AM_OID)
 
 
+static Page verify_gist_page(bytea *raw_page);
+
 Datum
 gist_page_opaque_info(PG_FUNCTION_ARGS)
 {
 	bytea	   *raw_page = PG_GETARG_BYTEA_P(0);
 	TupleDesc	tupdesc;
 	Page		page;
-	GISTPageOpaque opaq;
 	HeapTuple	resultTuple;
 	Datum		values[4];
 	bool		nulls[4];
@@ -51,35 +52,17 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("must be superuser to use raw page functions")));
 
-	page = get_page_from_raw(raw_page);
+	page = verify_gist_page(raw_page);
 
 	if (PageIsNew(page))
 		PG_RETURN_NULL();
 
-	/* verify the special space has the expected size */
-	if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData)))
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("input page is not a valid %s page", "GiST"),
-				 errdetail("Expected special size %d, got %d.",
-						   (int) MAXALIGN(sizeof(GISTPageOpaqueData)),
-						   (int) PageGetSpecialSize(page))));
-
-	opaq = GistPageGetOpaque(page);
-	if (opaq->gist_page_id != GIST_PAGE_ID)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("input page is not a valid %s page", "GiST"),
-				 errdetail("Expected %08x, got %08x.",
-						   GIST_PAGE_ID,
-						   opaq->gist_page_id)));
-
 	/* Build a tuple descriptor for our result type */
 	if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
 		elog(ERROR, "return type must be a row type");
 
 	/* Convert the flags bitmask to an array of human-readable names */
-	flagbits = opaq->flags;
+	flagbits = GistPageGetOpaque(page)->flags;
 	if (flagbits & F_LEAF)
 		flags[nflags++] = CStringGetTextDatum("leaf");
 	if (flagbits & F_DELETED)
@@ -101,7 +84,7 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
 
 	values[0] = LSNGetDatum(PageGetLSN(page));
 	values[1] = LSNGetDatum(GistPageGetNSN(page));
-	values[2] = Int64GetDatum(opaq->rightlink);
+	values[2] = Int64GetDatum(GistPageGetOpaque(page)->rightlink);
 	values[3] = PointerGetDatum(construct_array_builtin(flags, nflags, TEXTOID));
 
 	/* Build and return the result tuple. */
@@ -116,7 +99,6 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
 	bytea	   *raw_page = PG_GETARG_BYTEA_P(0);
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
 	Page		page;
-	GISTPageOpaque opaq;
 	OffsetNumber offset;
 	OffsetNumber maxoff = InvalidOffsetNumber;
 
@@ -127,29 +109,11 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
 
 	InitMaterializedSRF(fcinfo, 0);
 
-	page = get_page_from_raw(raw_page);
+	page = verify_gist_page(raw_page);
 
 	if (PageIsNew(page))
 		PG_RETURN_NULL();
 
-	/* verify the special space has the expected size */
-	if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData)))
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("input page is not a valid %s page", "GiST"),
-				 errdetail("Expected special size %d, got %d.",
-						   (int) MAXALIGN(sizeof(GISTPageOpaqueData)),
-						   (int) PageGetSpecialSize(page))));
-
-	opaq = GistPageGetOpaque(page);
-	if (opaq->gist_page_id != GIST_PAGE_ID)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("input page is not a valid %s page", "GiST"),
-				 errdetail("Expected %08x, got %08x.",
-						   GIST_PAGE_ID,
-						   opaq->gist_page_id)));
-
 	/* Avoid bogus PageGetMaxOffsetNumber() call with deleted pages */
 	if (GistPageIsDeleted(page))
 		elog(NOTICE, "page is deleted");
@@ -193,6 +157,40 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
 	return (Datum) 0;
 }
 
+/*
+ * Verify that the given bytea contains a GIST page or die in the attempt.
+ * A pointer to the page is returned.
+ */
+static Page
+verify_gist_page(bytea *raw_page)
+{
+	Page		page = get_page_from_raw(raw_page);
+	GISTPageOpaque opaq;
+
+	if (PageIsNew(page))
+		return page;
+
+	/* verify the special space has the expected size */
+	if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData)))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("input page is not a valid %s page", "GiST"),
+				 errdetail("Expected special size %d, got %d.",
+						   (int) MAXALIGN(sizeof(GISTPageOpaqueData)),
+						   (int) PageGetSpecialSize(page))));
+
+	opaq = GistPageGetOpaque(page);
+	if (opaq->gist_page_id != GIST_PAGE_ID)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("input page is not a valid %s page", "GiST"),
+				 errdetail("Expected %08x, got %08x.",
+						   GIST_PAGE_ID,
+						   opaq->gist_page_id)));
+
+	return page;
+}
+
 Datum
 gist_page_items(PG_FUNCTION_ARGS)
 {
@@ -220,7 +218,7 @@ gist_page_items(PG_FUNCTION_ARGS)
 				 errmsg("\"%s\" is not a %s index",
 						RelationGetRelationName(indexRel), "GiST")));
 
-	page = get_page_from_raw(raw_page);
+	page = verify_gist_page(raw_page);
 
 	if (PageIsNew(page))
 	{


Attachments:

  [text/plain] verify_gist_page.diff (5.0K, ../38ce17fb-37d8-252b-f11d-a8fcec944086@postgrespro.ru/2-verify_gist_page.diff)
  download | inline diff:
diff --git a/contrib/pageinspect/gistfuncs.c b/contrib/pageinspect/gistfuncs.c
index 100697814d..2b252f85bd 100644
--- a/contrib/pageinspect/gistfuncs.c
+++ b/contrib/pageinspect/gistfuncs.c
@@ -32,13 +32,14 @@ PG_FUNCTION_INFO_V1(gist_page_items_bytea);
 #define IS_GIST(r) ((r)->rd_rel->relam == GIST_AM_OID)
 
 
+static Page verify_gist_page(bytea *raw_page);
+
 Datum
 gist_page_opaque_info(PG_FUNCTION_ARGS)
 {
 	bytea	   *raw_page = PG_GETARG_BYTEA_P(0);
 	TupleDesc	tupdesc;
 	Page		page;
-	GISTPageOpaque opaq;
 	HeapTuple	resultTuple;
 	Datum		values[4];
 	bool		nulls[4];
@@ -51,35 +52,17 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
 				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
 				 errmsg("must be superuser to use raw page functions")));
 
-	page = get_page_from_raw(raw_page);
+	page = verify_gist_page(raw_page);
 
 	if (PageIsNew(page))
 		PG_RETURN_NULL();
 
-	/* verify the special space has the expected size */
-	if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData)))
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("input page is not a valid %s page", "GiST"),
-				 errdetail("Expected special size %d, got %d.",
-						   (int) MAXALIGN(sizeof(GISTPageOpaqueData)),
-						   (int) PageGetSpecialSize(page))));
-
-	opaq = GistPageGetOpaque(page);
-	if (opaq->gist_page_id != GIST_PAGE_ID)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("input page is not a valid %s page", "GiST"),
-				 errdetail("Expected %08x, got %08x.",
-						   GIST_PAGE_ID,
-						   opaq->gist_page_id)));
-
 	/* Build a tuple descriptor for our result type */
 	if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
 		elog(ERROR, "return type must be a row type");
 
 	/* Convert the flags bitmask to an array of human-readable names */
-	flagbits = opaq->flags;
+	flagbits = GistPageGetOpaque(page)->flags;
 	if (flagbits & F_LEAF)
 		flags[nflags++] = CStringGetTextDatum("leaf");
 	if (flagbits & F_DELETED)
@@ -101,7 +84,7 @@ gist_page_opaque_info(PG_FUNCTION_ARGS)
 
 	values[0] = LSNGetDatum(PageGetLSN(page));
 	values[1] = LSNGetDatum(GistPageGetNSN(page));
-	values[2] = Int64GetDatum(opaq->rightlink);
+	values[2] = Int64GetDatum(GistPageGetOpaque(page)->rightlink);
 	values[3] = PointerGetDatum(construct_array_builtin(flags, nflags, TEXTOID));
 
 	/* Build and return the result tuple. */
@@ -116,7 +99,6 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
 	bytea	   *raw_page = PG_GETARG_BYTEA_P(0);
 	ReturnSetInfo *rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
 	Page		page;
-	GISTPageOpaque opaq;
 	OffsetNumber offset;
 	OffsetNumber maxoff = InvalidOffsetNumber;
 
@@ -127,29 +109,11 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
 
 	InitMaterializedSRF(fcinfo, 0);
 
-	page = get_page_from_raw(raw_page);
+	page = verify_gist_page(raw_page);
 
 	if (PageIsNew(page))
 		PG_RETURN_NULL();
 
-	/* verify the special space has the expected size */
-	if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData)))
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("input page is not a valid %s page", "GiST"),
-				 errdetail("Expected special size %d, got %d.",
-						   (int) MAXALIGN(sizeof(GISTPageOpaqueData)),
-						   (int) PageGetSpecialSize(page))));
-
-	opaq = GistPageGetOpaque(page);
-	if (opaq->gist_page_id != GIST_PAGE_ID)
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("input page is not a valid %s page", "GiST"),
-				 errdetail("Expected %08x, got %08x.",
-						   GIST_PAGE_ID,
-						   opaq->gist_page_id)));
-
 	/* Avoid bogus PageGetMaxOffsetNumber() call with deleted pages */
 	if (GistPageIsDeleted(page))
 		elog(NOTICE, "page is deleted");
@@ -193,6 +157,40 @@ gist_page_items_bytea(PG_FUNCTION_ARGS)
 	return (Datum) 0;
 }
 
+/*
+ * Verify that the given bytea contains a GIST page or die in the attempt.
+ * A pointer to the page is returned.
+ */
+static Page
+verify_gist_page(bytea *raw_page)
+{
+	Page		page = get_page_from_raw(raw_page);
+	GISTPageOpaque opaq;
+
+	if (PageIsNew(page))
+		return page;
+
+	/* verify the special space has the expected size */
+	if (PageGetSpecialSize(page) != MAXALIGN(sizeof(GISTPageOpaqueData)))
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("input page is not a valid %s page", "GiST"),
+				 errdetail("Expected special size %d, got %d.",
+						   (int) MAXALIGN(sizeof(GISTPageOpaqueData)),
+						   (int) PageGetSpecialSize(page))));
+
+	opaq = GistPageGetOpaque(page);
+	if (opaq->gist_page_id != GIST_PAGE_ID)
+		ereport(ERROR,
+				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+				 errmsg("input page is not a valid %s page", "GiST"),
+				 errdetail("Expected %08x, got %08x.",
+						   GIST_PAGE_ID,
+						   opaq->gist_page_id)));
+
+	return page;
+}
+
 Datum
 gist_page_items(PG_FUNCTION_ARGS)
 {
@@ -220,7 +218,7 @@ gist_page_items(PG_FUNCTION_ARGS)
 				 errmsg("\"%s\" is not a %s index",
 						RelationGetRelationName(indexRel), "GiST")));
 
-	page = get_page_from_raw(raw_page);
+	page = verify_gist_page(raw_page);
 
 	if (PageIsNew(page))
 	{


view thread (6+ messages)  latest in thread

Message-ID: <38ce17fb-37d8-252b-f11d-a8fcec944086@postgrespro.ru>
Permalink:  ../38ce17fb-37d8-252b-f11d-a8fcec944086@postgrespro.ru/
Also on:    postgresql.org/message-id/38ce17fb-37d8-252b-f11d-a8fcec944086@postgrespro.ru

 ·  · 

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: pgsql-bugs@postgresql.org
  Cc: d.koval@postgrespro.ru, pgsql-bugs@lists.postgresql.org
  Subject: Re: BUG #17815: Server crashes on executing gist_page_items() in pageinspect extension
  In-Reply-To: <38ce17fb-37d8-252b-f11d-a8fcec944086@postgrespro.ru>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox