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

uninitialized lex as filehandle segmentation fault #780

Closed
p5pRT opened this issue Oct 24, 1999 · 10 comments
Closed

uninitialized lex as filehandle segmentation fault #780

p5pRT opened this issue Oct 24, 1999 · 10 comments

Comments

@p5pRT
Copy link

p5pRT commented Oct 24, 1999

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

Searchable as RT1705$

@p5pRT
Copy link
Author

p5pRT commented Oct 24, 1999

From skimo@kotnet.org

Using an unitialized lexical as a filehandle can result in
a segmentation fault.
I'm sorry for not having a sample program, but I've
identified the guilty parties.

in pp_rv2gv in pp.c
  GV *gv = (GV *) newSV(0);
  STRLEN len = 0;
  char *name = "";
  if (cUNOP->op_first->op_type == OP_PADSV) {
  SV *padname = *av_fetch(PL_comppad_name, cUNOP->op_first
->op_targ, 4);
  name = SvPV(padname,len);

This code creates an sv with a zero SvPVX.

Later on, in S_pad_findlex in op.c,
this results in a segmentation fault.

  if ((sv = svp[off]) &&
  sv != &PL_sv_undef &&
  seq <= SvIVX(sv) &&
  seq > I_32(SvNVX(sv)) &&
  strEQ(SvPVX(sv), name))

skimo

Perl Info


Site configuration information for perl 5.00562:

Configured by skimo at Fri Oct 15 20:57:29 CEST 1999.

Summary of my perl5 (revision 5.0 version 5 subversion 62) configuration:
  Platform:
    osname=linux, osvers=2.1.125, archname=i586-linux-thread
    uname='linux pool 2.1.125 #24 fri oct 16 13:44:56 mest 1998 i586 unknown '
    config_args='-Doptimize=-g -Dusethreads -Duseshrplib -Dprefix=/usr -des'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=define useperlio=undef d_sfio=undef
    use64bits=undef usemultiplicity=undef
  Compiler:
    cc='cc', optimize='-g', gccversion=egcs-2.91.60 19981201 (egcs-1.1.1 release)
    cppflags='-D_REENTRANT -Dbool=char -DHAS_BOOL -DDEBUGGING -fno-strict-aliasing -I/usr/local/include'
    ccflags ='-D_REENTRANT -Dbool=char -DHAS_BOOL -DDEBUGGING -fno-strict-aliasing -I/usr/local/include'
    stdchar='char', d_stdstdio=define, usevfork=false
    intsize=4, longsize=4, ptrsize=4, doublesize=8
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
    alignbytes=4, usemymalloc=n, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lnsl -lndbm -ldb -ldl -lm -lpthread -lc -lposix -lcrypt
    libc=/lib/libc-2.1.2.so, so=so, useshrplib=true, libperl=libperl.so
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic -Wl,-rpath,/usr/lib/perl5/5.00562/i586-linux-thread/CORE'
    cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    


@INC for perl 5.00562:
    /usr/lib/perl5/5.00562/i586-linux-thread
    /usr/lib/perl5/5.00562
    /usr/lib/site_perl/5.00562/i586-linux-thread
    /usr/lib/site_perl
    .


Environment for perl 5.00562:
    HOME=/home/skimo
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/skimo/bin:/usr/local/bin:/usr/bin:/usr/X11R6/bin:/bin:/usr/openwin/bin:/usr/games/bin:/usr/games:.
    PERL_BADLANG (unset)
    SHELL=/usr/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Oct 24, 1999

From [Unknown Contact. See original ticket]

<skimo@​pool.gv.kotnet.org> writes​:

This is a bug report for perl from skimo@​pool.gv.kotnet.org,
generated with the help of perlbug 1.27 running under perl 5.00562.

-----------------------------------------------------------------
[Please enter your report here]

Using an unitialized lexical as a filehandle can result in
a segmentation fault.
I'm sorry for not having a sample program, but I've
identified the guilty parties.

in pp_rv2gv in pp.c
GV *gv = (GV *) newSV(0);
STRLEN len = 0;
char *name = "";
if (cUNOP->op_first->op_type == OP_PADSV) {
SV *padname = *av_fetch(PL_comppad_name, cUNOP->op_first
->op_targ, 4);
name = SvPV(padname,len);

Hmm, guilty as charged :-(
I claim in mitigation that I did not know that the PADSV could be nameless.

The "fix" for the "handling stolen goods" charge is easy - check for NULL
and leave name = "" in that case. The question I have is what can cause
a nameless PADSV? Is that itself a bug or a feature?

The idea was that when vivifying file handles we give them a name so that
die/warn report things like "... at <$a> line 42 ...". I copied the code
from somewhere - so there is another bug lurking where I got it (unless
that place had more guards round the snippet).

This code creates an sv with a zero SvPVX.

Later on, in S_pad_findlex in op.c,
this results in a segmentation fault.

       if \(\(sv = svp\[off\]\) &&
           sv \!= &PL\_sv\_undef &&
           seq \<= SvIVX\(sv\) &&
           seq > I\_32\(SvNVX\(sv\)\) &&
           strEQ\(SvPVX\(sv\)\, name\)\)

skimo

[Please do not change anything below this line]
-----------------------------------------------------------------

---
Site configuration information for perl 5.00562​:

Configured by skimo at Fri Oct 15 20​:57​:29 CEST 1999.

Summary of my perl5 (revision 5.0 version 5 subversion 62) configuration​:
Platform​:
osname=linux, osvers=2.1.125, archname=i586-linux-thread
uname='linux pool 2.1.125 #24 fri oct 16 13​:44​:56 mest 1998 i586 unknown '
config_args='-Doptimize=-g -Dusethreads -Duseshrplib -Dprefix=/usr -des'
hint=recommended, useposix=true, d_sigaction=define
usethreads=define useperlio=undef d_sfio=undef
use64bits=undef usemultiplicity=undef
Compiler​:
cc='cc', optimize='-g', gccversion=egcs-2.91.60 19981201 (egcs-1.1.1 release)
cppflags='-D_REENTRANT -Dbool=char -DHAS_BOOL -DDEBUGGING -fno-strict-aliasing -I/usr/local/include'
ccflags ='-D_REENTRANT -Dbool=char -DHAS_BOOL -DDEBUGGING -fno-strict-aliasing -I/usr/local/include'
stdchar='char', d_stdstdio=define, usevfork=false
intsize=4, longsize=4, ptrsize=4, doublesize=8
d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
alignbytes=4, usemymalloc=n, prototype=define
Linker and Libraries​:
ld='cc', ldflags =' -L/usr/local/lib'
libpth=/usr/local/lib /lib /usr/lib
libs=-lnsl -lndbm -ldb -ldl -lm -lpthread -lc -lposix -lcrypt
libc=/lib/libc-2.1.2.so, so=so, useshrplib=true, libperl=libperl.so
Dynamic Linking​:
dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-rdynamic -Wl,-rpath,/usr/lib/perl5/5.00562/i586-linux-thread/CORE'
cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'

Locally applied patches​:

---
@​INC for perl 5.00562​:
/usr/lib/perl5/5.00562/i586-linux-thread
/usr/lib/perl5/5.00562
/usr/lib/site_perl/5.00562/i586-linux-thread
/usr/lib/site_perl
.

---
Environment for perl 5.00562​:
HOME=/home/skimo
LANG (unset)
LANGUAGE (unset)
LD_LIBRARY_PATH (unset)
LOGDIR (unset)
PATH=/home/skimo/bin​:/usr/local/bin​:/usr/bin​:/usr/X11R6/bin​:/bin​:/usr/openwin/bin​:/usr/games/bin​:/usr/games​:.
PERL_BADLANG (unset)
SHELL=/usr/bin/zsh

Message from the perl bug squashing team at 'perlbug@​perl.org'
--
Nick Ing-Simmons

@p5pRT
Copy link
Author

p5pRT commented Oct 24, 1999

From [Unknown Contact. See original ticket]

On Sun, Oct 24, 1999 at 10​:43​:32PM +0100, Nick Ing-Simmons wrote​:

in pp_rv2gv in pp.c
GV *gv = (GV *) newSV(0);
STRLEN len = 0;
char *name = "";
if (cUNOP->op_first->op_type == OP_PADSV) {
SV *padname = *av_fetch(PL_comppad_name, cUNOP->op_first
->op_targ, 4);
name = SvPV(padname,len);

Hmm, guilty as charged :-(
I claim in mitigation that I did not know that the PADSV could be nameless.

The "fix" for the "handling stolen goods" charge is easy - check for NULL
and leave name = "" in that case. The question I have is what can cause
a nameless PADSV? Is that itself a bug or a feature?

Well, the code looks something like

sub {
  my $var;
  open($var, "file");
  ...
}

I haven't been able to strip my program down to just the code
needed to exhibit the bug.

I can of course do some further debugging if you can tell me
where to look.

skimo

@p5pRT
Copy link
Author

p5pRT commented Oct 24, 1999

From [Unknown Contact. See original ticket]

Sven Verdoolaege <skimo@​kotnet.org> writes​:

On Sun, Oct 24, 1999 at 10​:43​:32PM +0100, Nick Ing-Simmons wrote​:

in pp_rv2gv in pp.c
GV *gv = (GV *) newSV(0);
STRLEN len = 0;
char *name = "";
if (cUNOP->op_first->op_type == OP_PADSV) {
SV *padname = *av_fetch(PL_comppad_name, cUNOP->op_first
->op_targ, 4);
name = SvPV(padname,len);

Hmm, guilty as charged :-(
I claim in mitigation that I did not know that the PADSV could be nameless.

The "fix" for the "handling stolen goods" charge is easy - check for NULL
and leave name = "" in that case. The question I have is what can cause
a nameless PADSV? Is that itself a bug or a feature?

Well, the code looks something like

sub {
my $var;
open($var, "file");
...
}

That is exactly the case this is supposed to handle.

Sarathy - I will check-in code that tests for NULL.

Meanwhile can other p5p folk tell me when this can occur?
e.g. does optimizer create name-less temporaries?

--
Nick Ing-Simmons

@p5pRT
Copy link
Author

p5pRT commented Oct 24, 1999

From [Unknown Contact. See original ticket]

Sven Verdoolaege <skimo@​kotnet.org> writes​:

On Sun, Oct 24, 1999 at 10​:43​:32PM +0100, Nick Ing-Simmons wrote​:

in pp_rv2gv in pp.c
GV *gv = (GV *) newSV(0);
STRLEN len = 0;
char *name = "";
if (cUNOP->op_first->op_type == OP_PADSV) {
SV *padname = *av_fetch(PL_comppad_name, cUNOP->op_first
->op_targ, 4);
name = SvPV(padname,len);

Hmm, guilty as charged :-(
I claim in mitigation that I did not know that the PADSV could be nameless.

The "fix" for the "handling stolen goods" charge is easy - check for NULL
and leave name = "" in that case. The question I have is what can cause
a nameless PADSV? Is that itself a bug or a feature?

Attached is the patch corresponding to the change I just made to the
mainline.

--
Nick Ing-Simmons

@p5pRT
Copy link
Author

p5pRT commented Oct 24, 1999

From [Unknown Contact. See original ticket]

patch

@p5pRT
Copy link
Author

p5pRT commented Oct 24, 1999

From [Unknown Contact. See original ticket]

On Sun, Oct 24, 1999 at 11​:13​:17PM +0100, Nick Ing-Simmons wrote​:

Attached is the patch corresponding to the change I just made to the
mainline.

Inline Patch
--- perl/pp.c.~1~       Sun Oct 24 22:10:16 1999
+++ perl/pp.c   Sun Oct 24 22:10:16 1999
@@ -245,8 +245,14 @@
                    STRLEN len = 0;
                    char *name = "";
                    if (cUNOP->op_first->op_type == OP_PADSV) {
-                       SV *padname = *av_fetch(PL_comppad_name,
cUNOP->op_first->op_targ, 4); \- name = SvPV\(padname\,len\); \+ SV \*\*namep = av\_fetch\(PL\_comppad\_name\, cUNOP\->op\_first\->op\_targ\, 4\); \+ if \(namep && \*namep\) \{ \+ name = SvPV\(\*namep\,len\); \+ if \(\!name\) \{ \+ name = ""; \+ len = 0; \+ \} \+ \}   \}   gv\_init\(gv\, PL\_curcop\->cop\_stash\, name\, len\, 0\);   sv\_upgrade\(sv\, SVt\_RV\);

How is that supposed to help.
padname wasn't 0, zo the if will succeed and then
the same code is executed as in the original.
It's the SvPV that creates the zero pv.
You even check for it !

skimo

@p5pRT
Copy link
Author

p5pRT commented Oct 24, 1999

From [Unknown Contact. See original ticket]

Sven Verdoolaege <skimo@​kotnet.org> writes​:

On Sun, Oct 24, 1999 at 11​:13​:17PM +0100, Nick Ing-Simmons wrote​:

Attached is the patch corresponding to the change I just made to the
mainline.

--- perl/pp.c.1 Sun Oct 24 22​:10​:16 1999
+++ perl/pp.c Sun Oct 24 22​:10​:16 1999
@​@​ -245,8 +245,14 @​@​
STRLEN len = 0;
char *name = "";
if (cUNOP->op_first->op_type == OP_PADSV) {
- SV *padname = *av_fetch(PL_comppad_name,
cUNOP->op_first->op_targ, 4);
- name = SvPV(padname,len);
+ SV **namep = av_fetch(PL_comppad_name,
cUNOP->op_first->op_targ, 4);
+ if (namep && *namep) {
+ name = SvPV(*namep,len);
+ if (!name) {
+ name = "";
+ len = 0;
+ }
+ }
}
gv_init(gv, PL_curcop->cop_stash, name, len, 0);
sv_upgrade(sv, SVt_RV);

How is that supposed to help.
padname wasn't 0,

That was extra check while I was there.

zo the if will succeed and then
the same code is executed as in the original.

It's the SvPV that creates the zero pv.

Okay - I understand the problem a bit better. But I do not see how
rv2gv is to "blame" yet - SvPV will only set a NULL PV if
Asking for SvPV 'upgrades' the sv and sets its PV.
As I understand the code in findlex() when it creates one of these
special SVs it should set the PV. And so by time rv2gv sees it there
should be no need to NULL it.

Now I am out of my depth - the SVs that S_pad_findlex() scan are special.
Upgrading them is itself probably the wrong thing to do as their IVX and NVX
fields are being abused ;-)

The question is what created the "unamed" PADSV in the first place,
or where did it get mangled so that SvPV re-sets its PV ?

You even check for it !

That was just more extra-safety.

skimo
--
Nick Ing-Simmons

@p5pRT
Copy link
Author

p5pRT commented Oct 24, 1999

From [Unknown Contact. See original ticket]

On Mon, Oct 25, 1999 at 01​:09​:52AM +0100, Nick Ing-Simmons wrote​:

Sven Verdoolaege <skimo@​kotnet.org> writes​:

On Sun, Oct 24, 1999 at 11​:13​:17PM +0100, Nick Ing-Simmons wrote​:

Attached is the patch corresponding to the change I just made to the
mainline.

--- perl/pp.c.1 Sun Oct 24 22​:10​:16 1999
+++ perl/pp.c Sun Oct 24 22​:10​:16 1999
@​@​ -245,8 +245,14 @​@​
STRLEN len = 0;
char *name = "";
if (cUNOP->op_first->op_type == OP_PADSV) {
- SV *padname = *av_fetch(PL_comppad_name,
cUNOP->op_first->op_targ, 4);
- name = SvPV(padname,len);
+ SV **namep = av_fetch(PL_comppad_name,
cUNOP->op_first->op_targ, 4);
+ if (namep && *namep) {
+ name = SvPV(*namep,len);
+ if (!name) {
+ name = "";
+ len = 0;
+ }
+ }
}
gv_init(gv, PL_curcop->cop_stash, name, len, 0);
sv_upgrade(sv, SVt_RV);

so the if will succeed and then
the same code is executed as in the original.

It's the SvPV that creates the zero pv.

Okay - I understand the problem a bit better. But I do not see how
rv2gv is to "blame" yet - SvPV will only set a NULL PV if
Asking for SvPV 'upgrades' the sv and sets its PV.
As I understand the code in findlex() when it creates one of these
special SVs it should set the PV. And so by time rv2gv sees it there
should be no need to NULL it.

Maybe I should've mentioned that the av entry didn't exist.
It's the av_fetch that creates it. (So I think your extra check
is useless.) Where should it have been created ?

skimo

@p5pRT
Copy link
Author

p5pRT commented Oct 24, 1999

From @gsar

On Mon, 25 Oct 1999 01​:09​:52 BST, Nick Ing-Simmons wrote​:

Okay - I understand the problem a bit better. But I do not see how
rv2gv is to "blame" yet - SvPV will only set a NULL PV if
Asking for SvPV 'upgrades' the sv and sets its PV.
As I understand the code in findlex() when it creates one of these
special SVs it should set the PV. And so by time rv2gv sees it there
should be no need to NULL it.

Now I am out of my depth - the SVs that S_pad_findlex() scan are special.
Upgrading them is itself probably the wrong thing to do as their IVX and NVX
fields are being abused ;-)

The question is what created the "unamed" PADSV in the first place,
or where did it get mangled so that SvPV re-sets its PV ?

I'm rather circumspect about groping through PL_comppad_name
at arbitrary points at run time. Note that everything else that
does that does it before lex_end().

My guess would be free_closures() is what's biting you.

Sarathy
gsar@​ActiveState.com

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