From @geeknik

Built v5.21.10 (v5.21.9-73-gd98e5cd) with the following command line​:

./Configure -des -Dusedevel -DDEBUGGING -Dcc=afl-gcc -Doptimize=-O2\ -g && AFL_HARDEN=1 make -j12 test-prep

Bug found with AFL (http​://lcamtuf.coredump.cx/afl)

Valgrind​:
==39011== Invalid read of size 8
==39011== at 0x8DC0E6​: Perl_pp_iter (pp_hot.c​:2348)
==39011== by 0x7CB49E​: Perl_runops_debug (dump.c​:2237)
==39011== by 0x53B4C8​: perl_run (perl.c​:2427)
==39011== by 0x42B167​: main (perlmain.c​:116)
==39011== Address 0x8 is not stack'd, malloc'd or (recently) free'd
==39011==
==39011==
==39011== Process terminating with default action of signal 11 (SIGSEGV)
==39011== Access not within mapped region at address 0x8
==39011== at 0x8DC0E6​: Perl_pp_iter (pp_hot.c​:2348)
==39011== by 0x7CB49E​: Perl_runops_debug (dump.c​:2237)
==39011== by 0x53B4C8​: perl_run (perl.c​:2427)
==39011== by 0x42B167​: main (perlmain.c​:116)
==39011== If you believe this happened as a result of a stack
==39011== overflow in your program's main thread (unlikely but
==39011== possible), you can try to increase the size of the
==39011== main thread stack using the --main-stacksize= flag.
==39011== The main thread stack size used in this run was 8388608.
Segmentation fault

GDB​:
gdb-peda$ file ~/perl/perl
gdb-peda$ set args test19-min
gdb-peda$ r
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".

Program received signal SIGSEGV, Segmentation fault.
[----------------------------------registers-----------------------------------]
RAX​: 0x3f8
RBX​: 0x11d0fb8 --> 0xabab0317
RCX​: 0x5
RDX​: 0x11cfb10 --> 0x11cdd40 --> 0x11cfb50 --> 0x0
RSI​: 0xb ('\x0b')
RDI​: 0x11d0f60 --> 0x2800102
RBP​: 0x11d0b50 --> 0x11cb260 --> 0x0
RSP​: 0x7fffffffe100 --> 0x0
RIP​: 0x8dc0e6 (<Perl_pp_iter+2182>​: and r9,QWORD PTR [r14+0x8])
R8 : 0x11ecb98 --> 0x11ee9d0 --> 0x11ecb08 --> 0x11ecac0 --> 0x11eca18 --> 0x0
R9 : 0xe00000ffffffff
R10​: 0x7
R11​: 0x11d0b50 --> 0x11cb260 --> 0x0
R12​: 0x11db960 --> 0x0
R13​: 0xffffffffffffffff
R14​: 0x0
R15​: 0x0
EFLAGS​: 0x10286 (carry PARITY adjust zero SIGN trap INTERRUPT direction overflow)
[-------------------------------------code-------------------------------------]
  0x8dc0d0 <Perl_pp_iter+2160>​: lea rsp,[rsp+0x98]
  0x8dc0d8 <Perl_pp_iter+2168>​: mov r14,QWORD PTR [r12]
  0x8dc0dc <Perl_pp_iter+2172>​: movabs r9,0xe00000ffffffff
=> 0x8dc0e6 <Perl_pp_iter+2182>​: and r9,QWORD PTR [r14+0x8]
  0x8dc0ea <Perl_pp_iter+2186>​: cmp r9,0x1
  0x8dc0ee <Perl_pp_iter+2190>​: jne 0x8dcdf0 <Perl_pp_iter+5520>
  0x8dc0f4 <Perl_pp_iter+2196>​: lea rsp,[rsp-0x98]
  0x8dc0fc <Perl_pp_iter+2204>​: mov QWORD PTR [rsp],rdx
[------------------------------------stack-------------------------------------]
0000| 0x7fffffffe100 --> 0x0
0008| 0x7fffffffe108 --> 0x56019b6d9fa02000
0016| 0x7fffffffe110 --> 0x0
0024| 0x7fffffffe118 --> 0x11ee9d0 --> 0x11ecb08 --> 0x11ecac0 --> 0x11eca18 --> 0x0
0032| 0x7fffffffe120 --> 0x0
0040| 0x7fffffffe128 --> 0x0
0048| 0x7fffffffe130 --> 0x7fffffffe3b0 --> 0x2
0056| 0x7fffffffe138 --> 0x0
[------------------------------------------------------------------------------]
Legend​: code, data, rodata, value
Stopped reason​: SIGSEGV
0x00000000008dc0e6 in Perl_pp_iter () at pp_hot.c​:2348
2348 if (LIKELY(SvREFCNT(oldsv) == 1 && !SvMAGICAL(oldsv))) {
gdb-peda$ exploit
Description​: Access violation near NULL on source operand
Short description​: SourceAvNearNull (16/22)
Hash​: a72345bbb4120d571c24a3a40f371684.a72345bbb4120d571c24a3a40f371684
Exploitability Classification​: PROBABLY_NOT_EXPLOITABLE
Explanation​: The target crashed on an access violation at an address matching the source operand of the current instruction. This likely indicates a read access violation, which may mean the application crashed on a simple NULL dereference to data structure that has no immediate effect on control of the processor.
Other tags​: AccessViolation (21/22)

Hexdump of 15-byte test case​:
0000000 5f2a 303d 7830 6630 726f 322d 2e2e 0030
000000f

Debian 7, Kernel 3.2.65-1+deb7u1 x86_64, GCC 4.9.2, libc 2.13-38+deb7u7

From @geeknik

test19-min

From @hvds

The test case can be simplified further to​:

% ./miniperl -e '*_ = "" for 0 .. 1'
Segmentation fault (core dumped)
%

This looks to be another stack refcounting issue, similar to the various other cases where we overwrite a glob part of which is in use.

Hugo

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

From @tonycoz

On Sat Mar 07 01​:12​:20 2015, hv wrote​:

The test case can be simplified further to​:

% ./miniperl -e '*_ = "" for 0 .. 1'
Segmentation fault (core dumped)
%

This looks to be another stack refcounting issue, similar to the
various other cases where we overwrite a glob part of which is in use.

This doesn't involve anything on the stack though.

In this case we have a GV * in the loop context, and we fetch a direct entry to its SV * member, which the *_ = "" has nulled out.

The following is probably the simplest fix​:

diff --git a/cop.h b/cop.h
index d36d189..98f3192 100644
--- a/cop.h
+++ b/cop.h
@@ -773,7 +773,7 @@ struct block_loop {
         ? (CxPADLOOP(c)                                                \
            ? CxITERVAR_PADSV(c)                                        \
            : isGV((c)->blk_loop.itervar_u.gv)                          \
-               ? &GvSV((c)->blk_loop.itervar_u.gv)                     \
+               ? &GvSVn((c)->blk_loop.itervar_u.gv)                    \
                : (SV **)&(c)->blk_loop.itervar_u.gv)                   \
         : (SV**)NULL)

diff --git a/pp_hot.c b/pp_hot.c
index e866841..fe48dfe 100644
--- a/pp_hot.c
+++ b/pp_hot.c
@@ -2558,7 +2558,7 @@ PP(pp_iter)
             RETPUSHNO;
 
         oldsv = *itersvp;
-        if (LIKELY(SvREFCNT(oldsv) == 1 && !SvMAGICAL(oldsv))) {
+        if (LIKELY(oldsv && SvREFCNT(oldsv) == 1 && !SvMAGICAL(oldsv))) {
             /* safe to reuse old SV */
             sv_setsv(oldsv, cur);
         }
@@ -2568,7 +2568,7 @@ PP(pp_iter)
              * completely new SV for closures/references to work as
              * they used to */
             *itersvp = newSVsv(cur);
-            SvREFCNT_dec_NN(oldsv);
+            SvREFCNT_dec(oldsv);
         }
         if (strEQ(SvPVX_const(cur), max))
             sv_setiv(cur, 0); /* terminate next time */
@@ -2585,7 +2585,7 @@ PP(pp_iter)
 
         oldsv = *itersvp;
        /* don't risk potential race */
-       if (LIKELY(SvREFCNT(oldsv) == 1 && !SvMAGICAL(oldsv))) {
+       if (LIKELY(oldsv && SvREFCNT(oldsv) == 1 && !SvMAGICAL(oldsv))) {
            /* safe to reuse old SV */
            sv_setiv(oldsv, cur);
        }
@@ -2595,7 +2595,7 @@ PP(pp_iter)
             * completely new SV for closures/references to work as they
             * used to */
            *itersvp = newSViv(cur);
-           SvREFCNT_dec_NN(oldsv);
+           SvREFCNT_dec(oldsv);
        }
 
        if (UNLIKELY(cur == IV_MAX)) {


Tony

From @iabyn

On Mon, Oct 19, 2015 at 04​:48​:57PM -0700, Tony Cook via RT wrote​:

On Sat Mar 07 01​:12​:20 2015, hv wrote​:

The test case can be simplified further to​:

% ./miniperl -e '*_ = "" for 0 .. 1'
Segmentation fault (core dumped)
%

This looks to be another stack refcounting issue, similar to the
various other cases where we overwrite a glob part of which is in use.

This doesn't involve anything on the stack though.

In this case we have a GV * in the loop context, and we fetch a direct entry to its SV * member, which the *_ = "" has nulled out.

The following is probably the simplest fix​:

Note that I have a private branch that *heavily* reworks cop.h and
pp_iter, so could you hold off applying anything like this, thanks?
Having said that, my branch also SEGV's, so I'll try to apply something
similar to the below to may branch sometime soon, once I've looked into
it.

--
That he said that that that that is is is debatable, is debatable.

From @iabyn

On Tue, Oct 20, 2015 at 01​:32​:04PM +0100, Dave Mitchell wrote​:

On Mon, Oct 19, 2015 at 04​:48​:57PM -0700, Tony Cook via RT wrote​:

On Sat Mar 07 01​:12​:20 2015, hv wrote​:

The test case can be simplified further to​:

% ./miniperl -e '*_ = "" for 0 .. 1'
Segmentation fault (core dumped)
%

This looks to be another stack refcounting issue, similar to the
various other cases where we overwrite a glob part of which is in use.

This doesn't involve anything on the stack though.

In this case we have a GV * in the loop context, and we fetch a direct entry to its SV * member, which the *_ = "" has nulled out.

The following is probably the simplest fix​:
...
but it means we're creating a new SV and then calling sv_setiv() on it
which might be more expensive than that newSViv(), perhaps the
following would be better​:

Note that I have a private branch that *heavily* reworks cop.h and
pp_iter, so could you hold off applying anything like this, thanks?
Having said that, my branch also SEGV's, so I'll try to apply something
similar to the below to may branch sometime soon, once I've looked into
it.

I've now applied your second variant to my private context branch, which
I intend to push for smoking Real Soon Now.

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

From @iabyn

On Fri, Dec 18, 2015 at 03​:34​:45PM +0000, Dave Mitchell wrote​:

On Tue, Oct 20, 2015 at 01​:32​:04PM +0100, Dave Mitchell wrote​:

On Mon, Oct 19, 2015 at 04​:48​:57PM -0700, Tony Cook via RT wrote​:

On Sat Mar 07 01​:12​:20 2015, hv wrote​:

The test case can be simplified further to​:

% ./miniperl -e '*_ = "" for 0 .. 1'
Segmentation fault (core dumped)
%

This looks to be another stack refcounting issue, similar to the
various other cases where we overwrite a glob part of which is in use.

This doesn't involve anything on the stack though.

In this case we have a GV * in the loop context, and we fetch a direct entry to its SV * member, which the *_ = "" has nulled out.

The following is probably the simplest fix​:
...
but it means we're creating a new SV and then calling sv_setiv() on it
which might be more expensive than that newSViv(), perhaps the
following would be better​:

Note that I have a private branch that *heavily* reworks cop.h and
pp_iter, so could you hold off applying anything like this, thanks?
Having said that, my branch also SEGV's, so I'll try to apply something
similar to the below to may branch sometime soon, once I've looked into
it.

I've now applied your second variant to my private context branch, which
I intend to push for smoking Real Soon Now.

And now merged into blead, as

6d3ca00

--
I don't want to achieve immortality through my work...
I want to achieve it through not dying.
  -- Woody Allen

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

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

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