Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attempt to free unreferenced scalar fiddling with the symbol table #9351

Closed
p5pRT opened this issue May 27, 2008 · 9 comments
Closed

Attempt to free unreferenced scalar fiddling with the symbol table #9351

p5pRT opened this issue May 27, 2008 · 9 comments

Comments

@p5pRT
Copy link

p5pRT commented May 27, 2008

Migrated from rt.perl.org#54934 (status was 'resolved')

Searchable as RT54934$

@p5pRT
Copy link
Author

p5pRT commented May 27, 2008

From @ntyni

This is a bug report for perl from Niko Tyni <ntyni@​debian.org>,
generated with the help of perlbug 1.36 running under perl 5.10.0.


As seen in <http​://cpantesters.perl.org/show/PerlIO-via-dynamic.html>,
the test suite of the PerlIO​::via​::dynamic CPAN module generates
'Attempt to free unreferenced scalar' warnings with recent perl versions,
including 5.10.0 but not 5.8.8.

The warnings are still there on blead @​33937; bisecting shows the warnings
started with

Change 24966 by nicholas@​ship-in-a-bottle on 2005/06/23 21​:30​:33

Remove the reference loop between symbol tables and typeglobs. Typeglobs
now have a weak reference onto their symbol table.

This snippet is the smallest I can make it. The original code deletes
a dynamically created package instead which makes more sense than this.

#!/usr/bin/perl
package test;
sub PUSHED {
  return \*FOO;
}
1;
package main;
{
  open my $fh, '</';
  binmode($fh, '​:via(test)');
}
delete $​::{'test​::'};
__END__



Flags​:
  category=core
  severity=medium


Site configuration information for perl 5.10.0​:

Configured by Debian Project at Thu May 8 11​:57​:24 UTC 2008.

Summary of my perl5 (revision 5 version 10 subversion 0) configuration​:
  Platform​:
  osname=linux, osvers=2.6.18-6-xen-amd64, archname=x86_64-linux-gnu-thread-multi
  uname='linux sid 2.6.18-6-xen-amd64 #1 smp thu apr 24 05​:10​:26 utc 2008 x86_64 gnulinux '
  config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.10 -Darchlib=/usr/lib/perl/5.10 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.10.0 -Dsitearch=/usr/local/lib/perl/5.10.0 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Ud_ualarm -Uusesfio -Uusenm -DDEBUGGING=-g -Doptimize=-O2 -Duseshrplib -Dlibperl=libperl.so.5.10.0 -Dd_dosuid -des'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=define, usemultiplicity=define
  useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O2 -g',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include'
  ccversion='', gccversion='4.2.3 (Debian 4.2.3-5)', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
  ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
  alignbytes=8, prototype=define
  Linker and Libraries​:
  ld='cc', ldflags =' -L/usr/local/lib'
  libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64
  libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
  perllibs=-ldl -lm -lpthread -lc -lcrypt
  libc=/lib/libc-2.7.so, so=so, useshrplib=true, libperl=libperl.so.5.10.0
  gnulibc_version='2.7'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O2 -g -L/usr/local/lib'

Locally applied patches​:
 


@​INC for perl 5.10.0​:
  /etc/perl
  /usr/local/lib/perl/5.10.0
  /usr/local/share/perl/5.10.0
  /usr/lib/perl5
  /usr/share/perl5
  /usr/lib/perl/5.10
  /usr/share/perl/5.10
  /usr/local/lib/site_perl
  .


Environment for perl 5.10.0​:
  HOME=/home/niko
  LANG=en_US.UTF-8
  LANGUAGE (unset)
  LC_CTYPE=fi_FI.UTF-8
  LD_LIBRARY_PATH (unset)
  LOGDIR (unset)
  PATH=/home/niko/bin​:/usr/local/bin​:/usr/bin​:/bin​:/usr/bin/X11​:/usr/games​:/sbin​:/usr/sbin
  PERL_BADLANG (unset)
  SHELL=/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented May 30, 2008

From @rgs

2008/5/27 via RT Niko Tyni <perlbug-followup@​perl.org>​:

