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

Segmentation fault in S_scan_heredoc() #14462

Closed
p5pRT opened this issue Feb 1, 2015 · 12 comments
Closed

Segmentation fault in S_scan_heredoc() #14462

p5pRT opened this issue Feb 1, 2015 · 12 comments

Comments

@p5pRT
Copy link

p5pRT commented Feb 1, 2015

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

Searchable as RT123712$

@p5pRT
Copy link
Author

p5pRT commented Feb 1, 2015

From thecrux@gmail.com

Sigsegv testcase​:

  $ echo -n '/$a[/<<' | perl
  Use of bare << to mean <<"" is deprecated at - line 1.
  zsh​: done echo -n '/$a[/<<' |
  zsh​: segmentation fault perl

(gdb) run
Program received signal SIGSEGV, Segmentation fault.
__memcmp_sse4_1 () at ../sysdeps/x86_64/multiarch/memcmp-sse4.S​:949
949 ../sysdeps/x86_64/multiarch/memcmp-sse4.S​: No such file or directory.

(gdb) bt
#0 __memcmp_sse4_1 () at ../sysdeps/x86_64/multiarch/memcmp-sse4.S​:949
#1 0x000000000058f2ce in S_scan_heredoc (s=0x0) at toke.c​:9274
#2 Perl_yylex () at toke.c​:5891
#3 0x00000000005bc185 in Perl_yyparse (gramtype=<optimized out>) at perly.c​:322
#4 0x00000000004e2d45 in S_parse_body (xsinit=0x426dc0 <xs_init>, env=0x0) at perl.c​:2273
#5 perl_parse (my_perl=<optimized out>, xsinit=0x426dc0 <xs_init>, argc=<optimized out>,
  argv=<optimized out>, env=0x0) at perl.c​:1607
#6 0x00000000004269dc in main (argc=2, argv=0x7fffffffe498, env=0x7fffffffe4b0) at perlmain.c​:114
#7 0x00007ffff70d4ec5 in __libc_start_main (main=0x426870 <main>, argc=2, argv=0x7fffffffe498,
  init=<optimized out>, fini=<optimized out>, rtld_fini=<optimized out>, stack_end=0x7fffffffe488)
  at libc-start.c​:287
#8 0x0000000000426cf3 in _start ()

(gdb) frame 1
(gdb) list
9269 }
9270 linestr = shared->ls_linestr;
9271 bufend = SvEND(linestr);
9272 d = s;
9273 while (s < bufend - len + 1 &&
9274 memNE(s,PL_tokenbuf,len) ) {
9275 if (*s++ == '\n')
9276 ++PL_parser->herelines;
9277 }
9278 if (s >= bufend - len + 1) {

Crash reproduced with perl 5.18, 5.20, 5.21.8
Bug was found by afl fuzzer (http​://lcamtuf.coredump.cx/afl/)

@p5pRT
Copy link
Author

p5pRT commented Feb 1, 2015

From @cpansprout

On Sun Feb 01 13​:26​:13 2015, crux wrote​:

Sigsegv testcase​:

$ echo -n '/$a[/<<' | perl
Use of bare << to mean <<"" is deprecated at - line 1.
zsh​: done echo -n '/$a[/<<' |
zsh​: segmentation fault perl

(gdb) run
Program received signal SIGSEGV, Segmentation fault.

This is likely related to #123617, though it appears to be more recent. I’m running a bisect.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Feb 1, 2015

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

@p5pRT
Copy link
Author

p5pRT commented Feb 1, 2015

From @cpansprout

On Sun Feb 01 14​:38​:39 2015, sprout wrote​:

On Sun Feb 01 13​:26​:13 2015, crux wrote​:

Sigsegv testcase​:

$ echo -n '/$a[/<<' | perl
Use of bare << to mean <<"" is deprecated at - line 1.
zsh​: done echo -n '/$a[/<<' |
zsh​: segmentation fault perl

(gdb) run
Program received signal SIGSEGV, Segmentation fault.

This is likely related to #123617, though it appears to be more
recent. I’m running a bisect.

4efe39d is the first bad commit
commit 4efe39d
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Wed Aug 29 22​:07​:18 2012 -0700

  toke.c​:scan_heredoc​: Merge similar code
 
  The code for looking in outer lexing scopes was mostly identical to
  the code for looking in PL_linestr.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2015

From @hvds

On Sun Feb 01 14​:41​:10 2015, sprout wrote​:

On Sun Feb 01 14​:38​:39 2015, sprout wrote​:

This is likely related to #123617, though it appears to be more
recent. I’m running a bisect.

4efe39d is the first bad commit
commit 4efe39d
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Wed Aug 29 22​:07​:18 2012 -0700

toke\.c&#8203;:scan\_heredoc&#8203;: Merge similar code

The code for looking in outer lexing scopes was mostly identical to
the code for looking in PL\_linestr\.

The below is one aspect changed from the original, and is enough to make it survive (and pass tests); I'm not sure why reality doesn't match the various comments though (so that we're here with !infile and !PL_lex_inwhat, and it isn't an eval, and doesn't have a newline), so I'm not sure how to fix those.

Hugo

Inline Patch
diff --git a/toke.c b/toke.c
index 24b5ed0..13f30e7 100644
--- a/toke.c
+++ b/toke.c
@@ -9275,7 +9275,8 @@ S_scan_heredoc(pTHX_ char *s)
          }
        else {  /* eval */
            s = (char*)memchr((void*)s, '\n', PL_bufend - s);
-           assert(s);
+           if (!s)
+                s = PL_bufend;
        }
        linestr = shared->ls_linestr;
        bufend = SvEND(linestr);

@p5pRT
Copy link
Author

p5pRT commented Feb 10, 2015

From @cpansprout

On Tue Feb 10 10​:15​:07 2015, hv wrote​:

On Sun Feb 01 14​:41​:10 2015, sprout wrote​:

On Sun Feb 01 14​:38​:39 2015, sprout wrote​:

This is likely related to #123617, though it appears to be more
recent. I’m running a bisect.

4efe39d is the first bad commit
commit 4efe39d
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Wed Aug 29 22​:07​:18 2012 -0700

toke.c​:scan_heredoc​: Merge similar code

The code for looking in outer lexing scopes was mostly identical to
the code for looking in PL_linestr.

The below is one aspect changed from the original, and is enough to
make it survive (and pass tests); I'm not sure why reality doesn't
match the various comments though (so that we're here with !infile and
!PL_lex_inwhat, and it isn't an eval, and doesn't have a newline), so
I'm not sure how to fix those.

A call to skipspace that happens inside /$a[/ tries to read the next line of the file. When EOF is reached PL_rsfp is set to NULL, which is why S_scan_heredoc thinks we are inside an eval, which is not the case.

A similar circumstance can arise with​:

perl -e 'print q|/$a[<<end]/+<<| . "\nend"'|./miniperl

which does indeed make S_scan_heredoc think it is in an eval, but the first line does end with \n, so the memchr returns something and the assertion does not fail.

scan_heredoc’s assumptions are reasonable. I think we need to fix that /$a[/ bug, which will unfortunately break my japhs. :-(

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Feb 16, 2015

From @cpansprout

On Tue Feb 10 12​:38​:15 2015, sprout wrote​:

On Tue Feb 10 10​:15​:07 2015, hv wrote​:

On Sun Feb 01 14​:41​:10 2015, sprout wrote​:

On Sun Feb 01 14​:38​:39 2015, sprout wrote​:

This is likely related to #123617, though it appears to be more
recent. I’m running a bisect.

4efe39d is the first bad commit
commit 4efe39d
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Wed Aug 29 22​:07​:18 2012 -0700

toke.c​:scan_heredoc​: Merge similar code

The code for looking in outer lexing scopes was mostly identical to
the code for looking in PL_linestr.

The below is one aspect changed from the original, and is enough to
make it survive (and pass tests); I'm not sure why reality doesn't
match the various comments though (so that we're here with !infile
and
!PL_lex_inwhat, and it isn't an eval, and doesn't have a newline), so
I'm not sure how to fix those.

A call to skipspace that happens inside /$a[/ tries to read the next
line of the file. When EOF is reached PL_rsfp is set to NULL, which
is why S_scan_heredoc thinks we are inside an eval, which is not the
case.

A similar circumstance can arise with​:

perl -e 'print q|/$a[<<end]/+<<| . "\nend"'|./miniperl

which does indeed make S_scan_heredoc think it is in an eval, but the
first line does end with \n, so the memchr returns something and the
assertion does not fail.

scan_heredoc’s assumptions are reasonable. I think we need to fix
that /$a[/ bug, which will unfortunately break my japhs. :-(

I have finally fixed this in e47d32d. What I said about skipspace was not correct. skipspace was doing the right thing, but many other paths were calling lex_read_space or lex_next_chunk without the proper guards. But putting guards all over the place seemed the wrong approach, so I modified lex_next_chunk instead.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Feb 16, 2015

From @cpansprout

On Sun Feb 15 17​:39​:17 2015, sprout wrote​:

I have finally fixed this in e47d32d. What I said about skipspace was
not correct. skipspace was doing the right thing, but many other
paths were calling lex_read_space or lex_next_chunk without the proper
guards. But putting guards all over the place seemed the wrong
approach, so I modified lex_next_chunk instead.

I followed up with another fix in d27f4b9. A similar case was still crashing.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Feb 16, 2015

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

@p5pRT
Copy link
Author

p5pRT commented Jun 2, 2015

From @khwilliamson

Thanks for submitting this ticket

The issue should be resolved with the release today of Perl v5.22, available at http​://www.perl.org/get.html
If you find that the problem persists, feel free to reopen this ticket

--
Karl Williamson for the Perl 5 porters team

@p5pRT
Copy link
Author

p5pRT commented Jun 2, 2015

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

@p5pRT p5pRT closed this as completed Jun 2, 2015
@p5pRT
Copy link
Author

p5pRT commented Jun 6, 2016

From @karenetheridge

On Mon Jun 01 20​:50​:31 2015, khw wrote​:

Thanks for submitting this ticket

The issue should be resolved with the release today of Perl v5.22,
available at http​://www.perl.org/get.html
If you find that the problem persists, feel free to reopen this ticket

For tracability, this fix appears to also have been backported to perl 5.20.3, so the resulting BBC bug https://rt.perl.org/Public/Bug/Display.html?id=123865 is also in 5.20.3 (therefore Devel-Declare's fixes are needed for 5.20.3 as well).

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

No branches or pull requests

1 participant