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

assertion failures when running with -t switch and tainted @INC #10105

Closed
p5pRT opened this issue Jan 24, 2010 · 10 comments
Closed

assertion failures when running with -t switch and tainted @INC #10105

p5pRT opened this issue Jan 24, 2010 · 10 comments

Comments

@p5pRT
Copy link

p5pRT commented Jan 24, 2010

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

Searchable as RT72330$

@p5pRT
Copy link
Author

p5pRT commented Jan 24, 2010

From @ntyni

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


A suitable combination of tainted @​INC, the '-t' switch and stubs for
undefined constants causes an assertion failure with -DDEBUGGING and
intermittent segmentation faults otherwise.

Originally reported as http​://bugs.debian.org/565703, reproducible
on current bleadperl​:

% ./perl -t -Ilib -e 'unshift @​INC, shift; require Fcntl' foo
Insecure dependency in require while running with -t switch at -e line 1.
Insecure dependency in require while running with -t switch at -e line 1.
Insecure dependency in require while running with -t switch at lib/Fcntl.pm line 58.
Insecure dependency in require while running with -t switch at lib/Fcntl.pm line 58.
Insecure dependency in require while running with -t switch at lib/Fcntl.pm line 58.
Insecure dependency in require while running with -t switch at lib/Fcntl.pm line 62.
Insecure dependency in require while running with -t switch at lib/Fcntl.pm line 62.
Insecure dependency in require while running with -t switch at lib/Fcntl.pm line 62.
perl​: gv.c​:241​: Perl_gv_init​: Assertion `!((gv)->sv_flags & 0x00004000)' failed.
zsh​: abort (core dumped) ./perl -t -Ilib -e 'unshift @​INC, shift; require Fcntl' foo

Other variants include

% echo 'XSLoader​::load(q(Fcntl))' > T.pm && ./perl -Ilib -MXSLoader -I. -t -e 'unshift @​INC, shift; require T; *a = \&{"Fcntl​::S_IFWHT"}' foo
Insecure dependency in require while running with -t switch at -e line 1.
[...]
perl​: mg.c​:219​: Perl_mg_get​: Assertion `!(((_svmagic)->sv_flags & (0x40000000|0x00040000)) == (0x40000000|0x00040000))' failed.

and

% echo 'XSLoader​::load(q(POSIX))' > T.pm && ./perl -Ilib -MXSLoader -I. -t -e 'unshift @​INC, shift; require T; *a = \&{"POSIX​::CLK_TCK"}' foo

which fails in the same way.

The problem seems to be related to the stubs for undefined constants;
the ones used above are not defined on this platform.

./perl -Ilib -MPOSIX -E 'say POSIX​::CLK_TCK'
Your vendor has not defined POSIX macro CLK_TCK, used at -e line 1
./perl -Ilib -MFcntl -E 'say Fcntl​::S_IFWHT'
Your vendor has not defined Fcntl macro S_IFWHT, used at -e line 1.

Blead backtrace of the first case​:

Core was generated by `./perl -t -Ilib -e unshift @​INC, shift; require Fcntl foo'.
Program terminated with signal 6, Aborted.
#0 0x00007f86090b5f55 in *__GI_raise (sig=<value optimized out>)
  at ../nptl/sysdeps/unix/sysv/linux/raise.c​:64
64 ../nptl/sysdeps/unix/sysv/linux/raise.c​: No such file or directory.
  in ../nptl/sysdeps/unix/sysv/linux/raise.c
(gdb) bt
#0 0x00007f86090b5f55 in *__GI_raise (sig=<value optimized out>)
  at ../nptl/sysdeps/unix/sysv/linux/raise.c​:64
#1 0x00007f86090b8d90 in *__GI_abort () at abort.c​:88
#2 0x00007f86090af07a in *__GI___assert_fail (assertion=0x74b9d8 "!((gv)->sv_flags & 0x00004000)",
  file=<value optimized out>, line=241, function=0x7524df "Perl_gv_init") at assert.c​:78
#3 0x0000000000464e44 in Perl_gv_init (my_perl=0xd20010, gv=0xd7ab10, stash=0xd42898,
  name=0xd88528 "S_IFWHT", len=7, multi=2) at gv.c​:241
#4 0x000000000046c902 in Perl_gv_fetchpvn_flags (my_perl=0xd20010, nambeg=0xd88528 "S_IFWHT",
  full_len=7, flags=3, sv_type=SVt_PVCV) at gv.c​:1142
#5 0x000000000046af9b in Perl_gv_fetchsv (my_perl=0xd20010, name=0xd885c0, flags=3, sv_type=SVt_PVCV)
  at gv.c​:911
#6 0x000000000043eec0 in Perl_ck_rvconst (my_perl=0xd20010, o=0xd89670) at op.c​:6788
#7 0x000000000042c343 in Perl_newUNOP (my_perl=0xd20010, type=17, flags=0, first=0xd89630) at op.c​:3109
#8 0x000000000043d7b4 in Perl_newCVREF (my_perl=0xd20010, flags=0, o=0xd89630) at op.c​:6393
#9 0x00000000004991f2 in Perl_yylex (my_perl=0xd20010) at toke.c​:6056
#10 0x00000000004c0fd9 in Perl_yyparse (my_perl=0xd20010) at perly.c​:414
#11 0x000000000065280f in S_doeval (my_perl=0xd20010, gimme=2, startop=0x0, outside=0x0, seq=0)
  at pp_ctl.c​:3089
#12 0x0000000000659836 in Perl_pp_require (my_perl=0xd20010) at pp_ctl.c​:3657
#13 0x0000000000518cb7 in Perl_runops_debug (my_perl=0xd20010) at dump.c​:2049
#14 0x00000000004548ea in S_run_body (my_perl=0xd20010, oldscope=1) at perl.c​:2308
#15 0x0000000000453bb2 in perl_run (my_perl=0xd20010) at perl.c​:2233
#16 0x000000000042302d in main (argc=6, argv=0x7fffb80bc108, env=0x7fffb80bc140) at perlmain.c​:117



Flags​:
  category=core
  severity=low


Site configuration information for perl 5.11.4​:

Configured by niko at Sun Jan 24 20​:28​:38 EET 2010.

Summary of my perl5 (revision 5 version 11 subversion 4) configuration​:
  Commit id​: fe61459
  Platform​:
  osname=linux, osvers=2.6.32-trunk-amd64, archname=x86_64-linux-gnu-thread-multi
  uname='linux madeleine 2.6.32-trunk-amd64 #1 smp sun jan 10 22​:40​:40 utc 2010 x86_64 gnulinux '
  config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.11 -Darchlib=/usr/lib/perl/5.11 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.11.4 -Dsitearch=/usr/local/lib/perl/5.11.4 -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=both -Doptimize=-O0 -Dusedevel -Uuseshrplib -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 -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-O0 -g',
  cppflags='-D_REENTRANT -D_GNU_SOURCE -DDEBIAN -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
  ccversion='', gccversion='4.4.3 20100108 (prerelease)', 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 =' -fstack-protector -L/usr/local/lib'
  libpth=/usr/local/lib /lib /usr/lib /lib64 /usr/lib64
  libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc -lgdbm_compat
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
  libc=/lib/libc-2.10.2.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.10.2'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -O0 -g -L/usr/local/lib -fstack-protector'

Locally applied patches​:
 


@​INC for perl 5.11.4​:
  lib
  /usr/local/lib/perl/5.11.4
  /usr/local/share/perl/5.11.4
  /usr/lib/perl5
  /usr/share/perl5
  /usr/lib/perl/5.11
  /usr/share/perl/5.11
  .


Environment for perl 5.11.4​:
  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​:/home/niko/bin​:/home/niko/bin​:/usr/local/bin​:/usr/bin​:/bin​:/usr/local/games​:/usr/games​:/sbin​:/usr/sbin​:/sbin​:/usr/sbin
  PERL_BADLANG (unset)
  SHELL=/bin/zsh

@p5pRT
Copy link
Author

p5pRT commented Jan 25, 2010

From @iabyn

On Sun, Jan 24, 2010 at 11​:02​:46AM -0800, Niko Tyni wrote​:

perl​: gv.c​:241​: Perl_gv_init​: Assertion `!((gv)->sv_flags & 0x00004000)' failed.

This appears to have been introduced in 5.10.0

--
The Enterprise is captured by a vastly superior alien intelligence which
does not put them on trial.
  -- Things That Never Happen in "Star Trek" #10

@p5pRT
Copy link
Author