As seen in <http​://cpantesters.perl.org/show/PerlIO-via-dynamic.html>,
the test suite of the PerlIO​::via​::dynamic CPAN module generates
'Attempt to free unreferenced scalar' warnings with recent perl versions,
including 5.10.0 but not 5.8.8.

The warnings are still there on blead @​33937; bisecting shows the warnings
started with

Change 24966 by nicholas@​ship-in-a-bottle on 2005/06/23 21​:30​:33

Remove the reference loop between symbol tables and typeglobs. Typeglobs
now have a weak reference onto their symbol table.

This snippet is the smallest I can make it. The original code deletes
a dynamically created package instead which makes more sense than this.

#!/usr/bin/perl
package test;
sub PUSHED {
return \*FOO;
}
1;
package main;
{
open my $fh, '</';
binmode($fh, '​:via(test)');
}
delete $​::{'test​::'};

Good catch. I've traced the double-freed scalar; the patch below fixes
the error. However I'm not sure at all it creates no leak.

Inline Patch
--- ext/PerlIO/via/via.xs.orig	2008-01-25 10:00:53.000000000 +0100
+++ ext/PerlIO/via/via.xs	2008-05-30 14:43:11.000000000 +0200
@@ -89,7 +89,7 @@ PerlIOVia_method(pTHX_ PerlIO * f, const
 	    if (!s->fh) {
 		GV *gv = newGVgen(HvNAME_get(s->stash));
 		GvIOp(gv) = newIO();
-		s->fh = newRV_noinc((SV *) gv);
+		s->fh = newRV((SV *) gv);
 		s->io = GvIOp(gv);
 	    }
 	    IoIFP(s->io) = PerlIONext(f);
End of Patch.

So what leaks is a gv generated to hold the "FOO" reference. (Its name
is "_GEN_0".)

@p5pRT
Copy link
Author

p5pRT commented May 30, 2008

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented May 30, 2008

From alexmv@bestpractical.com

On Fri, 2008-05-30 at 14​:55 +0200, Rafael Garcia-Suarez wrote​:

Good catch. I've traced the double-freed scalar; the patch below fixes
the error. However I'm not sure at all it creates no leak.

This bug also causes segfaults in SVK under 5.10 and blead. Getting
this patch (or whatever it ends up as) into 5.10.1 would be much
appreciated.
  Thanks!
- Alex

@p5pRT
Copy link
Author

p5pRT commented May 30, 2008

From @ntyni

On Fri, May 30, 2008 at 02​:55​:22PM +0200, Rafael Garcia-Suarez wrote​:

2008/5/27 via RT Niko Tyni <perlbug-followup@​perl.org>​:

As seen in <http​://cpantesters.perl.org/show/PerlIO-via-dynamic.html>,
the test suite of the PerlIO​::via​::dynamic CPAN module generates
'Attempt to free unreferenced scalar' warnings with recent perl versions,
including 5.10.0 but not 5.8.8.

Good catch. I've traced the double-freed scalar; the patch below fixes
the error. However I'm not sure at all it creates no leak.

I'm happy to report that the patch fixes all the numerous failures in
the SVK v2.0.2 test suite on Perl 5.10.0.

http​://cpantesters.perl.org/show/SVK.html#SVK-v2.0.2

Many thanks for looking at this.
--
Niko Tyni ntyni@​debian.org

@p5pRT
Copy link
Author

p5pRT commented Jun 6, 2008

From @ntyni

On Fri, May 30, 2008 at 02​:55​:22PM +0200, Rafael Garcia-Suarez wrote​:

2008/5/27 via RT Niko Tyni <perlbug-followup@​perl.org>​:

As seen in <http​://cpantesters.perl.org/show/PerlIO-via-dynamic.html>,
the test suite of the PerlIO​::via​::dynamic CPAN module generates
'Attempt to free unreferenced scalar' warnings with recent perl versions,
including 5.10.0 but not 5.8.8.

Good catch. I've traced the double-freed scalar; the patch below fixes
the error. However I'm not sure at all it creates no leak.

--- ext/PerlIO/via/via.xs.orig 2008-01-25 10​:00​:53.000000000 +0100
+++ ext/PerlIO/via/via.xs 2008-05-30 14​:43​:11.000000000 +0200
@​@​ -89,7 +89,7 @​@​ PerlIOVia_method(pTHX_ PerlIO * f, const
if (!s->fh) {
GV *gv = newGVgen(HvNAME_get(s->stash));
GvIOp(gv) = newIO();
- s->fh = newRV_noinc((SV *) gv);
+ s->fh = newRV((SV *) gv);
s->io = GvIOp(gv);
}
IoIFP(s->io) = PerlIONext(f);
End of Patch.

So what leaks is a gv generated to hold the "FOO" reference. (Its name
is "_GEN_0".)

Hi,

I don't see this applied yet, hope it isn't falling through the cracks.
While PerlIO-via-dynamic-0.13 now has a workaround for 5.10.0, Bastian
Blank raised concerns about possible security implications of the double
frees in <http​://bugs.debian.org/479698> (Cc'd).

Again, many thanks for looking at this.
--
Niko Tyni ntyni@​debian.org

@p5pRT
Copy link
Author

p5pRT commented Jun 6, 2008

From @rgs

2008/6/6 Niko Tyni <ntyni@​debian.org>​:

--- ext/PerlIO/via/via.xs.orig 2008-01-25 10​:00​:53.000000000 +0100
+++ ext/PerlIO/via/via.xs 2008-05-30 14​:43​:11.000000000 +0200
@​@​ -89,7 +89,7 @​@​ PerlIOVia_method(pTHX_ PerlIO * f, const
if (!s->fh) {
GV *gv = newGVgen(HvNAME_get(s->stash));
GvIOp(gv) = newIO();
- s->fh = newRV_noinc((SV *) gv);
+ s->fh = newRV((SV *) gv);
s->io = GvIOp(gv);
}
IoIFP(s->io) = PerlIONext(f);
End of Patch.

So what leaks is a gv generated to hold the "FOO" reference. (Its name
is "_GEN_0".)

Hi,

I don't see this applied yet, hope it isn't falling through the cracks.
While PerlIO-via-dynamic-0.13 now has a workaround for 5.10.0, Bastian
Blank raised concerns about possible security implications of the double
frees in <http​://bugs.debian.org/479698> (Cc'd).

No. Nicholas commented on IRC that this apparently creates no link,
and I'm not able to prove him false, so I'll probably apply this
patch. (or someone could beat me to it)

@p5pRT
Copy link
Author

p5pRT commented Jun 8, 2008

From @rgs

2008/5/30 Rafael Garcia-Suarez <rgarciasuarez@​gmail.com>​:

Good catch. I've traced the double-freed scalar; the patch below fixes
the error. However I'm not sure at all it creates no leak.

I've now applied this and bumped the version of PerlIO​::via, so this
bug can be closed​:

Change 34025 by rgs@​scipion on 2008/06/08 14​:00​:59

  Fix for bug [perl #54934] Attempt to free unreferenced scalar
fiddling with the symbol table
  Keep the refcount of the globs generated by PerlIO​::via balanced.

Affected files ...

... //depot/perl/ext/PerlIO/via/via.pm#9 edit
... //depot/perl/ext/PerlIO/via/via.xs#17 edit

Differences ...

==== //depot/perl/ext/PerlIO/via/via.pm#9 (text) ====

@​@​ -1,5 +1,5 @​@​
package PerlIO​::via;
-our $VERSION = '0.05';
+our $VERSION = '0.06';
use XSLoader ();
XSLoader​::load 'PerlIO​::via';
1;

==== //depot/perl/ext/PerlIO/via/via.xs#17 (text) ====

@​@​ -89,7 +89,7 @​@​
  if (!s->fh) {
  GV *gv = newGVgen(HvNAME_get(s->stash));
  GvIOp(gv) = newIO();
- s->fh = newRV_noinc((SV *) gv);
+ s->fh = newRV((SV *) gv);
  s->io = GvIOp(gv);
  }
  IoIFP(s->io) = PerlIONext(f);

@p5pRT
Copy link
Author

p5pRT commented Jun 8, 2008

@rgs - Status changed from 'open' to 'resolved'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant