Re: pg_migrator issue with contrib - Mailing list pgsql-hackers
From | Tom Lane |
---|---|
Subject | Re: pg_migrator issue with contrib |
Date | |
Msg-id | 14907.1244392563@sss.pgh.pa.us Whole thread Raw |
In response to | Re: pg_migrator issue with contrib (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: pg_migrator issue with contrib
(Bruce Momjian <bruce@momjian.us>)
Re: pg_migrator issue with contrib (Merlin Moncure <mmoncure@gmail.com>) |
List | pgsql-hackers |
I wrote: > * pageinspect has changed the ABI of get_raw_page() in a way that will > likely make it dump core if the function definition is migrated from > an old DB. This needs to be fixed. > [ and similarly for some other contrib modules ] After thinking about this some more, I think that there is a fairly simple coding rule we can adopt to prevent post-migration crashes of the sort I'm worrying about above. That is: * If you change the ABI of a C-language function, change its C name. This will ensure that if someone tries to migrate the old function definition from an old database, they will get a pg_migrator failure, or at worst a clean runtime failure when they attempt to use the old definition. They won't get a core dump or some worse form of security problem. As an example, the problem in pageinspect is this diff: *************** *** 6,16 **** -- -- get_raw_page() -- ! CREATE OR REPLACE FUNCTION get_raw_page(text, int4) RETURNS bytea AS 'MODULE_PATHNAME', 'get_raw_page' LANGUAGE C STRICT; -- -- page_header() -- --- 6,21 ---- -- -- get_raw_page() -- ! CREATE OR REPLACE FUNCTION get_raw_page(text, text, int4) RETURNS bytea AS 'MODULE_PATHNAME', 'get_raw_page' LANGUAGE CSTRICT; + CREATE OR REPLACE FUNCTION get_raw_page(text, int4) + RETURNS bytea + AS $$ SELECT get_raw_page($1, 'main', $2); $$ + LANGUAGE SQL STRICT; + -- -- page_header() -- *************** The underlying C-level get_raw_page function is still there, but it now expects three arguments not two, and will crash if it's passed an int4 where it's expecting a text argument. But the old function definition will migrate without error --- there's no way for pg_migrator to realize it's installing a security hazard. The way we should have done this, which I intend to go change it to, is something like CREATE OR REPLACE FUNCTION get_raw_page(text, int4) RETURNS bytea AS 'MODULE_PATHNAME', 'get_raw_page' LANGUAGE C STRICT; CREATE OR REPLACE FUNCTION get_raw_page(text, text, int4) RETURNS bytea AS 'MODULE_PATHNAME', 'get_raw_page_3' LANGUAGE C STRICT; so that the old function's ABI is preserved. Migration of the old contrib module will then lead to the 3-argument function not being immediately available, but the 2-argument one still works. Had we not wanted to keep the 2-argument form for some reason, we would have provided only get_raw_page_3 in the .so file, and attempts to use the old function definition would fail safely. (We have actually seen similar problems before with people trying to dump and reload database containing contrib modules. pg_migrator is not creating a problem that wasn't there before, it's just making it worse.) Comments, better ideas? regards, tom lane
pgsql-hackers by date: