Re: [HACKERS] pg_upgrade in 9.5 broken for adminpack

[Andreas Seltenreich]

Bruce Momjian writes:

> On Thu, Apr 16, 2015 at 11:29:07PM -0700, Jeff Janes wrote:
>> Of course after sending that it became obvious.  The C function is not 
>> getting
>> called because the SQL function is marked as being strict, yet is called with
>> NULL arguments.
>> 
>> Trivial patch attached to unset strict flag in pg_proc.h.
>> 
>> But  CATALOG_VERSION_NO probably needs another bump as well.
>
> Patch applied and catversion bumped.  Thanks.

Shouldn't there be some validation of arguments now that the function is
no longer marked strict?  Currently, unprivileged users can crash the
server calling binary_upgrade_create_empty_extension with null
arguments.  Found using sqlsmith.

regards,
Andreas


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

Re: [HACKERS] pg_upgrade in 9.5 broken for adminpack

[Tom Lane]

Andreas Seltenreich  writes:
> Shouldn't there be some validation of arguments now that the function is
> no longer marked strict?  Currently, unprivileged users can crash the
> server calling binary_upgrade_create_empty_extension with null
> arguments.  Found using sqlsmith.

Good catch, thanks!  Will fix.

regards, tom lane


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

Re: [HACKERS] pg_upgrade in 9.5 broken for adminpack

[Bruce Momjian]

On Fri, Apr 17, 2015 at 04:11:44PM +0900, Michael Paquier wrote:
 On Fri, Apr 17, 2015 at 3:29 PM, Jeff Janes wrote:
  Of course after sending that it became obvious.  The C function is not
  getting called because the SQL function is marked as being strict, yet is
  called with NULL arguments.
 
  Trivial patch attached to unset strict flag in pg_proc.h.
 
  But  CATALOG_VERSION_NO probably needs another bump as well.
 
 This looks good to me. I have tested your fix and the problem goes
 away. create_empty_extension() was the only function not declared as
 STRICT before 30982be4 (see install_support_functions_in_new_db() in
 contrib/pg_upgrade/function.c).

OK, as I am not the author of that change, I am going to wait a little
while for the author to commit the fix.  I can see it is an easy mistake
to make.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


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

Re: [HACKERS] pg_upgrade in 9.5 broken for adminpack

[Bruce Momjian]

On Thu, Apr 16, 2015 at 11:29:07PM -0700, Jeff Janes wrote:
 Of course after sending that it became obvious.  The C function is not getting
 called because the SQL function is marked as being strict, yet is called with
 NULL arguments.
 
 Trivial patch attached to unset strict flag in pg_proc.h.
 
 But  CATALOG_VERSION_NO probably needs another bump as well.

Patch applied and catversion bumped.  Thanks.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


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

[HACKERS] pg_upgrade in 9.5 broken for adminpack

[Jeff Janes]

pg_upgrade was recently broken for use upgrading from a system with
adminpack installed.

Breaking commit is:

commit 30982be4e5019684e1772dd9170aaa53f5a8e894
Author: Peter Eisentraut pete...@gmx.net

Integrate pg_upgrade_support module into backend


from pg_upgrade_dump_12870.log

pg_restore: creating EXTENSION adminpack
pg_restore: creating COMMENT EXTENSION adminpack
pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 2806; 0 0 COMMENT
EXTENSION adminpack
pg_restore: [archiver (db)] could not execute query: ERROR:  extension
adminpack does not exist
Command was: COMMENT ON EXTENSION adminpack IS 'administrative
functions for PostgreSQL';


I get the same error whether the source database is 9.2.10 or 9.5.HEAD.

Cheers,

Jeff

Re: [HACKERS] pg_upgrade in 9.5 broken for adminpack

[Bruce Momjian]

On Thu, Apr 16, 2015 at 07:33:50PM -0700, Jeff Janes wrote:
 pg_upgrade was recently broken for use upgrading from a system with adminpack
 installed.
 
 Breaking commit is:
 
 commit 30982be4e5019684e1772dd9170aaa53f5a8e894
 Author: Peter Eisentraut pete...@gmx.net
 
     Integrate pg_upgrade_support module into backend
 
 
 from pg_upgrade_dump_12870.log
 
 pg_restore: creating EXTENSION adminpack
 pg_restore: creating COMMENT EXTENSION adminpack
 pg_restore: [archiver (db)] Error while PROCESSING TOC:
 pg_restore: [archiver (db)] Error from TOC entry 2806; 0 0 COMMENT EXTENSION
 adminpack
 pg_restore: [archiver (db)] could not execute query: ERROR:  extension
 adminpack does not exist
     Command was: COMMENT ON EXTENSION adminpack IS 'administrative functions
 for PostgreSQL';
 
 
 I get the same error whether the source database is 9.2.10 or 9.5.HEAD.

Uh, I am confused how moving pg_upgrade or pg_upgrade_support would
break the loading of the adminpack extension.

-- 
  Bruce Momjian  br...@momjian.ushttp://momjian.us
  EnterpriseDB http://enterprisedb.com

  + Everyone has their own god. +


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