Bug plperl.c

Started by Mark Murawskiabout 4 years ago8 messagesbugs
Jump to latest
#1Mark Murawski
markm-lists@intellasoft.net

Affects Versions: 12 (and probably all others)

Steps to Reproduce:
- Run a query during plperl validation -- ie: inside a warn handler

CREATE OR REPLACE FUNCTION foo() RETURNS text
 LANGUAGE plperlu
AS $function$

use warnings;
use strict;

$SIG{'__WARN__'} = sub {
  my $pid = main::spi_exec_query('SELECT
pg_backend_pid()')->{rows}[0]{pg_backend_pid};
};

my $foo;
my $foo;  # <---- causes a warning

$function$

2022-02-22 12:17:04 EST -  -  -  - 12950 -  - 0 - LOG:  server process
(PID 19702) was terminated by signal 11: Segmentation fault

#0 0x00007f32cfc33e65 in plperl_spi_exec (query=query@entry=0x55f7157b9290 "pg_backend_pid()", limit=limit@entry=0) at plperl.c:3184
#1 0x00007f32cfc36bbe in XS__spi_exec_query (my_perl=0x55f7157b7490, cv=<optimized out>) at SPI.xs:38
#2 0x00007f32cf9d85b1 in Perl_pp_entersub () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#3 0x00007f32cf9ce896 in Perl_runops_standard () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#4 0x00007f32cf943c85 in Perl_call_sv () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#5 0x00007f32cf9aff22 in ?? () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#6 0x00007f32cf9b0ce1 in Perl_vwarn () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#7 0x00007f32cf9b115f in Perl_warner () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#8 0x00007f32cf97df83 in Perl_pad_add_name_pvn () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#9 0x00007f32cf92582d in Perl_allocmy () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#10 0x00007f32cf96600b in Perl_yylex () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#11 0x00007f32cf978e80 in Perl_yyparse () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#12 0x00007f32cfa1240f in ?? () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#13 0x00007f32cfa1dbed in Perl_pp_entereval () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#14 0x00007f32cf9ce896 in Perl_runops_standard () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#15 0x00007f32cf943ea1 in Perl_call_sv () from /usr/lib/x86_64-linux-gnu/libperl.so.5.28
#16 0x00007f32cfc2dbf3 in plperl_create_sub (
s=s@entry=0x55f7158d7858 "\n\nuse strict;\nuse warnings;\n\nuse IntellaConfig { ProgramName =>
'PgLiveQueueCreateViews' };\nuse SimpleSPI;\nuse PostgresInfo;\nuse
UpgradeUtils;\n\nuse Data::Dumper;\nu se JSON;\n\nmy ($cmd) = $_[0] ||
'repla"..., fn_oid=fn_oid@entry=39623, prodesc=<optimized out>, prodesc=<optimized out>) at plperl.c:2135
#17 0x00007f32cfc2e5d5 in compile_plperl_function (fn_oid=fn_oid@entry=39623, is_trigger=is_trigger@entry=false, is_event_trigger=is_event_trigger@entry=false) at plperl.c:2989
#18 0x00007f32cfc33c98 in plperl_validator (fcinfo=<optimized out>) at plperl.c:2053
#19 0x000055f713965590 in FunctionCall1Coll (flinfo=0x7ffe96003c50, collation=<optimized out>, arg1=<optimized out>) at fmgr.c:1140
#20 0x000055f713965c4e in OidFunctionCall1Coll (functionId=functionId@entry=24589, collation=collation@entry=0, arg1=arg1@entry=39623) at fmgr.c:1418
#21 0x000055f7136228a5 in ProcedureCreate (procedureName=<optimized out>, procNamespace=procNamespace@entry=39605, replace=<optimized out>, returnsSet=returnsSet@entry=false,
returnType=returnType@entry=25, proowner=16385, languageObjectId=24590, languageValidator=24589,
prosrc=0x55f7157c1408 "\n\nuse strict;\nuse warnings;\n\nuse IntellaConfig { ProgramName =>
'PgLiveQueueCreateViews' };\nuse SimpleSPI;\nuse PostgresInfo;\nuse
UpgradeUtils;\n\nuse Data::Dumper;\nuse JSON;\n\nmy ($cmd) = $_[0] ||
'repla"..., probin=0x0, prokind=102 'f', security_definer=false, isLeakProof=false, isStrict=false, volatility=118 'v', parallel=117 'u', parameterTypes=0x55f7156f12d0,
allParameterTypes=0, parameterModes=0, parameterNames=94519704883840, parameterDefaults=0x0, trftypes=0, proconfig=0, prosupport=0, procost=procost@entry=100, prorows=prorows@entry=0)
at pg_proc.c:726
#22 0x000055f71369a3d0 in CreateFunction (pstate=pstate@entry=0x55f7156f0bf0, stmt=stmt@entry=0x55f7156d2e48) at functioncmds.c:1152
#23 0x000055f71384786e in ProcessUtilitySlow (pstate=pstate@entry=0x55f7156f0bf0, pstmt=pstmt@entry=0x55f7156d3188,
queryString=queryString@entry=0x7f32cfc3e048 "CREATE OR REPLACE FUNCTION live_queue.create_views(cmd text)\n RETURNS
text\n LANGUAGE plperlu\nAS $function$\n\nuse strict;\nuse
warnings;\n\nuse Intella Config { ProgramName =>
'PgLiveQueueCreateViews' };\nus"..., context=context@entry=PROCESS_UTILITY_TOPLEVEL, params=params@entry=0x0, queryEnv=queryEnv@entry=0x0, completionTag=0x7ffe96004620 "",
dest=0x55f7156d3268) at utility.c:1521
#24 0x000055f7138465d2 in standard_ProcessUtility (pstmt=0x55f7156d3188,
queryString=0x7f32cfc3e048 "CREATE OR REPLACE FUNCTION live_queue.create_views(cmd text)\n RETURNS
text\n LANGUAGE plperlu\nAS $function$\n\nuse strict;\nuse
warnings;\n\nuse IntellaConfig { ProgramNa --Type <RET> for more, q to
quit, c to continue without paging--
me => 'PgLiveQueueCreateViews' };\nus"..., context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x55f7156d3268, completionTag=0x7ffe96004620 "") at utility.c:927
#25 0x000055f7138449aa in PortalRunUtility (portal=portal@entry=0x55f7157666f0, pstmt=pstmt@entry=0x55f7156d3188, isTopLevel=isTopLevel@entry=true, setHoldSnapshot=setHoldSnapshot@entry=false, dest=0x55f7156d3268,
completionTag=0x7ffe96004620 "") at pquery.c:1171
#26 0x000055f713844af5 in PortalRunMulti (portal=portal@entry=0x55f7157666f0, isTopLevel=isTopLevel@entry=true, setHoldSnapshot=setHoldSnapshot@entry=false, dest=dest@entry=0x55f7156d3268,
altdest=altdest@entry=0x55f7156d3268, completionTag=completionTag@entry=0x7ffe96004620 "") at pquery.c:1327
#27 0x000055f713844fa1 in PortalRun (portal=portal@entry=0x55f7157666f0, count=count@entry=9223372036854775807, isTopLevel=isTopLevel@entry=true, run_once=run_once@entry=true, dest=dest@entry=0x55f7156d3268,
altdest=altdest@entry=0x55f7156d3268, completionTag=0x7ffe96004620 "") at pquery.c:805
#28 0x000055f71384107d in exec_simple_query (
query_string=0x7f32cfc3e048 "CREATE OR REPLACE FUNCTION live_queue.create_views(cmd text)\n RETURNS
text\n LANGUAGE plperlu\nAS $function$\n\nuse strict;\nuse
warnings;\n\nuse IntellaConfig { ProgramName => 'PgLiveQueueCreateViews'
};\nus"...) at postgres.c:1215
#29 0x000055f7138429d1 in PostgresMain (argc=<optimized out>, argv=argv@entry=0x55f715723a78, dbname=<optimized out>, username=<optimized out>) at postgres.c:4271
#30 0x000055f7137ce4d9 in BackendRun (port=0x55f715716ad0, port=0x55f715716ad0) at postmaster.c:4510
#31 BackendStartup (port=0x55f715716ad0) at postmaster.c:4193
#32 ServerLoop () at postmaster.c:1725
#33 0x000055f7137cf400 in PostmasterMain (argc=5, argv=0x55f7156cd030) at postmaster.c:1398
#34 0x000055f71355bcca in main (argc=5, argv=0x55f7156cd030) at main.c:228' Culprit: plperl_spi_exec(char *query, int limit)
....snip.... PG_TRY(); ...snip.... spi_rv = SPI_execute(query,
current_call_data->prodesc->fn_readonly, limit); Fix: bool read_only =
(current_call_data ? current_call_data->prodesc->fn_readonly : 0);
...snip... spi_rv = SPI_execute(query, read_only, limit); Attached patch
against 12.10

