Assert fail w/o other symptoms - pp_sys.c:690 Perl_pp_pipe_op when first arg to pipe is definitely not a filehandle

[p5pRT]

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

Searchable as RT126480$

[p5pRT]

From @dcollinsn

Greetings Porters,

I have compiled bleadperl with the afl-gcc compiler using​:

./Configure -Dusedevel -Dprefix='/usr/local/perl-afl' -Dcc='ccache afl-gcc' -Duselongdouble -Duse64bitall -Doptimize=-g -Uversiononly -Uman1dir -Uman3dir -des
AFL_HARDEN=1 make && make test

And then fuzzed the resulting binary using​:

AFL_NO_VAR_CHECK=1 afl-fuzz -i in -o out bin/perl @​@​

After reducing testcases using `afl-tmin` and performing additional minimization by hand, I have located the following testcase that triggers an assert fail in DEBUGGING perls without any other symptoms in the normal perl interpreter. The testcase is the file​:

pipe$$5,0

dcollins@​nightshade64​:/usr/local/perl-afl$ ./bin/perl -e 'pipe$$5,0'
perl​: pp_sys.c​:690​: Perl_pp_pipe_op​: Assertion `((((rgv)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((rgv)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((rgv)->sv_flags & 0xff)) == SVt_PVLV))' failed.
Aborted

The output with a normal perl is the expected error​:

dcollins@​nightshade64​:/usr/local/perl-afl$ ~/perl/perl -e 'pipe$$5,0'
Bad symbol for filehandle at -e line 1.

**GDB**

(gdb) run
Starting program​: /usr/local/perl-afl/bin/perl -e pipe\$\$5,0
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
bperl​: pp_sys.c​:690​: Perl_pp_pipe_op​: Assertion `((((rgv)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((rgv)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((rgv)->sv_flags & 0xff)) == SVt_PVLV))' failed.

Program received signal SIGABRT, Aborted.
0x00007ffff6cf4107 in raise () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) bt
#0 0x00007ffff6cf4107 in raise () from /lib/x86_64-linux-gnu/libc.so.6
#1 0x00007ffff6cf54e8 in abort () from /lib/x86_64-linux-gnu/libc.so.6
#2 0x00007ffff6ced226 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#3 0x00007ffff6ced2d2 in __assert_fail ()
  from /lib/x86_64-linux-gnu/libc.so.6
#4 0x0000000000b95275 in Perl_pp_pipe_op () at pp_sys.c​:690
#5 0x00000000007bdf7f in Perl_runops_debug () at dump.c​:2224
#6 0x0000000000527fc1 in S_run_body (oldscope=1) at perl.c​:2459
#7 perl_run (my_perl=<optimized out>) at perl.c​:2382
#8 0x0000000000428b18 in main (argc=3, argv=0x7fffffffe658,
  env=0x7fffffffe678) at perlmain.c​:116
(gdb) f 4
#4 0x0000000000b95275 in Perl_pp_pipe_op () at pp_sys.c​:690
690 assert (isGV_with_GP(rgv));
(gdb) info locals
sp = 0x11a8a20
rstio = <optimized out>
wstio = <optimized out>
fd = {0, 0}
wgv = 0x11ba5a0
rgv = 0x11a30a0 <PL_sv_undef>
__PRETTY_FUNCTION__ = "Perl_pp_pipe_op"
(gdb) q
A debugging session is active.

  Inferior 1 [process 4065] will be killed.

Quit anyway? (y or n) y

**VALGRIND**

dcollins@​nightshade64​:/usr/local/perl-afl$ valgrind ./bin/perl -e 'pipe$$5,0'
==11068== Memcheck, a memory error detector
==11068== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==11068== Using Valgrind-3.11.0 and LibVEX; rerun with -h for copyright info
==11068== Command​: ./bin/perl -e pipe$$5,0
==11068==
perl​: pp_sys.c​:690​: Perl_pp_pipe_op​: Assertion `((((rgv)->sv_flags & (0x00004000|0x00008000)) == 0x00008000) && (((svtype)((rgv)->sv_flags & 0xff)) == SVt_PVGV || ((svtype)((rgv)->sv_flags & 0xff)) == SVt_PVLV))' failed.
==11068==
==11068== Process terminating with default action of signal 6 (SIGABRT)
==11068== at 0x5BDC107​: raise (raise.c​:56)
==11068== by 0x5BDD4E7​: abort (abort.c​:89)
==11068== by 0x5BD5225​: __assert_fail_base (assert.c​:92)
==11068== by 0x5BD52D1​: __assert_fail (assert.c​:101)
==11068== by 0xB95274​: Perl_pp_pipe_op (pp_sys.c​:690)
==11068== by 0x7BDF7E​: Perl_runops_debug (dump.c​:2224)
==11068== by 0x527FC0​: S_run_body (perl.c​:2459)
==11068== by 0x527FC0​: perl_run (perl.c​:2382)
==11068== by 0x428B17​: main (perlmain.c​:116)
==11068==
==11068== HEAP SUMMARY​:
==11068== in use at exit​: 110,234 bytes in 542 blocks
==11068== total heap usage​: 681 allocs, 139 frees, 134,449 bytes allocated
==11068==
==11068== LEAK SUMMARY​:
==11068== definitely lost​: 176 bytes in 1 blocks
==11068== indirectly lost​: 1,974 bytes in 20 blocks
==11068== possibly lost​: 0 bytes in 0 blocks
==11068== still reachable​: 108,084 bytes in 521 blocks
==11068== suppressed​: 0 bytes in 0 blocks
==11068== Rerun with --leak-check=full to see details of leaked memory
==11068==
==11068== For counts of detected and suppressed errors, rerun with​: -v
==11068== ERROR SUMMARY​: 0 errors from 0 contexts (suppressed​: 0 from 0)
Aborted

**PERL -V**

dcollins@​nightshade64​:/usr/local/perl-afl$ ./bin/perl -V
Summary of my perl5 (revision 5 version 23 subversion 5) configuration​:
  Commit id​: 7195e5d
  Platform​:
  osname=linux, osvers=3.16.0-4-amd64, archname=x86_64-linux-ld
  uname='linux nightshade64 3.16.0-4-amd64 #1 smp debian 3.16.7-ckt11-1+deb8u4 (2015-09-19) x86_64 gnulinux '
  config_args='-Dusedevel -Dprefix=/usr/local/perl-afl -Dcc=ccache afl-gcc -Duselongdouble -Duse64bitall -Doptimize=-g -Uversiononly -Uman1dir -Uman3dir -DDEBUGGING -DPERL_POISON -des'
  hint=recommended, useposix=true, d_sigaction=define
  useithreads=undef, usemultiplicity=undef
  use64bitint=define, use64bitall=define, uselongdouble=define
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='ccache afl-gcc', ccflags ='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
  optimize='-g',
  cppflags='-fwrapv -DDEBUGGING -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion='', gccversion='5.2.0', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3
  ivtype='long', ivsize=8, nvtype='long double', nvsize=16, Off_t='off_t', lseeksize=8
  alignbytes=16, prototype=define
  Linker and Libraries​:
  ld='ccache afl-gcc', ldflags =' -fstack-protector-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/local/lib/gcc/x86_64-unknown-linux-gnu/5.2.0/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.19.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.19'
  Dynamic Linking​:
  dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
  cccdlflags='-fPIC', lddlflags='-shared -g -L/usr/local/lib -fstack-protector-strong'

Characteristics of this binary (from libperl)​:
  Compile-time options​: DEBUGGING HAS_TIMES PERLIO_LAYERS PERL_COPY_ON_WRITE
  PERL_DONT_CREATE_GVSV
  PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_MALLOC_WRAP
  PERL_PRESERVE_IVUV PERL_USE_DEVEL USE_64_BIT_ALL
  USE_64_BIT_INT USE_LARGE_FILES USE_LOCALE
  USE_LOCALE_COLLATE USE_LOCALE_CTYPE
  USE_LOCALE_NUMERIC USE_LOCALE_TIME USE_LONG_DOUBLE
  USE_PERLIO USE_PERL_ATOF
  Built under linux
  Compiled at Oct 22 2015 15​:44​:40
  @​INC​:
  /usr/local/perl-afl/lib/site_perl/5.23.5/x86_64-linux-ld
  /usr/local/perl-afl/lib/site_perl/5.23.5
  /usr/local/perl-afl/lib/5.23.5/x86_64-linux-ld
  /usr/local/perl-afl/lib/5.23.5
  /usr/local/perl-afl/lib/site_perl/5.23.4
  /usr/local/perl-afl/lib/site_perl
  .

[p5pRT]

From @tonycoz

On Wed Oct 28 19​:59​:00 2015, dcollinsn@​gmail.com wrote​:

After reducing testcases using `afl-tmin` and performing additional
minimization by hand, I have located the following testcase that
triggers an assert fail in DEBUGGING perls without any other symptoms
in the normal perl interpreter. The testcase is the file​:

pipe$$5,0

dcollins@​nightshade64​:/usr/local/perl-afl$ ./bin/perl -e 'pipe$$5,0'
perl​: pp_sys.c​:690​: Perl_pp_pipe_op​: Assertion `((((rgv)->sv_flags &
(0x00004000|0x00008000)) == 0x00008000) && (((svtype)((rgv)->sv_flags
& 0xff)) == SVt_PVGV || ((svtype)((rgv)->sv_flags & 0xff)) ==
SVt_PVLV))' failed.
Aborted

The output with a normal perl is the expected error​:

dcollins@​nightshade64​:/usr/local/perl-afl$ ~/perl/perl -e 'pipe$$5,0'
Bad symbol for filehandle at -e line 1.

I suspect the assertions are incorrect here.