p5pRT commented Jan 25, 2010

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

@p5pRT
Copy link
Author

p5pRT commented Feb 9, 2010

From @iabyn

On Sun, Jan 24, 2010 at 11​:02​:46AM -0800, Niko Tyni wrote​:

Other variants include

% echo 'XSLoader​::load(q(Fcntl))' > T.pm && ./perl -Ilib -MXSLoader -I. -t -e 'unshift @​INC, shift; require T; *a = \&{"Fcntl​::S_IFWHT"}' foo
Insecure dependency in require while running with -t switch at -e line 1.
[...]
perl​: mg.c​:219​: Perl_mg_get​: Assertion `!(((_svmagic)->sv_flags & (0x40000000|0x00040000)) == (0x40000000|0x00040000))' failed.

I can trigger the same assertion failure with the following simplified
code​:

$ cat /tmp/Foo.pm
package Foo;
our $foo; sub f { $foo }
1;

$ ./perl -t -I/tmp -e 'unshift @​INC, $^X; require Foo'
Insecure dependency in require while running with -t switch at -e line 1.
Insecure dependency in require while running with -t switch at -e line 1.
perl​: mg.c​:219​: Perl_mg_get​: Assertion `!(((_svmagic)->sv_flags & (0x40000000|0x00040000)) == (0x40000000|0x00040000))' failed.
Aborted

It boils down to​:
having a tainted @​INC then doing a require with -t (rather than -T)
prints the taint warning, then proceeds to compile the whole of Foo.pm
with PL_tainted=1. This means that everything during the compilation gets
tainted, in particular, the name SVs in pads, so they get taint magic
attached. This then fails the assertion added by Nicholas​:

  commit 885ffcb
  Author​: Nicholas Clark <nick@​ccl4.org>
  AuthorDate​: Tue May 2 17​:00​:56 2006 +0000
  Commit​: Nicholas Clark <nick@​ccl4.org>
  CommitDate​: Tue May 2 17​:00​:56 2006 +0000

  Assert that SvMAGIC() isn't being called on PVMGs which are using the
  same union to store the stash for our.

  ...

  @​@​ -1164,6 +1164,8 @​@​ the scalar's value cannot change unless written to.
  # define SvMAGIC(sv) \
  (*({ SV *const _svi = (SV *) sv; \
  assert(SvTYPE(_svi) >= SVt_PVMG); \
  + if(SvTYPE(_svi) == SVt_PVMG) \
  + assert(!SvPAD_OUR(_svi)); \

I'm assuming, Nicholas, that this assertion is correct, and thus its the
names SVs getting tainted that is BAD and needs fixing?

In which case, its a bit unclear to me what to do. Doing a whole compi;e
tainted is a bit of a strange things to do. Arguably if a require or
similar tries to compile something tainted under -t, we could print the
warning, then reset PL_tainting so the compilation is done untainted.
Which might suppress some further taint warnings, but would stop the
crashes.

I would speculate that the other reported assertion failures have a
similar root cause.

Anyway, probably not something for 5.12, even though its a 5.10 regression.

--
Modern art​:
  "It's easy, I could have done that!"
  "Yes, but you didn't"

@p5pRT
Copy link
Author

p5pRT commented Feb 9, 2010

From @rgarcia

On 9 February 2010 01​:17, Dave Mitchell <davem@​iabyn.com> wrote​:

On Sun, Jan 24, 2010 at 11​:02​:46AM -0800, Niko Tyni wrote​:

Other variants include

% echo 'XSLoader​::load(q(Fcntl))' > T.pm && ./perl -Ilib -MXSLoader -I. -t -e 'unshift @​INC, shift; require T; *a = \&{"Fcntl​::S_IFWHT"}' foo
Insecure dependency in require while running with -t switch at -e line 1.
[...]
perl​: mg.c​:219​: Perl_mg_get​: Assertion `!(((_svmagic)->sv_flags & (0x40000000|0x00040000)) == (0x40000000|0x00040000))' failed.

I can trigger the same assertion failure with the following simplified
code​:

$ cat /tmp/Foo.pm
package Foo;
our $foo; sub f { $foo }
1;

$ ./perl -t -I/tmp -e 'unshift @​INC, $^X; require Foo'
Insecure dependency in require while running with -t switch at -e line 1.
Insecure dependency in require while running with -t switch at -e line 1.
perl​: mg.c​:219​: Perl_mg_get​: Assertion `!(((_svmagic)->sv_flags & (0x40000000|0x00040000)) == (0x40000000|0x00040000))' failed.
Aborted

It boils down to​:
having a tainted @​INC then doing a require with -t (rather than -T)
prints the taint warning, then proceeds to compile the whole of Foo.pm
with PL_tainted=1. This means that everything during the compilation gets
tainted, in particular, the name SVs in pads, so they get taint magic
attached. This then fails the assertion added by Nicholas​:

   commit 885ffcb
   Author​:     Nicholas Clark <nick@​ccl4.org>
   AuthorDate​: Tue May 2 17​:00​:56 2006 +0000
   Commit​:     Nicholas Clark <nick@​ccl4.org>
   CommitDate​: Tue May 2 17​:00​:56 2006 +0000

       Assert that SvMAGIC() isn't being called on PVMGs which are using the
       same union to store the stash for our.

   ...

   @​@​ -1164,6 +1164,8 @​@​ the scalar's value cannot change unless written to.
    #    define SvMAGIC(sv)                                                    \
           (*({ SV *const _svi = (SV *) sv;                            \
               assert(SvTYPE(_svi) >= SVt_PVMG);                               \
   +       if(SvTYPE(_svi) == SVt_PVMG)                                \
   +           assert(!SvPAD_OUR(_svi));                               \

I'm assuming, Nicholas, that this assertion is correct, and thus its the
names SVs getting tainted that is BAD and needs fixing?

In which case, its a bit unclear to me what to do. Doing a whole compi;e
tainted is a bit of a strange things to do. Arguably if a require or
similar tries to compile something tainted under -t, we could print the
warning, then reset PL_tainting so the compilation is done untainted.
Which might suppress some further taint warnings, but would stop the
crashes.

Yes, that sounds like a reasonable line in the sand to draw. Code
loaded via a tainted @​INC could be considered tainted, but for the
sake of only reporting warnings, we should choose the practical side.

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2010

From @iabyn

On Tue, Feb 09, 2010 at 12​:52​:48PM +0100, Rafael Garcia-Suarez wrote​:

On 9 February 2010 01​:17, Dave Mitchell <davem@​iabyn.com> wrote​:

On Sun, Jan 24, 2010 at 11​:02​:46AM -0800, Niko Tyni wrote​:

Other variants include

% echo 'XSLoader​::load(q(Fcntl))' > T.pm && ./perl -Ilib -MXSLoader -I. -t -e 'unshift @​INC, shift; require T; *a = \&{"Fcntl​::S_IFWHT"}' foo
Insecure dependency in require while running with -t switch at -e line 1.
[...]
perl​: mg.c​:219​: Perl_mg_get​: Assertion `!(((_svmagic)->sv_flags & (0x40000000|0x00040000)) == (0x40000000|0x00040000))' failed.

(note to self) This might also be related to

  http​://www.perlmonks.org/?node_id=822374

So I'll need to check that too when I get round to fixing it.

--
It's not that I'm afraid to die, I just don't want to be there when it
happens.
  -- Woody Allen

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2010

From @nwc10

On Tue, Feb 09, 2010 at 12​:52​:48PM +0100, Rafael Garcia-Suarez wrote​:

It boils down to​:
having a tainted @​INC then doing a require with -t (rather than -T)
prints the taint warning, then proceeds to compile the whole of Foo.pm
with PL_tainted=1. This means that everything during the compilation gets
tainted, in particular, the name SVs in pads, so they get taint magic
attached. This then fails the assertion added by Nicholas​:

   commit 885ffcb
   Author​:     Nicholas Clark <nick@​ccl4.org>
   AuthorDate​: Tue May 2 17​:00​:56 2006 +0000
   Commit​:     Nicholas Clark <nick@​ccl4.org>
   CommitDate​: Tue May 2 17​:00​:56 2006 +0000

       Assert that SvMAGIC() isn't being called on PVMGs which are using the
       same union to store the stash for our.

   ...

   @​@​ -1164,6 +1164,8 @​@​ the scalar's value cannot change unless written to.
    #    define SvMAGIC(sv)                                                    \
           (*({ SV *const _svi = (SV *) sv;                            \
               assert(SvTYPE(_svi) >= SVt_PVMG);                               \
   +       if(SvTYPE(_svi) == SVt_PVMG)                                \
   +           assert(!SvPAD_OUR(_svi));                               \

I'm assuming, Nicholas, that this assertion is correct, and thus its the
names SVs getting tainted that is BAD and needs fixing?

Yes, and yes.

Otherwise you end up with pointers which half the core code thinks point
to MAGIC structures, and half the code thinks points to HVx.

In which case, its a bit unclear to me what to do. Doing a whole compi;e
tainted is a bit of a strange things to do. Arguably if a require or
similar tries to compile something tainted under -t, we could print the
warning, then reset PL_tainting so the compilation is done untainted.
Which might suppress some further taint warnings, but would stop the
crashes.

Yes, that sounds like a reasonable line in the sand to draw. Code
loaded via a tainted @​INC could be considered tainted, but for the
sake of only reporting warnings, we should choose the practical side.

This seems like the best solution.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Apr 29, 2016

From @ntyni

On Sun, Jan 24, 2010 at 11​:02​:46AM -0800, Niko Tyni wrote​:

# New Ticket Created by Niko Tyni
# Please include the string​: [perl #72330]
# in the subject line of all future correspondence about this issue.
# <URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=72330 >

A suitable combination of tainted @​INC, the '-t' switch and stubs for
undefined constants causes an assertion failure with -DDEBUGGING and
intermittent segmentation faults otherwise.

Originally reported as http​://bugs.debian.org/565703, reproducible
on current bleadperl​:

% ./perl -t -Ilib -e 'unshift @​INC, shift; require Fcntl' foo
Insecure dependency in require while running with -t switch at -e line 1.
Insecure dependency in require while running with -t switch at -e line 1.
Insecure dependency in require while running with -t switch at lib/Fcntl.pm line 58.
Insecure dependency in require while running with -t switch at lib/Fcntl.pm line 58.
Insecure dependency in require while running with -t switch at lib/Fcntl.pm line 58.
Insecure dependency in require while running with -t switch at lib/Fcntl.pm line 62.
Insecure dependency in require while running with -t switch at lib/Fcntl.pm line 62.
Insecure dependency in require while running with -t switch at lib/Fcntl.pm line 62.
perl​: gv.c​:241​: Perl_gv_init​: Assertion `!((gv)->sv_flags & 0x00004000)' failed.
zsh​: abort (core dumped) ./perl -t -Ilib -e 'unshift @​INC, shift; require Fcntl' foo

This crash seems to have been fixed since. Bisecting gives

commit 0f94cb1
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Thu Nov 27 22​:30​:54 2014 -0800

  [perl #123223] Make PADNAME a separate type
 
  distinct from SV. This should fix the CPAN modules that were failing
  when the PadnameLVALUE flag was added, because it shared the same
  bit as SVs_OBJECT and pad names were going through code paths not
  designed to handle pad names.
 
  Unfortunately, it will probably break other CPAN modules, but I think
  this change is for the better, as it makes both pad names and SVs sim-
  pler and makes pad names take less memory.

So this ticket can probably be closed?

Thanks,
--
Niko Tyni ntyni@​debian.org

@p5pRT p5pRT closed this as completed May 2, 2016
@p5pRT
Copy link
Author

p5pRT commented May 2, 2016

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

@p5pRT
Copy link
Author

p5pRT commented May 2, 2016

From @iabyn

On Fri, Apr 29, 2016 at 09​:23​:41AM +0300, Niko Tyni wrote​:

This crash seems to have been fixed since. Bisecting gives

commit 0f94cb1
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Thu Nov 27 22​:30​:54 2014 -0800

\[perl \#123223\] Make PADNAME a separate type

distinct from SV\.  This should fix the CPAN modules that were failing
when the PadnameLVALUE flag was added\, because it shared the same
bit as SVs\_OBJECT and pad names were going through code paths not
designed to handle pad names\.

Unfortunately\, it will probably break other CPAN modules\, but I think
this change is for the better\, as it makes both pad names and SVs sim\-
pler and makes pad names take less memory\.

So this ticket can probably be closed?

Closed now, thanks.

--
"There's something wrong with our bloody ships today, Chatfield."
  -- Admiral Beatty at the Battle of Jutland, 31st May 1916.

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