Mailing List Archive: Perl: porters

[perl #113486] CopSTASH can point to freed-and-reused SV

Index | Next | Previous | View Threaded

perlbug-followup at perl

Jun 3, 2012, 6:32 PM

Post #1 of 2 (45 views)
Permalink

[perl #113486] CopSTASH can point to freed-and-reused SV

# New Ticket Created by Father Chrysostomos
# Please include the string: [perl #113486]
# in the subject line of all future correspondence about this issue.
# <URL: https://rt.perl.org:443/rt3/Ticket/Display.html?id=113486 >

On non-threaded builds, cops have a direct pointer to their stash, which is not reference-counted.

caller returns undef in that case:

$ ./perl -ILib -e 'package foo { sub bar { main::bar() } } sub bar { delete $::{"foo::"}; warn scalar caller }; foo::bar'
Warning: something's wrong at -e line 1.

But it returns undef by accident. cop_stash is pointing to a freed scalar, which is not SvOOK, so HvNAME_HEK returns false.

I haven’t come up with a test case yet, but the freed scalar could be reused for another stash, giving erroneous results. Or it could be used for a scalar with the offset hack applied, which would result in crashes.

In fixing another bug, there is a chance I will extend this bug to threaded perls, too.

This is how I know the scalar is freed:

$ gdb --args ./perl -ILib -e 'package foo { sub bar { main::bar() } } sub bar { delete $::{"foo::"}; warn caller }; foo::bar'
GNU gdb 6.3.50-20050815 (Apple version gdb-1469) (Wed May 5 04:30:06 UTC 2010)
Copyright 2004 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB. Type "show warranty" for details.
This GDB was configured as "i386-apple-darwin"...Reading symbols for shared libraries .... done

(gdb) break Perl_pp_caller
Breakpoint 1 at 0x17225f: file pp_ctl.c, line 1877.
(gdb) run
Starting program: /Users/sprout/Perl/perl.git-copy/perl -ILib -e package\ foo\ \{\ sub\ bar\ \{\ main::bar\ \}\ \}\ sub\ bar\ \{\ delete\ \$::\{\"foo::\"\}\;\ warn\ caller\ \}\;\ foo::bar
Reading symbols for shared libraries +++. done

Breakpoint 1, Perl_pp_caller () at pp_ctl.c:1877
1877 dSP;
(gdb) n
Current language: auto; currently c++
1883 bool has_arg = MAXARG && TOPs;
(gdb)
1891 cx = caller_cx(count + !!(PL_op->op_private & OPpOFFBYONE), &dbcx);
(gdb)
1892 if (!cx) {
(gdb)
1900 stash_hek = HvNAME_HEK((HV*)CopSTASH(cx->blk_oldcop));
(gdb)
1901 if (GIMME != G_ARRAY) {
(gdb) call Perl_sv_dump( cx->cx_u.cx_blk.blku_oldcop->cop_stash)
SV = UNKNOWN(0xff) (0xabababab) at 0x821b70
REFCNT = -1414812757
FLAGS = ()
(gdb)

perlbug-comment at perl

Jun 4, 2012, 10:07 PM

Post #2 of 2 (38 views)
Permalink

[perl #113486] CopSTASH can point to freed-and-reused SV [In reply to]

On Sun Jun 03 18:32:35 2012, sprout wrote:
> On non-threaded builds, cops have a direct pointer to their stash,
> which is not reference-counted.
>
> caller returns undef in that case:
>
> $ ./perl -ILib -e 'package foo { sub bar { main::bar() } } sub bar {
> delete $::{"foo::"}; warn scalar caller }; foo::bar'
> Warning: something's wrong at -e line 1.
>
> But it returns undef by accident. cop_stash is pointing to a freed
> scalar, which is not SvOOK, so HvNAME_HEK returns false.
>
> I haven’t come up with a test case yet, but the freed scalar could be
> reused for another stash, giving erroneous results.

I’m wondering whether this is even worth fixing.

For non-threaded perls, it would require modifying any remaining op
trees when freeing hashes. It would probably then require every stash
to have symtab magic attached to it.

Under threads, we would have to make cops hold a refcount on the
PL_stashpad slot so we know when it can be reused.

Having cops hold a refcount on the stash would result in too many
circular references, so that’s a not a good option.

> Or it could be
> used for a scalar with the offset hack applied, which would result
> in crashes.

{ package foo; sub bar { main::bar() } }
sub bar {
delete $::{"foo::"};
my $x = \($1+2);
my $y = \($1+2); # this is the one that reuses the mem addr, but
my $z = \($1+2); # try the others just in case
s/2// for $$x, $$y, $$z; # now SvOOK
warn scalar caller # boom
};
foo::bar

I’ve fixed that in commit e788621.

> In fixing another bug, there is a chance I will extend this bug to
> threaded perls, too.

Which I have done. But fixing that other bug actually fixed about three
bugs at once.

--

Father Chrysostomos

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact Gossamer Threads