S_rv2gv returns &PL_sv_undef when it can't turn the value into something resembling a GV, and the GvIOn() macro can handle that (by croaking).

So the assertions would be something like​:

Inline Patch

diff --git a/pp_sys.c b/pp_sys.c
index 373590f..8589413 100644
--- a/pp_sys.c
+++ b/pp_sys.c
@@ -695,8 +695,10 @@ PP(pp_pipe_op)
     GV * const wgv = MUTABLE_GV(POPs);
     GV * const rgv = MUTABLE_GV(POPs);
 
-    assert (isGV_with_GP(rgv));
-    assert (isGV_with_GP(wgv));
+    /* rv2gv pushes PL_sv_undef when it can't make a GV, and GvIOn() properly croaks
+       when it's supplied with such */
+    assert ((SV*)rgv == &PL_sv_undef || isGV_with_GP(rgv));
+    assert ((SV*)wgv == &PL_sv_undef || isGV_with_GP(wgv));
     rstio = GvIOn(rgv);
     if (IoIFP(rstio))
        do_close(rgv, FALSE);

The other I/O "openers" (socket, sysopen, open) either call GvIOn() or do a non-assertion test of isGV_with_GP().

Possibly the assertions should just be removed.

Tony

[p5pRT]

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

[p5pRT]

From @tonycoz

On Wed Oct 28 20​:49​:47 2015, tonyc wrote​:

Possibly the assertions should just be removed.

Like in the attached patch.

Tony

[p5pRT]

From @tonycoz

0001-perl-126480-pipe-doesn-t-need-the-assertions.patch

From 992ebee3f63e16317109eca1eb0d76925c594593 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Mon, 2 Nov 2015 17:22:25 +1100
Subject: [perl 126480] pipe() doesn't need the assertions

GvIOn() already performs the checks and produces a nice error message.
---
 MANIFEST           |  1 +
 pp_sys.c           |  2 --
 t/lib/croak/pp_sys | 16 ++++++++++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)
 create mode 100644 t/lib/croak/pp_sys

diff --git a/MANIFEST b/MANIFEST
index f07488f..ed47673 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -5043,6 +5043,7 @@ t/lib/croak/op			Test croak calls from op.c
 t/lib/croak/pp			Test croak calls from pp.c
 t/lib/croak/pp_ctl		Test croak calls from pp_ctl.c
 t/lib/croak/pp_hot		Test croak calls from pp_hot.c
+t/lib/croak/pp_sys		Test croak calls from pp_sys.c
 t/lib/croak.t			Test calls to Perl_croak() in the C source.
 t/lib/croak/toke		Test croak calls from toke.c
 t/lib/cygwin.t			Builtin cygwin function tests
diff --git a/pp_sys.c b/pp_sys.c
index 373590f..15b4d8b 100644
--- a/pp_sys.c
+++ b/pp_sys.c
@@ -695,8 +695,6 @@ PP(pp_pipe_op)
     GV * const wgv = MUTABLE_GV(POPs);
     GV * const rgv = MUTABLE_GV(POPs);
 
-    assert (isGV_with_GP(rgv));
-    assert (isGV_with_GP(wgv));
     rstio = GvIOn(rgv);
     if (IoIFP(rstio))
 	do_close(rgv, FALSE);
diff --git a/t/lib/croak/pp_sys b/t/lib/croak/pp_sys
new file mode 100644
index 0000000..001baa3
--- /dev/null
+++ b/t/lib/croak/pp_sys
@@ -0,0 +1,16 @@
+__END__
+# pp_sys.c
+# NAME pipe() croaks on bad left side [perl #126480]
+# SKIP ? use Config; !$Config{d_pipe} && "No pipe() available"
+my $fh;
+pipe($$5, $fh)
+EXPECT
+Bad symbol for filehandle at - line 3.
+########
+# NAME pipe() croaks on bad left side [perl #126480]
+# SKIP ? use Config; !$Config{d_pipe} && "No pipe() available"
+my $fh;
+pipe($fh, $$5)
+EXPECT
+Bad symbol for filehandle at - line 2.
+########
-- 
2.1.4

[p5pRT]

From @tonycoz

On Sun Nov 01 22​:23​:07 2015, tonyc wrote​:

On Wed Oct 28 20​:49​:47 2015, tonyc wrote​:

Possibly the assertions should just be removed.

Like in the attached patch.

Applied as e8c18a8, with a fix for the NAME line of the second test.

Tony

[p5pRT]

@tonycoz - Status changed from 'open' to 'pending release'

[p5pRT]

From @khwilliamson

Thank you for submitting this report. You have helped make Perl better.
 
With the release of Perl 5.24.0 on May 9, 2016, this and 149 other issues have been resolved.

Perl 5.24.0 may be downloaded via https://metacpan.org/release/RJBS/perl-5.24.0

[p5pRT]

@khwilliamson - Status changed from 'pending release' to 'resolved'