Thread: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Tim Bunce
Date:
[Resend. I misspelled the mailing list address on the original.]

David Wheeler has discovered a new PL/Perl failure when using Safe 2.2x.

It's not good.

In this email I'll try to explain the cause and some possible solutions.

PL/Perl compiles plperl functions inside a 'Safe compartment' which
restricts what operations can be compiled and effectively does a chroot
in the package namespace.

The compilation returns a reference to the compiled subroutine.

Until recent versions of Safe the _execution_ of that subroutine
reference happened 'outside' of the Safe compartment. Safe was only
'in effect' during the compilation.

The "big change" in Safe 2.20 (contributed by Alex Hunsaker) was that
returned subroutine references were 'wrapped' in extra code that
executed the subroutine inside the Safe compartment.

The ticket for this change, with much discussion, is at
http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-628047

The original motivation for the change was to fix a problem with using
sort {} in a threaded perl. As a side-effect it also increased security:
the Safe restrictions for plperl were in effect at runtime as well as
compiletime.

Unfortunately it also had a number of subtle knock-on effects that we've
been dealing with via Safe 2.21..2.23.

So far so good, but now we've hit another problem.

----- Forwarded message from "David E. Wheeler" <david.wheeler@pgexperts.com> -----

[...] When I move that function so that it's created just before my test
function that uses it, it works. It's only if it's created by another
process and already in the database when my test script connects that it fails.

Okay, here's the test case: Create this function:

    CREATE OR REPLACE FUNCTION foo( integer) RETURNS SETOF INT LANGUAGE plperl AS $$ $$;

Then disconnect and reconnect and run this:

    CREATE OR REPLACE FUNCTION try() RETURNS VOID LANGUAGE plperl AS $$
        my $sth = spi_query("SELECT id FROM foo( 0 ) AS g(id)");
        while( my $row = spi_fetchrow($sth) ) {
        }
    $$;
    SELECT try();

Result:

ERROR:  error from Perl function "try": Undefined subroutine &main::mksafefunc called at line 3.
    (in cleanup) creation of Perl function "foo" failed:     (in cleanup) Undefined subroutine &main::mksafefunc called
atline 3. at line 3. 

FYI, it's the call to spi_fetchrow() that fails, not spi_prepare().
PostgreSQL 8.4.2, Perl 5.10.1 with ithreads, Safe 2.23.

----- End forwarded message -----

I believe (but haven't yet confirmed) that the problem here is recursion.

When plperl recurses into itself via foo() the Safe restrictions are
still in effect. So plperl can't find the mksafefunc subroutine because
it's still 'inside' the chroot'd namespace.

This affects all versions of PostgreSQL.

After some head scratching I think the best course of action is to
change Safe to make the new behaviour optional and default to off.
In other words restore the pre-2.20 behaviour.

That'll fix the immediate problem for all PostgreSQL versions.
Security will be no better or worse that it was before Safe 2.20.
The old sort { } bug (where $a & $b) don't work will return but
that seems a very small price to pay for a simple fix.

I'd like to see PostgreSQL re-enable use of the wrapped subroutines
in the future but it'll require some development effort. The plperl
entry points will need to detect if Safe is 'in effect' and locally undo
it.  There's some prior art in http://search.cpan.org/perldoc?Safe::Hole
that might be useful.

I don't think it's viable to tackle this for PostgreSQL 9.0.

Tomorrow I'll confirm my hypothesis and work on (another) patch for Safe
to disable the wrapping and test it with PostgreSQL 8.4 and 9.0.

Tim.

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Tim Bunce
Date:
On Tue, Feb 23, 2010 at 02:59:05PM -0700, Alex Hunsaker wrote:
> On Tue, Feb 23, 2010 at 14:29, Tim Bunce <Tim.Bunce@pobox.com> wrote:
> > David Wheeler has discovered a new PL/Perl failure when using Safe 2.2x.
>
> *sigh*.
>
> Thanks for all the testing David!  And of course many thanks to Tim
> for all the hours of analysis and hard work!
>
> > That'll fix the immediate problem for all PostgreSQL versions.
> > Security will be no better or worse that it was before Safe 2.20.
> > The old sort { } bug (where $a & $b) don't work will return but
> > that seems a very small price to pay for a simple fix.
>
> +1.  Although I think I might be in favor of just ripping it out all
> together.  There are a couple of goofy things about the current
> behavior anyway. Notably if you are not using a threaded build you
> never get the extra protection.

Yes, that needs resolving. Thanks for the reminder.

> At the time it was really just a fix
> for the namespace issues.  I do agree the increased security is
> nice... But that was not the primary goal :)

We'll discuss the issues and API option for Safe on the perl5-porters
mailing list.

> > I'd like to see PostgreSQL re-enable use of the wrapped subroutines
> > in the future but it'll require some development effort.
>
> I would to.
>
> > The plperl
> > entry points will need to detect if Safe is 'in effect' and locally undo
> > it.  There's some prior art in http://search.cpan.org/perldoc?Safe::Hole
> > that might be useful.
>
> Yick... There must be a better way?

Doesn't seem too icky. Basically plperl would need to save the values of
PL_defstash, PL_incgv and PL_op_mask when a plperl interpreter is
initialized.  And then local()'ly restore them in the plperl entry points.
Should only be a few lines of code - in theory :)

Tim.

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Alex Hunsaker
Date:
On Tue, Feb 23, 2010 at 15:23, Tim Bunce <Tim.Bunce@pobox.com> wrote:

> I believe (but haven't yet confirmed) that the problem here is recursion.
> This affects all versions of PostgreSQL.

Hrm... This seems to work for me in HEAD.  It certainly breaks in 8.3.
 Am I missing something?

----
$ bin/psql postgres
psql (9.0devel)
Type "help" for help.

postgres=#  CREATE OR REPLACE FUNCTION foo( integer) RETURNS SETOF INT
LANGUAGE plperl AS $$ $$;
CREATE FUNCTION
postgres=# \q
$ bin/psql postgres
psql (9.0devel)
Type "help" for help.

postgres=#  CREATE OR REPLACE FUNCTION try() RETURNS VOID LANGUAGE plperl AS $$
       my $sth = spi_query("SELECT id FROM foo( 0 ) AS g(id)");
       while( my $row = spi_fetchrow($sth) ) {
       }
   $$;
CREATE FUNCTION
postgres=# SELECT try();
 try
-----

(1 row)

Seems like assuming I did the above correctly we just have a bug in
the older branches where the "SELECT id FROM foo(0)..." part is
getting compiled/executed in the wrong perl context.  In-fact I would
not be surprised at all if there are other dragons lurking when plperl
calls something that tries to compile/call a plperl function.  Safe
>2.20 or older.

Ill keep digging.

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Alex Hunsaker
Date:
On Wed, Feb 24, 2010 at 19:17, Alex Hunsaker <badalex@gmail.com> wrote:
> On Tue, Feb 23, 2010 at 15:23, Tim Bunce <Tim.Bunce@pobox.com> wrote:
>
>> I believe (but haven't yet confirmed) that the problem here is recursion.
>> This affects all versions of PostgreSQL.

> Ill keep digging.

Ok I understand now, basically the problem is (as Tim also described elsewhere):

postgres->plperl_call_perl_func->SPI->postgres->plperl_create_sub

On that last call to plperl_create_sub we are still executing under
Safe (as its the same interpreter).   And so it fails when it tries to
compile a new sub.

ISTM the easiest and safest fix would be to not allow recursive plperl
creations.  You could still call plperl functions within functions,
just not if they are not defined.  This limitation really blows so im
hoping someone else has a better idea?  Alternately we could also
break out of the safe, compile the sub and then go back to it as Tim
suggested up-thread.  I think this could work as long as its not to
nasty (which Tim does not seem to think it would be).

Thoughts? Better Ideas?

[ patch against 8.3/8.4 ]
----
*** a/src/pl/plperl/plperl.c
--- b/src/pl/plperl/plperl.c
***************
*** 126,131 **** static HTAB *plperl_proc_hash = NULL;
--- 126,132 ----
  static HTAB *plperl_query_hash = NULL;

  static bool plperl_use_strict = false;
+ static bool plperl_executing = false;

  /* this is saved and restored by plperl_call_handler */
  static plperl_call_data *current_call_data = NULL;
***************
*** 1117,1125 **** plperl_call_perl_func(plperl_proc_desc *desc,
FunctionCallInfo fcinfo)
--- 1118,1132 ----
      }
      PUTBACK;

+     if (desc->lanpltrusted)
+         plperl_executing = true;
+
      /* Do NOT use G_KEEPERR here */
      count = perl_call_sv(desc->reference, G_SCALAR | G_EVAL);

+     if (desc->lanpltrusted)
+         plperl_executing = false;
+
      SPAGAIN;

      if (count != 1)
***************
*** 1697,1702 **** compile_plperl_function(Oid fn_oid, bool is_trigger)
--- 1704,1721 ----

          check_interp(prodesc->lanpltrusted);

+         /************************************************************
+          * Dont let us recursively create a plperl function from a plperl function
+          * as plperl_create_sub gets called we are running under Safe and fails.
+          * TODO: We could break out of the safe via Safe::HOLE or some such.
+          ************************************************************/
+         if (prodesc->lanpltrusted && plperl_executing)
+             ereport(ERROR,
+                 (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                  errmsg("could not create plperl function \"%s\"", prodesc->proname),
+                  errdetail("plperl functions can not recursivley define other
plperl functions"),
+                  errhint("try calling the function first")));
+
          prodesc->reference = plperl_create_sub(prodesc->proname,
                                                 proc_source,
                                                 prodesc->lanpltrusted);

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Tom Lane
Date:
Alex Hunsaker <badalex@gmail.com> writes:
> ISTM the easiest and safest fix would be to not allow recursive plperl
> creations.  You could still call plperl functions within functions,
> just not if they are not defined.  This limitation really blows

That's the understatement of the month.  What you're saying, IIUC, is
that if function A calls function B via a SPI command, and B wasn't
executed previously in the current session, it would fail?  Seems
entirely unacceptable.

            regards, tom lane

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Alex Hunsaker
Date:
On Wed, Feb 24, 2010 at 20:19, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>What you're saying, IIUC, is
> that if function A calls function B via a SPI command, and B wasn't
> executed previously in the current session, it would fail? =C2=A0Seems
> entirely unacceptable.

Yep, thats right :(.  Thanks, thats exactly the kind of feedback I
wanted to get.

I think we will see if we can get this fixed on the Safe/perl side then.

Tim, I think unless the Safe::Hole stuff is really straight forward it
seems like (as we previously agreed) the best change is to revert safe
to its old behavior for now.

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Alex Hunsaker
Date:
On Wed, Feb 24, 2010 at 20:37, Alex Hunsaker <badalex@gmail.com> wrote:
> On Wed, Feb 24, 2010 at 20:19, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Seems entirely unacceptable.

> I think we will see if we can get this fixed on the Safe/perl side then.

BTW the trade off here is we revert back to sort { $a <=> $b } not
working.  That is if you could call it a trade off... The level of
breaking is not really comparable :)

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Tom Lane
Date:
Alex Hunsaker <badalex@gmail.com> writes:
> On Wed, Feb 24, 2010 at 20:37, Alex Hunsaker <badalex@gmail.com> wrote:
>> On Wed, Feb 24, 2010 at 20:19, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> Seems entirely unacceptable.

>> I think we will see if we can get this fixed on the Safe/perl side then.

> BTW the trade off here is we revert back to sort { $a <=> $b } not
> working.  That is if you could call it a trade off... The level of
> breaking is not really comparable :)

That's two unacceptable alternatives, you need to find a third one.
I think most people will have no trouble settling on "do not update
to Safe 2.2x" if you don't offer a better solution than these.

            regards, tom lane

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Alex Hunsaker
Date:
On Tue, Feb 23, 2010 at 15:54, Tim Bunce <Tim.Bunce@pobox.com> wrote:
> Doesn't seem too icky. Basically plperl would need to save the values of
> PL_defstash, PL_incgv and PL_op_mask when a plperl interpreter is
> initialized. =C2=A0And then local()'ly restore them in the plperl entry p=
oints.
> Should only be a few lines of code - in theory :)

Ok I can get behind this.  I did some testing and we could probably
even store less than than that if we could do the equivalent of:
Safe->share('::mksafefunc');
pl_perl_createsub()
Safe->unshare('::mksafefunc');

See my quick test case:
my $s =3D Safe->new();
$s->permit(qw(print));

our $obj =3D sub { return eval 'sub { print "b\n";}' };
$obj->()->();
$s->share(qw($obj));
$s->reval('$obj->()->();');
print $@ . "\n";
---
b
b

(BTW the above fails with the helpful "Undefined subroutine &main::
called at (eval 6) line 1." without the ->permt(qw(print))")

So we might not even have to store anything if we can make it behave
as above.  However I think it will be cleaner to me to locally restore
them as your originally suggested.

BTW sorry for my scatter braininess.  I keep flip flopping between
revet Safe or patch postgres.  ATM it seems if the patch is simple we
can get it back patched and into 9.0.  So my vote is lets try that, if
its to hard then lets see about reverting Safe.  Sound Ok?

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
"David E. Wheeler"
Date:
On Feb 24, 2010, at 7:19 PM, Tom Lane wrote:

>> ISTM the easiest and safest fix would be to not allow recursive plperl
>> creations.  You could still call plperl functions within functions,
>> just not if they are not defined.  This limitation really blows
>
> That's the understatement of the month.  What you're saying, IIUC, is
> that if function A calls function B via a SPI command, and B wasn't
> executed previously in the current session, it would fail?  Seems
> entirely unacceptable.

Exactly what I was thinking. This "fix" is right out.

Best,

David

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Alex Hunsaker
Date:
On Wed, Feb 24, 2010 at 22:01, Alex Hunsaker <badalex@gmail.com> wrote:
> Ok I can get behind this. =C2=A0I did some testing and we could probably
> even store less than than that if we could do the equivalent of:
> Safe->share('::mksafefunc');
> pl_perl_createsub()
> Safe->unshare('::mksafefunc');

On 2nd thought this basically requires your fix anyway.  To make it so
you can share something in safe from within safe means we will need to
enable more opcodes there... so it would end up being the same
solution.

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Tim Bunce
Date:
On Wed, Feb 24, 2010 at 10:58:17PM -0500, Tom Lane wrote:
> Alex Hunsaker <badalex@gmail.com> writes:
> > On Wed, Feb 24, 2010 at 20:37, Alex Hunsaker <badalex@gmail.com> wrote:
> >> On Wed, Feb 24, 2010 at 20:19, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> >>> Seems entirely unacceptable.
>
> >> I think we will see if we can get this fixed on the Safe/perl side then.
>
> > BTW the trade off here is we revert back to sort { $a <=> $b } not
> > working.  That is if you could call it a trade off... The level of
> > breaking is not really comparable :)
>
> That's two unacceptable alternatives, you need to find a third one.
> I think most people will have no trouble settling on "do not update
> to Safe 2.2x" if you don't offer a better solution than these.

I believe the next version of Safe will revert to Safe 1.19 behaviour
because the side effects of the change in 2.20 are too severe for it to
be left enabled by default.

Tim.

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
"David E. Wheeler"
Date:
On Feb 25, 2010, at 10:01 AM, Tim Bunce wrote:

>> That's two unacceptable alternatives, you need to find a third one.
>> I think most people will have no trouble settling on "do not update
>> to Safe 2.2x" if you don't offer a better solution than these.
>=20
> I believe the next version of Safe will revert to Safe 1.19 behaviour
> because the side effects of the change in 2.20 are too severe for it to
> be left enabled by default.

Which means losing sort $a <=3D> $b again, alas. Such was always the case i=
n the past, so that might be an okay tradeoff to get recursive calls workin=
g again, but I certainly hope that Safe can be updated in the near future t=
o give us both.

There seem to be no good answers here.

David

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
"David E. Wheeler"
Date:
On Feb 25, 2010, at 10:01 AM, Tim Bunce wrote:

>> That's two unacceptable alternatives, you need to find a third one.
>> I think most people will have no trouble settling on "do not update
>> to Safe 2.2x" if you don't offer a better solution than these.
>=20
> I believe the next version of Safe will revert to Safe 1.19 behaviour
> because the side effects of the change in 2.20 are too severe for it to
> be left enabled by default.

Which means losing sort $a <=3D> $b again, alas. Such was always the case i=
n the past, so that might be an okay tradeoff to get recursive calls workin=
g again, but I certainly hope that Safe can be updated in the near future t=
o give us both.

There seem to be no good answers here.

David

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Alex Hunsaker
Date:
On Thu, Feb 25, 2010 at 11:04, David E. Wheeler
<david.wheeler@pgexperts.com> wrote:
> There seem to be no good answers here.

Yeah, Here is the thing I don't think we can fix 'safe' or even patch
perl to get recursive calls to work.  Maybe Tim sees a way?  We can
work around it in 9.0 with plperl.on_init.  But breaking the back
branches makes it a non-starter.

Basically the problem as I see it is:
1) we call a plperl function which makes perl safe by denying certain
perl operations such as eval.
2) now that the interpreter is in that context, we try to compile a
new sub (using eval) and it fails.

I just don't see a way to make it work without making Safe useless.
For instance we could import the function that compiles the perl sub
into the safe.  But then anyone could call it and eval in random code.

Maybe Tim has something up his sleeve? (That does not require plperl.on_init?)

Here are the options I see:
1) revert safe to pre 2.2x behavior breaking sort {}, but fixing this
issue.  There would be a new function or a way to opt-in to the new
2.2x secure behavior (which would also fix sort, but this issue would
remain). (Tim's favored ATM)
[ breaks sort ]

2) patch perl to fix the sort {} issue (last I looked it would be
quite invasive and I think I would have a hard time getting it into
blead let alone 5.10.2 and 5.8.10), this issue would still be broken
[ still need to revert safe ]

3) patch postgres to fix the recursive issue (What I'm leaning towards)
[ fixes both issues ]

4) patch postgres to fix the *a, *b issue
[ still need to revert safe ]

5) make safe default import *a, *b to fix sort {} (rejected in the past)
[ still need to revert safe]

6) we might be able to do something in Safe to work around this... I
have an idea or two but I don't think they will pan out. (Basically it
used to work because we were only in the Safe context for that sub {},
we might be able to restore that behavior somehow.  that would fix
both issues...  I don't have any bright ideas at the moment)

Ill also point out Tim has more or less pointed out all these
solutions up-thread as well.

Anyone see any other options?  There are hybrid solutions here.  For
instance if we did #1, we could also prepare a patch for 9.1 that will
'opt-in' for the more secure closures and the sort {} fixes.  The
patch would in essence be #3. After that's been field tested for a
while we could see about back patching it.

If wishes were horses we'd all be eating steak.

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Tom Lane
Date:
Alex Hunsaker <badalex@gmail.com> writes:
> 3) patch postgres to fix the recursive issue (What I'm leaning towards)
> [ fixes both issues ]

How exactly would you propose doing that?

            regards, tom lane

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Alex Hunsaker
Date:
On Thu, Feb 25, 2010 at 12:20, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alex Hunsaker <badalex@gmail.com> writes:
>> 3) patch postgres to fix the recursive issue (What I'm leaning towards)
>> [ fixes both issues ]
>
> How exactly would you propose doing that?

Well that's the thing, probably by what I described below that. Namely
get something working for 9.1 and after we know its good and solid see
if we can back patch it.  Unfeasible?  If its really really simple and
straight forward maybe we can find a -commiter willing to commit it
sooner.  But I'm dubious.  I think the feeling between me and Tim is
patching postgres is a last resort...  Maybe if its to fix both sort
{} and this it might be worth it. (That's at least how I parsed what
you said :) ).  Ill see if I can figure something out via straight
Safe tonight.

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
"David E. Wheeler"
Date:
On Feb 25, 2010, at 11:29 AM, Alex Hunsaker wrote:

> Well that's the thing, probably by what I described below that. Namely
> get something working for 9.1 and after we know its good and solid see
> if we can back patch it.  Unfeasible?  If its really really simple and
> straight forward maybe we can find a -commiter willing to commit it
> sooner.  But I'm dubious.  I think the feeling between me and Tim is
> patching postgres is a last resort...  Maybe if its to fix both sort
> {} and this it might be worth it. (That's at least how I parsed what
> you said :) ).  Ill see if I can figure something out via straight
> Safe tonight.

I think Tom meant, what sorts of changes to PostgreSQL do you think might s=
olve the problem?

Best,

David

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
"Greg Sabino Mullane"
Date:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160


> Well that's the thing, probably by what I described below that. Namely
> get something working for 9.1 and after we know its good and solid see
> if we can back patch it.

Just don't break anything in 9.0 that relies on plperl please. :) To that
end, let me know when HEAD has something somewhat stable, and I'll
run some tests against it (e.g. Bucardo, which uses lots of plperl)

- --
Greg Sabino Mullane greg@turnstep.com
End Point Corporation http://www.endpoint.com/
PGP Key: 0x14964AC8 201002251458
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iEYEAREDAAYFAkuG1qIACgkQvJuQZxSWSshX4gCgrEPDLc5GQFKF0zf0eEZv4wDv
Qt8AoOngHlVD+OXs26naSnqcrJl4xGFG
=Ec6y
-----END PGP SIGNATURE-----

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Alex Hunsaker
Date:
On Thu, Feb 25, 2010 at 12:31, David E. Wheeler
<david.wheeler@pgexperts.com> wrote:
> I think Tom meant, what sorts of changes to PostgreSQL do you think might solve the problem?

Here is what Tim said:
>Doesn't seem too icky. Basically plperl would need to save the values of
>PL_defstash, PL_incgv and PL_op_mask when a plperl interpreter is
>initialized.  And then local()'ly restore them in the plperl entry points.
>Should only be a few lines of code - in theory :)

Basically when we go to compile a new plperl sub we would 'break' out
of the safe, compile the sub (which it itself would go back into the
safe) and then because we local()ly set/restore we would be reset to
the same context when we returned.

Not only is there some prior art for this method (Safe::Hole).  After
playing with it a bit last night I agree it should be fairly small and
simple.  Im a bit worried there might be some corner cases.  All the
easy ones I see cant happen with plperl only with arbitrary 3rd party
modules.  Things we might need to do in addition would be: ignoring
END blocks, saving PL_curstash, Invalidating the ISA and method
caches, saving/restoring INC...  And that would only be because im
worried there might be some strange issues with the new
plperl.plperl_init.  Its hand waving at this point.

Another way I played with last night is calling
Opcode::_save_call_sv("main", Opcode::full_ops, sub_to_compile)
directly instead of perl_call_sv() to compile the sub (or in
pl/plperl.c plperl_create_sub replace the call to perl_call_sv with
Opcode::_safe_call_sv).  It should be even simpler and safer.  We
would still need to save and local() a few things... But it should
work.

Anyway, im hoping to look at this more tonight... My lunch is over :)

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Alex Hunsaker
Date:
On Thu, Feb 25, 2010 at 12:59, Greg Sabino Mullane <greg@turnstep.com> wrote:
> Just don't break anything in 9.0 that relies on plperl please. :) To that
> end, let me know when HEAD has something somewhat stable, and I'll
> run some tests against it (e.g. Bucardo, which uses lots of plperl)

Defiantly, the goal is to not break anything :).

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Alex Hunsaker
Date:
On Thu, Feb 25, 2010 at 13:03, Alex Hunsaker <badalex@gmail.com> wrote:
> On Thu, Feb 25, 2010 at 12:59, Greg Sabino Mullane <greg@turnstep.com> wrote:
>> Just don't break anything in 9.0 that relies on plperl please. :) To that
>> end, let me know when HEAD has something somewhat stable, and I'll
>> run some tests against it (e.g. Bucardo, which uses lots of plperl)
>
> Defiantly, the goal is to not break anything :).

Err oops, as David Fetter pointed out... I *think* i meant to say definitely.

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Tim Bunce
Date:
On Thu, Feb 25, 2010 at 10:04:44AM -0800, David E. Wheeler wrote:
> On Feb 25, 2010, at 10:01 AM, Tim Bunce wrote:
>
> >> That's two unacceptable alternatives, you need to find a third one.
> >> I think most people will have no trouble settling on "do not update
> >> to Safe 2.2x" if you don't offer a better solution than these.
> >
> > I believe the next version of Safe will revert to Safe 1.19 behaviour
> > because the side effects of the change in 2.20 are too severe for it to
> > be left enabled by default.
>
> Which means losing sort $a <=> $b again, alas. Such was always the
> case in the past, so that might be an okay tradeoff to get recursive
> calls working again, but I certainly hope that Safe can be updated in
> the near future to give us both.
>
> There seem to be no good answers here.

There is one fairly good answer:

Use a perl that's compiled to support multiplicity but not threads.
That avoids the sort bug and, as an extra bonus, gives plperl a
significant speed boost.

Tim.

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
"David E. Wheeler"
Date:
On Feb 25, 2010, at 12:58 PM, Tim Bunce wrote:

>> Which means losing sort $a <=3D> $b again, alas. Such was always the
>> case in the past, so that might be an okay tradeoff to get recursive
>> calls working again, but I certainly hope that Safe can be updated in
>> the near future to give us both.
>>=20
>> There seem to be no good answers here.
>=20
> There is one fairly good answer:
>=20
> Use a perl that's compiled to support multiplicity but not threads.
> That avoids the sort bug and, as an extra bonus, gives plperl a
> significant speed boost.

That solves the problem with recursion or with $a and $b or both?

I'm happy to rebuild Perl without threads, since I'm not going to use Padre=
 after all. But that won't help the millions who rely on package-supplied P=
erls, which are nearly always threaded AFAICT.

Best,

David

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Tom Lane
Date:
Tim Bunce <Tim.Bunce@pobox.com> writes:
> On Thu, Feb 25, 2010 at 10:04:44AM -0800, David E. Wheeler wrote:
>> There seem to be no good answers here.

> There is one fairly good answer:

> Use a perl that's compiled to support multiplicity but not threads.
> That avoids the sort bug and, as an extra bonus, gives plperl a
> significant speed boost.

[ scratches head... ] Why would threading have anything to do with the
security behavior imposed by Safe?

            regards, tom lane

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Alex Hunsaker
Date:
On Thu, Feb 25, 2010 at 14:06, David E. Wheeler
<david.wheeler@pgexperts.com> wrote:
> On Feb 25, 2010, at 12:58 PM, Tim Bunce wrote:
>> There is one fairly good answer:
>>
>> Use a perl that's compiled to support multiplicity but not threads.

> That solves the problem with recursion or with $a and $b or both?

Yes ATM because we only kick in the extra security if you are on
threads... Its a bit of a kludge in Safe.  I know Tim wants to rectify
that...

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
"David E. Wheeler"
Date:
On Feb 25, 2010, at 1:08 PM, Alex Hunsaker wrote:

>> That solves the problem with recursion or with $a and $b or both?
>
> Yes ATM because we only kick in the extra security if you are on
> threads... Its a bit of a kludge in Safe.  I know Tim wants to rectify
> that...

By adding the extra security when *not* under threads as well?

I find this all very confusing, frankly.

Best,

David

Re: New PL/Perl failure with Safe 2.2x due to recursion (8.x & 9.0)

From
Tim Bunce
Date:
On Thu, Feb 25, 2010 at 04:06:51PM -0500, Tom Lane wrote:
> Tim Bunce <Tim.Bunce@pobox.com> writes:
> > On Thu, Feb 25, 2010 at 10:04:44AM -0800, David E. Wheeler wrote:
> >> There seem to be no good answers here.
>
> > There is one fairly good answer:
>
> > Use a perl that's compiled to support multiplicity but not threads.
> > That avoids the sort bug and, as an extra bonus, gives plperl a
> > significant speed boost.
>
> [ scratches head... ] Why would threading have anything to do with the
> security behavior imposed by Safe?

The new security behavior imposed by Safe 2.20+ was added by
Alex to workaround the problem with sort() not seeing $a & $b
in perl built with threads enabled [1]

The extra security was simply a side effect of the workaround.
(And the extra security only applied if threads were enabled.)

So using a perl built without threads is an alternative solution
to the (longstanding) sort bug that might suit some people.

Here's how I see this all working out:

- Safe 2.24 will default to the 2.19 behaviour for closures.
    Wrapping of closures will be possible via a new method call.
    The utf8 fixes will remain.

- Which means sort() with $a/$b in threaded perls will revert to being broken.
    People have got used to that and there are workarounds available

- Someone (not me anytime soon) might work on adding code to
    plperl_call_handler(), plperl_validator() etc. to temporarily undo
    the Safe restrictions if they're in effect on entry (due to recursion).
    As outlined in http://archives.postgresql.org/pgsql-bugs/2010-02/msg00249.php

- Then the plc_safe_ok.pl code could be altered to call the new Safe
    method, if available, to wrap the returned closures and thus
    re-enable the security features and re-fix the sort() bug.

Tim.

[1] http://rt.perl.org/rt3/Ticket/Display.html?id=60374#txn-627691