ALTER PUBLICATION and segmentation fault

Started by Fujii Masaoalmost 9 years ago3 messages

masao.fujii@gmail.com

almost 9 years ago

Hi,

When I logged in PostgreSQL as non-superuser and ran
ALTER PUBLICATION command, I got a segmentation fault.
The code checking the owner of publication might have a bug.

LOG: server process (PID 80356) was terminated by signal 11: Segmentation fault
DETAIL: Failed process was running: ALTER PUBLICATION mypub RENAME TO hoge;

Regards,

--
Fujii Masao

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Masahiko Sawada

sawada.mshk@gmail.com

almost 9 years ago

In reply to: Fujii Masao (#1)

1 attachment(s)

Re: ALTER PUBLICATION and segmentation fault

On Wed, Mar 8, 2017 at 12:45 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

Hi,

When I logged in PostgreSQL as non-superuser and ran
ALTER PUBLICATION command, I got a segmentation fault.
The code checking the owner of publication might have a bug.

=# CREATE ROLE foo NOSUPERUSER LOGIN
=# \c - foo
=> \dRp
List of publications
Name | Owner | Inserts | Updates | Deletes
-------+----------+---------+---------+---------
mypub | postgres | t | t | t
=> ALTER PUBLICATION mypub RENAME TO hoge;

LOG: server process (PID 80356) was terminated by signal 11: Segmentation fault
DETAIL: Failed process was running: ALTER PUBLICATION mypub RENAME TO hoge;

This issue happen even ALTER SUBSCRIPTION. I guess the main cause is
that acl_kind of pg_publication and pg_subscription of ObjectProperty
array are not correct. These values should be set ACL_KIND_PUBLICATION
and ACL_KIND_SUBSCRIPTION instead of -1. Attached small patch fixes
this issue and adds regression test.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

fix_alter_pub_sub_rename.patchapplication/octet-stream; name=fix_alter_pub_sub_rename.patchDownload

diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c
index f2b9601..3a7f049 100644
--- a/src/backend/catalog/objectaddress.c
+++ b/src/backend/catalog/objectaddress.c
@@ -464,7 +464,7 @@ static const ObjectPropertyType ObjectProperty[] =
 		InvalidAttrNumber,
 		Anum_pg_publication_pubowner,
 		InvalidAttrNumber,
-		-1,
+		ACL_KIND_PUBLICATION,
 		true
 	},
 	{
@@ -476,7 +476,7 @@ static const ObjectPropertyType ObjectProperty[] =
 		InvalidAttrNumber,
 		Anum_pg_subscription_subowner,
 		InvalidAttrNumber,
-		-1,
+		ACL_KIND_SUBSCRIPTION,
 		true
 	}
 };
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 6416fbb..134ba81 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -2,6 +2,7 @@
 -- PUBLICATION
 --
 CREATE ROLE regress_publication_user LOGIN SUPERUSER;
+CREATE ROLE dummy_publication_user LOGIN NOSUPERUSER;
 SET SESSION AUTHORIZATION 'regress_publication_user';
 CREATE PUBLICATION testpub_default;
 CREATE PUBLICATION testpib_ins_trunct WITH (nopublish delete, nopublish update);
@@ -148,6 +149,11 @@ DROP TABLE testpub_tbl1;
  t       | t       | t
 (1 row)
 
+-- faile - must be owner of publication
+SET ROLE dummy_publication_user;
+ALTER PUBLICATION testpub_default RENAME TO testpub_dummy;
+ERROR:  must be owner of publication testpub_default
+RESET ROLE;
 ALTER PUBLICATION testpub_default RENAME TO testpub_foo;
 \dRp testpub_foo
                          List of publications
@@ -163,3 +169,4 @@ DROP SCHEMA pub_test CASCADE;
 NOTICE:  drop cascades to table pub_test.testpub_nopk
 RESET SESSION AUTHORIZATION;
 DROP ROLE regress_publication_user;
+DROP ROLE dummy_publication_user;
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index a8a61ee..34cd698 100644
--- a/src/test/regress/expected/subscription.out
+++ b/src/test/regress/expected/subscription.out
@@ -2,6 +2,7 @@
 -- SUBSCRIPTION
 --
 CREATE ROLE regress_subscription_user LOGIN SUPERUSER;
+CREATE ROLE dummy_subscription_user LOGIN NOSUPERUSER;
 SET SESSION AUTHORIZATION 'regress_subscription_user';
 -- fail - no publications
 CREATE SUBSCRIPTION testsub CONNECTION 'foo';
@@ -66,6 +67,11 @@ ALTER SUBSCRIPTION testsub DISABLE;
 (1 row)
 
 COMMIT;
+-- fail - must be owner of subscription
+SET ROLE dummy_subscription_user;
+ALTER SUBSCRIPTION testsub RENAME TO testsub_dummy;
+ERROR:  must be owner of subscription testsub
+RESET ROLE;
 ALTER SUBSCRIPTION testsub RENAME TO testsub_foo;
 \dRs
                          List of subscriptions
@@ -84,3 +90,4 @@ DROP SUBSCRIPTION testsub_foo NODROP SLOT;
 COMMIT;
 RESET SESSION AUTHORIZATION;
 DROP ROLE regress_subscription_user;
+DROP ROLE dummy_subscription_user;
diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 9563ea1..e921330 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -2,6 +2,7 @@
 -- PUBLICATION
 --
 CREATE ROLE regress_publication_user LOGIN SUPERUSER;
+CREATE ROLE dummy_publication_user LOGIN NOSUPERUSER;
 SET SESSION AUTHORIZATION 'regress_publication_user';
 
 CREATE PUBLICATION testpub_default;
@@ -73,6 +74,11 @@ DROP TABLE testpub_tbl1;
 
 \dRp+ testpub_default
 
+-- faile - must be owner of publication
+SET ROLE dummy_publication_user;
+ALTER PUBLICATION testpub_default RENAME TO testpub_dummy;
+RESET ROLE;
+
 ALTER PUBLICATION testpub_default RENAME TO testpub_foo;
 
 \dRp testpub_foo
@@ -85,3 +91,4 @@ DROP SCHEMA pub_test CASCADE;
 
 RESET SESSION AUTHORIZATION;
 DROP ROLE regress_publication_user;
+DROP ROLE dummy_publication_user;
diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql
index 0b6c8a3..f1c6969 100644
--- a/src/test/regress/sql/subscription.sql
+++ b/src/test/regress/sql/subscription.sql
@@ -3,6 +3,7 @@
 --
 
 CREATE ROLE regress_subscription_user LOGIN SUPERUSER;
+CREATE ROLE dummy_subscription_user LOGIN NOSUPERUSER;
 SET SESSION AUTHORIZATION 'regress_subscription_user';
 
 -- fail - no publications
@@ -43,6 +44,11 @@ ALTER SUBSCRIPTION testsub DISABLE;
 
 COMMIT;
 
+-- fail - must be owner of subscription
+SET ROLE dummy_subscription_user;
+ALTER SUBSCRIPTION testsub RENAME TO testsub_dummy;
+RESET ROLE;
+
 ALTER SUBSCRIPTION testsub RENAME TO testsub_foo;
 
 \dRs
@@ -58,3 +64,4 @@ COMMIT;
 
 RESET SESSION AUTHORIZATION;
 DROP ROLE regress_subscription_user;
+DROP ROLE dummy_subscription_user;

Peter Eisentraut

peter.eisentraut@2ndquadrant.com

almost 9 years ago

In reply to: Masahiko Sawada (#2)

Re: ALTER PUBLICATION and segmentation fault

On 3/7/17 11:56, Masahiko Sawada wrote:

This issue happen even ALTER SUBSCRIPTION. I guess the main cause is
that acl_kind of pg_publication and pg_subscription of ObjectProperty
array are not correct. These values should be set ACL_KIND_PUBLICATION
and ACL_KIND_SUBSCRIPTION instead of -1. Attached small patch fixes
this issue and adds regression test.

committed, thanks

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers