Skip Menu |
Report information
Id: 92264
Status: pending release
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: sprout <sprout [at] cpan.org>
Cc:
AdminCc:

Operating System: darwin
PatchStatus: (no value)
Severity: low
Type: core
Perl Version: 5.14.0
Fixed In: 5.27.7



Subject: Freeing $a or $b during sort causes a double free
Date: Sun, 5 Jun 2011 15:51:16 -0700
To: perlbug [...] perl.org
From: Father Chrysostomos <sprout [...] cpan.org>
Download (untitled) / with headers
text/plain 2.6k
$ perl5.14.0 -e '@_ = sort { *a = \1 } 1, 2' Attempt to free unreferenced scalar: SV 0x826480, Perl interpreter: 0x800000. I think pp_sort is misusing SAVESPTR. --- Flags: category=core severity=low ack=no --- Site configuration information for perl 5.14.0: Configured by sprout at Wed May 11 13:45:58 PDT 2011. Summary of my perl5 (revision 5 version 14 subversion 0) configuration: Snapshot of: eb70bb4a400e88a66c7e10414a2d52b5da4cfd1f Platform: osname=darwin, osvers=10.5.0, archname=darwin-thread-multi-2level uname='darwin pint.local 10.5.0 darwin kernel version 10.5.0: fri nov 5 23:20:39 pdt 2010; root:xnu-1504.9.17~1release_i386 i386 ' config_args='-Dusedevel -de -Duseithreads -Doptimize=-g' hint=recommended, useposix=true, d_sigaction=define useithreads=define, usemultiplicity=define useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef use64bitint=undef, use64bitall=undef, uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include', optimize='-g', cppflags='-fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion='', gccversion='4.2.1 (Apple Inc. build 5664)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /usr/lib libs=-ldbm -ldl -lm -lutil -lc perllibs=-ldl -lm -lutil -lc libc=/usr/lib/libc.dylib, so=dylib, useshrplib=false, libperl=libperl.a gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector' Locally applied patches: RC3 --- @INC for perl 5.14.0: /usr/local/lib/perl5/site_perl/5.14.0/darwin-thread-multi-2level /usr/local/lib/perl5/site_perl/5.14.0 /usr/local/lib/perl5/5.14.0/darwin-thread-multi-2level /usr/local/lib/perl5/5.14.0 /usr/local/lib/perl5/site_perl . --- Environment for perl 5.14.0: DYLD_LIBRARY_PATH (unset) HOME=/Users/sprout LANG=en_US.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/X11/bin:/usr/local/bin PERL_BADLANG (unset) SHELL=/bin/bash
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 537b
On Sun Jun 05 15:51:34 2011, sprout wrote: Show quoted text
> $ perl5.14.0 -e '@_ = sort { *a = \1 } 1, 2' > Attempt to free unreferenced scalar: SV 0x826480, Perl interpreter: > 0x800000. > > I think pp_sort is misusing SAVESPTR.
Attached is a test case. I don’t know how to fix this efficiently. Currently GvSV(*a) and GvSV(*b) are not reference-counted during sort. But no other code knows about that. Making them reference-counted is the obvious fix, but I presume it was not done originally for efficency’s sake. -- Father Chrysostomos
Download open_OmsfssFN.txt
text/plain 692b
diff --git a/t/op/sort.t b/t/op/sort.t index 6dedeeb..5911810 100644 --- a/t/op/sort.t +++ b/t/op/sort.t @@ -6,7 +6,7 @@ BEGIN { require 'test.pl'; } use warnings; -plan( tests => 171 ); +plan( tests => 172 ); # these shouldn't hang { @@ -960,3 +960,16 @@ is @x, 0, 'sort; returns empty list'; eval '{@x = sort} 1'; is $@, '', '{sort} does not die'; is @x, 0, '{sort} returns empty list'; + +# [perl #92264] Freeing $a or $b during sort +{ + my $w; + local $SIG{__WARN__} = sub { warn shift; ++ $w }; + eval { + for (1,2) { + @x = sort { *a = \1; 1 } 1, 2; + @x = sort { *b = \1; 1 } 1, 2; + } + }; + is $@, "", 'freeing $a or $b inside sort block'; +}
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 922b
On Sat Jan 14 22:53:05 2012, sprout wrote: Show quoted text
> On Sun Jun 05 15:51:34 2011, sprout wrote:
> > $ perl5.14.0 -e '@_ = sort { *a = \1 } 1, 2' > > Attempt to free unreferenced scalar: SV 0x826480, Perl interpreter: > > 0x800000. > > > > I think pp_sort is misusing SAVESPTR.
> > Attached is a test case. I don’t know how to fix this efficiently. > Currently GvSV(*a) and GvSV(*b) are not reference-counted during sort. > But no other code knows about that. > > Making them reference-counted is the obvious fix, but I presume it was > not done originally for efficency’s sake.
But incrementing and decrementing a reference count should not be too slow, and correctness is definitely a good thing. The attached patch is the obvious fix, but it changes an existing TODO test into an assertion failure. So, this being rare enough, it should probably wait until the stack is reference-counted. -- Father Chrysostomos
To: perl5-porters [...] perl.org
Date: Tue, 12 Dec 2017 07:30:47 +0000
From: Zefram <zefram [...] fysh.org>
Subject: Re: [perl #92264] Freeing $a or $b during sort causes a double free
Download (untitled) / with headers
text/plain 369b
The original test case on this ticket stopped producing the freeing warning in commit 8b0c3377906a6f991cd6c21a674bf9561d85e3cb in 5.25.7. That doesn't reflect any substantive difference relevant to this ticket. This test case still warns: $ perl -le '() = sort { *a = \1 } 1, 2' Attempt to free unreferenced scalar: SV 0x1e4b270, Perl interpreter: 0x1e24010. -zefram
Date: Tue, 12 Dec 2017 10:03:30 +0000
To: perl5-porters [...] perl.org
Subject: Re: [perl #92264] Freeing $a or $b during sort causes a double free
From: Zefram <zefram [...] fysh.org>
Download (untitled) / with headers
text/plain 1.3k
This ticket has been misclassified as a stack-not-refcounted bug; it has nothing to do with the stack. The comparands are on the stack, and if this were a stack-not-refcounted bug then the issue would be the existence of a comparand being undermined, causing it to be freed early and some part of the operation seeing it freed. But in the supplied test case, the comparands are in no danger of being undermined: they're from literals, so are referenced by const ops, and those references are not going away. The issue is that the GvSV slot in globs *is* reference counted, but sort wasn't respecting that reference counting. sort was writing to GvSV without ever diddling reference counting. It got away with this as long as nothing else wrote to GvSV of those globs during a sort, because at the end it restored the original contents. But "*a = \1" performs a refcount-respecting write to GvSV. This messes up the refcount of the comparand, which gets decremented, *and* the refcount of the "1", which gets incremented with the references leaked. By performing a larger sort, with more instances of "*a = \1" executed, it's possible to move refcounts the wrong way repeatedly: the comparand can be subjected to multiple attempts to free it when already freed, and the substitute $a scalar can have its refcount go arbitrarily high from leaked refs. Fixed in commit 16ada235c332e017667585e1a5a00ce43a31c529. -zefram


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org