Thread: Bug plperl.c

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

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

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

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
>

HI Tom,

Were you able to reproduce using the updated example?

Thanks.



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
>>
>

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

diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 81d9c46e00..3f221ee01b 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -3097,6 +3097,21 @@ check_spi_usage_allowed(void)
         /* simple croak as we don't want to involve PostgreSQL code */
         croak("SPI functions can not be used in END blocks");
     }
+
+    /*
+     * Disallow SPI usage if we're not executing a fully-compiled plperl
+     * function.  It might seem impossible to get here in that case, but there
+     * are cases where Perl will try to execute code during compilation.  If
+     * we proceed we are likely to crash trying to dereference the prodesc
+     * pointer.  Working around that might be possible, but it seems unwise
+     * because it'd allow code execution to happen while validating a
+     * function, which is undesirable.
+     */
+    if (current_call_data == NULL || current_call_data->prodesc == NULL)
+    {
+        /* simple croak as we don't want to involve PostgreSQL code */
+        croak("SPI functions can not be used during function compilation");
+    }
 }
 
 
@@ -3961,6 +3976,8 @@ plperl_spi_commit(void)
 {
     MemoryContext oldcontext = CurrentMemoryContext;
 
+    check_spi_usage_allowed();
+
     PG_TRY();
     {
         SPI_commit();
@@ -3986,6 +4003,8 @@ plperl_spi_rollback(void)
 {
     MemoryContext oldcontext = CurrentMemoryContext;
 
+    check_spi_usage_allowed();
+
     PG_TRY();
     {
         SPI_rollback();

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


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
>

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&a=commitdiff&h=638300fef

            regards, tom lane