Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

segfault in node v0.6.6 #2438

Closed
ry opened this issue Dec 29, 2011 · 4 comments
Closed

segfault in node v0.6.6 #2438

ry opened this issue Dec 29, 2011 · 4 comments

Comments

@ry
Copy link

ry commented Dec 29, 2011

Experiencing crashes in what appears to be a bad ref in node_http_parser.cc during StringPtr::Update.

(gdb) bt
#0  0x00007f470e1fcd30 in memcpy () from /lib/libc.so.6
#1  0x000000000053e58b in node::StringPtr::Update (p_=<value optimized out>, 
    at=0x662efea "92395c572&now=1325093117.714518 HTTP/1.1\r\nHost: prod-nr263.vox^?\226?\217\027RIR\031J\026\222\231{94\\?\202??\235\a??.?K? \017\004\061a\n     9d8\nyR\026S]\234??S?c?@\034\030?9@??ſ?\217z\213\004\061f\n     9ec\nyj???\016??\212?ȿ\fs\225H\035?\210\205?a\f?GU?y/f\017\004\061b\n     a00\ny\212\217\035m޺?\226\204"..., 
    length=31) at /usr/include/bits/string3.h:52
#2  node::Parser::on_url_ (p_=<value optimized out>, 
    at=0x662efea "92395c572&now=1325093117.714518 HTTP/1.1\r\nHost: prod-nr263.vox^?\226?\217\027RIR\031J\026\222\231{94\\?\202??\235\a??.?K? \017\004\061a\n     9d8\nyR\026S]\234??S?c?@\034\030?9@??ſ?\217z\213\004\061f\n     9ec\nyj???\016??\212?ȿ\fs\225H\035?\210\205?a\f?GU?y/f\017\004\061b\n     a00\ny\212\217\035m޺?\226\204"..., 
    length=31) at ../src/node_http_parser.cc:250
#3  node::Parser::on_url (p_=<value optimized out>, 
    at=0x662efea "92395c572&now=1325093117.714518 HTTP/1.1\r\nHost: prod-nr263.vox^?\226?\217\027RIR\031J\026\222\231{94\\?\202??\235\a??.?K? \017\004\061a\n     9d8\nyR\026S]\234??S?c?@\034\030?9@??ſ?\217z\213\004\061f\n     9ec\nyj???\016??\212?ȿ\fs\225H\035?\210\205?a\f?GU?y/f\017\004\061b\n     a00\ny\212\217\035m޺?\226\204"..., 
    length=31) at ../src/node_http_parser.cc:248
#4  0x000000000057aa5b in http_parser_execute (parser=0x4bb72e8, settings=0xc3d000, data=<value optimized out>, 
    len=<value optimized out>) at ../deps/http_parser/http_parser.c:993
#5  0x000000000053e3b6 in node::Parser::Execute (args=<value optimized out>) at ../src/node_http_parser.cc:457
#6  0x00000000005a9d41 in v8::internal::Builtin_HandleApiCall(v8::internal::(anonymous namespace)::BuiltinArguments<(v8::internal::BuiltinExtraArguments)1>, v8::internal::Isolate*) ()

Crashes are very rare. At first we thought this was a problem with missing HandleScope and made 73cf8e8 to fix it. It turns out that didn't stop the problems and was reverted in 448c5e0.

cc @mranney @bnoordhuis
ry added a commit that referenced this issue Dec 29, 2011
@ry
Add test for #2438
432a2e4 
Unfortunately valgrind must be used to see the bad read. It would be nice if
we could improve this test to cause a segfault.
@mranney
Copy link

mranney commented Dec 29, 2011

Stacks from various cores run into trouble in a number of places, all after http_parser somewhere. Memory corruption is tricky.

@ry
Copy link
Author

ry commented Dec 30, 2011

potential fix https://gist.github.com/1536827

@ry
Copy link
Author

ry commented Dec 31, 2011

potential fix didn't work. resulted in new crashes https://gist.github.com/53efa2e0f2b1efac75c8

ry added a commit that referenced this issue Jan 4, 2012
@ry
Add another test to test-http-parser-bad-ref.js demoing #2438
2cde498
ry added a commit that referenced this issue Jan 4, 2012
@ry
Potential fix for #2438
f3da6c6 
- Save StringPtr if the header hasn't been completely received yet after one
  packet.
- Add one to num_fields and num_values. They were actually one less than the
  number of fields and values.
- Remove always_inline makes debugging difficult, and has negligible
  performance benefits.
@ry
Copy link
Author

ry commented Jan 5, 2012

f3da6c6 was tested in production and does fix the bug.

@ry ry closed this as completed Jan 5, 2012
isaacs added a commit to isaacs/node-v0.x-archive that referenced this issue Jan 6, 2012
@isaacs
2012.01.06, Version 0.6.7 (stable)
f15ef99 
* Upgrade V8 to 3.6.6.15

* Upgrade npm to 1.1.0-beta-10 (isaacs)

* many doc updates (Ben Noordhuis, Jeremy Martin, koichik, Dave Irvine,
  Seong-Rak Choi, Shannen, Adam Malcontenti-Wilson, koichik)

* nodejs#2438 segfault in node v0.6.6

* dgram, timers: fix memory leaks (Ben Noordhuis, Yoshihiro Kukuchi)

* repl: fix repl.start not passing the `ignoreUndefined` arg (Damon Oehlman)

* nodejs#1980: Socket.pause null reference when called on a closed Stream (koichik)

* nodejs#2263: XMLHttpRequest piped in a writable file stream hang (koichik)

* nodejs#2069: http resource leak (koichik)

* buffer.readInt global pollution fix (Phil Sung)

* timers: fix performance regression (Ben Noordhuis)

* nodejs#2308, nodejs#2246: node swallows openssl error on request (koichik)

* nodejs#2114: timers: remove _idleTimeout from item in .unenroll() (James Hartig)

* nodejs#2379: debugger: Request backtrace w/o refs (Fedor Indutny)

* simple DTrace ustack helper (Dave Pacheco)

* crypto: rewrite HexDecode without snprintf (Roman Shtylman)

* crypto: add SecureContext.clearOptions() method (Ben Noordhuis)

* crypto: don't ignore DH init errors (Ben Noordhuis)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ry @mranney