Attachments:

postgres-12.10-plperl-warn-handler-crash.patchtext/x-patch; charset=UTF-8; name=postgres-12.10-plperl-warn-handler-crash.patchDownload+3-2
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mark Murawski (#1)
Re: Bug plperl.c

Mark Murawski <markm-lists@intellasoft.net> writes:

Affects Versions: 12 (and probably all others)
Steps to Reproduce:
- Run a query during plperl validation -- ie: inside a warn handler
2022-02-22 12:17:04 EST -  -  -  - 12950 -  - 0 - LOG:  server process
(PID 19702) was terminated by signal 11: Segmentation fault

I couldn't reproduce this, either in HEAD or 12.10. However, maybe
there's something faulty about your example, because in my hands it
doesn't seem that plperl_spi_exec is reached at all.

If we do need to do something here, my inclination would be to
reject execution of any SPI operations during validation. That's
not a case that's supposed to have any side-effects.

regards, tom lane

#3Mark Murawski
markm-lists@intellasoft.net
In reply to: Tom Lane (#2)
Re: Bug plperl.c

Hi Tom,

Wow that was fast!

Upon further testing, my path isn't a full fix... there's other parts of
the flow that needs current_call_data

I trimmed down my example from the guts of a complex application. Sorry
I didn't re-test with the trimmed example.  But the use-case is very
much the same... it has to do with running spi_exec_query during parse time.

I'll work on redoing the example to reproduce the crash.

Show quoted text

On 2/22/22 14:48, Tom Lane wrote:

Mark Murawski <markm-lists@intellasoft.net> writes:

Affects Versions: 12 (and probably all others)
Steps to Reproduce:
- Run a query during plperl validation -- ie: inside a warn handler
2022-02-22 12:17:04 EST -  -  -  - 12950 -  - 0 - LOG:  server process
(PID 19702) was terminated by signal 11: Segmentation fault

I couldn't reproduce this, either in HEAD or 12.10. However, maybe
there's something faulty about your example, because in my hands it
doesn't seem that plperl_spi_exec is reached at all.

If we do need to do something here, my inclination would be to
reject execution of any SPI operations during validation. That's
not a case that's supposed to have any side-effects.

regards, tom lane

#4Mark Murawski
markm-lists@intellasoft.net
In reply to: Mark Murawski (#3)
Re: Bug plperl.c

Hi Tom

Here you go:

CREATE OR REPLACE FUNCTION public.foo()
 RETURNS text
 LANGUAGE plperlu
AS $function$

use warnings;
use strict;

use lib '/tmp';
use foo;

my @a;
my @a;

$function$

---
/tmp/foo.pm
---

package foo;

use Data::Dumper;

$SIG{'__WARN__'} = sub {
   my $pid = main::spi_exec_query('SELECT pg_backend_pid()')->{rows};
  main::elog(main::NOTICE, 'Warning:' . Dumper(\@_));
  main::elog(main::NOTICE, 'foo: ' . $pid);
};

1;

Show quoted text

On 2/22/22 15:06, Mark Murawski wrote:

Hi Tom,

Wow that was fast!

Upon further testing, my path isn't a full fix... there's other parts
of the flow that needs current_call_data

I trimmed down my example from the guts of a complex application.
Sorry I didn't re-test with the trimmed example.  But the use-case is
very much the same... it has to do with running spi_exec_query during
parse time.

I'll work on redoing the example to reproduce the crash.

On 2/22/22 14:48, Tom Lane wrote:

Mark Murawski <markm-lists@intellasoft.net> writes:

Affects Versions: 12 (and probably all others)
Steps to Reproduce:
- Run a query during plperl validation -- ie: inside a warn handler
2022-02-22 12:17:04 EST -  -  -  - 12950 -  - 0 - LOG:  server process
(PID 19702) was terminated by signal 11: Segmentation fault

I couldn't reproduce this, either in HEAD or 12.10.  However, maybe
there's something faulty about your example, because in my hands it
doesn't seem that plperl_spi_exec is reached at all.

If we do need to do something here, my inclination would be to
reject execution of any SPI operations during validation. That's
not a case that's supposed to have any side-effects.

            regards, tom lane

#5Mark Murawski
markm-lists@intellasoft.net
In reply to: Mark Murawski (#4)
Re: Bug plperl.c

HI Tom,

Were you able to reproduce using the updated example?

Thanks.

Show quoted text

On 2/22/22 15:27, Mark Murawski wrote:

Hi Tom

Here you go:

CREATE OR REPLACE FUNCTION public.foo()
 RETURNS text
 LANGUAGE plperlu
AS $function$

use warnings;
use strict;

use lib '/tmp';
use foo;

my @a;
my @a;

$function$

---
/tmp/foo.pm
---

package foo;

use Data::Dumper;

$SIG{'__WARN__'} = sub {
   my $pid = main::spi_exec_query('SELECT pg_backend_pid()')->{rows};
  main::elog(main::NOTICE, 'Warning:' . Dumper(\@_));
  main::elog(main::NOTICE, 'foo: ' . $pid);
};

1;

On 2/22/22 15:06, Mark Murawski wrote:

Hi Tom,

Wow that was fast!

Upon further testing, my path isn't a full fix... there's other parts
of the flow that needs current_call_data

I trimmed down my example from the guts of a complex application.
Sorry I didn't re-test with the trimmed example. But the use-case is
very much the same... it has to do with running spi_exec_query during
parse time.

I'll work on redoing the example to reproduce the crash.

On 2/22/22 14:48, Tom Lane wrote:

Mark Murawski <markm-lists@intellasoft.net> writes:

Affects Versions: 12 (and probably all others)
Steps to Reproduce:
- Run a query during plperl validation -- ie: inside a warn handler
2022-02-22 12:17:04 EST -  -  -  - 12950 -  - 0 - LOG: server process
(PID 19702) was terminated by signal 11: Segmentation fault

I couldn't reproduce this, either in HEAD or 12.10.  However, maybe
there's something faulty about your example, because in my hands it
doesn't seem that plperl_spi_exec is reached at all.

If we do need to do something here, my inclination would be to
reject execution of any SPI operations during validation. That's
not a case that's supposed to have any side-effects.

            regards, tom lane

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mark Murawski (#5)
Re: Bug plperl.c

Mark Murawski <markm-lists@intellasoft.net> writes:

Were you able to reproduce using the updated example?

Sorry, this wasn't at the top of my to-do queue. It does reproduce
for me, and I think what we need to do about it is the attached.
In the normal code paths, this change will disallow usage of SPI until
we have completed compile_plperl_function and have a valid "prodesc"
to look at. I didn't care for your proposed workaround because

(1) it'd allow execution of non-read-only code during compilation
of a supposedly read-only function;

(2) it didn't patch the dozen or so other places where plperl SPI
functions could try to dereference prodesc;

(3) allowing code execution during function validation is, if not
an actual security hole, certainly on the hairy edge of being one.

I'm somewhat comforted about (3) because it seems the problem is only
reachable from plperlu not plperl. It's still pretty scary though.

I realize that this solution might make your use-case rather awkward.
As far as function validation goes, you can still create your functions
by setting check_function_bodies = off. If you feel you need to have
Perl code that executes during compilation otherwise, I'm not sure
what to tell you, except that it doesn't seem like a great idea.

I also noticed while looking at this that the relatively-recently-added
plperl_spi_commit and plperl_spi_rollback functions neglected to do
check_spi_usage_allowed(), so this fixes that too.

regards, tom lane

Attachments:

prevent-spi-execution-during-plperl-compilation.patchtext/x-diff; charset=us-ascii; name=prevent-spi-execution-during-plperl-compilation.patchDownload+19-0
#7Mark Murawski
markm-lists@intellasoft.net
In reply to: Tom Lane (#6)
Re: Bug plperl.c

Hi Tom,

No rush on the bug fix, just making sure you don't need anything else
from me on the reproduction.

Yeah I realized my patch wasn't a full solution after sending it in...
My test environment was a little wiggity, and I compiled and tested...
but noticed I actually wasn't using the new build... (and thought that
it was fixed with my change)

Based on the side-effects I think it does make sense to block queries
entirely during parse

Show quoted text

On 2/25/22 16:36, Tom Lane wrote:

Mark Murawski <markm-lists@intellasoft.net> writes:

Were you able to reproduce using the updated example?

Sorry, this wasn't at the top of my to-do queue. It does reproduce
for me, and I think what we need to do about it is the attached.
In the normal code paths, this change will disallow usage of SPI until
we have completed compile_plperl_function and have a valid "prodesc"
to look at. I didn't care for your proposed workaround because

(1) it'd allow execution of non-read-only code during compilation
of a supposedly read-only function;

(2) it didn't patch the dozen or so other places where plperl SPI
functions could try to dereference prodesc;

(3) allowing code execution during function validation is, if not
an actual security hole, certainly on the hairy edge of being one.

I'm somewhat comforted about (3) because it seems the problem is only
reachable from plperlu not plperl. It's still pretty scary though.

I realize that this solution might make your use-case rather awkward.
As far as function validation goes, you can still create your functions
by setting check_function_bodies = off. If you feel you need to have
Perl code that executes during compilation otherwise, I'm not sure
what to tell you, except that it doesn't seem like a great idea.

I also noticed while looking at this that the relatively-recently-added
plperl_spi_commit and plperl_spi_rollback functions neglected to do
check_spi_usage_allowed(), so this fixes that too.

regards, tom lane

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Mark Murawski (#7)
Re: Bug plperl.c

Mark Murawski <markm-lists@intellasoft.net> writes:

No rush on the bug fix, just making sure you don't need anything else
from me on the reproduction.

Nope, it's dealt with, see

https://git.postgresql.org/gitweb/?p=postgresql.git&amp;a=commitdiff&amp;h=638300fef

regards, tom lane