Uninitialised value use in nsHttpConnection::CanReuse

RESOLVED FIXED in Firefox 14

Status

()

Core
Networking: HTTP
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jseward, Assigned: mcmanus)

Tracking

Trunk
mozilla15
x86_64
Linux
Points:
---

Firefox Tracking Flags

(firefox13 unaffected, firefox14 fixed, firefox15 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
/tests/toolkit/components/passwordmgr/test/test_bug_627616.html


(DISPLAY=:3.0 make -C ff-opt mochitest-plain TEST_PATH=toolkit/components/passwordmgr/test/test_bug_627616.html EXTRA_TEST_ARGS="--close-when-done --debugger=/home/sewardj/VgTRUNK/tchain/Inst/bin/valgrind --debugger-args='--smc-check=all-non-file --suppressions=/home/sewardj/MOZ/SUPPS/mochitest-mc.supp --error-limit=no --stats=no --trace-children=yes --child-silent-after-fork=yes "--trace-children-skip=/usr/bin/hg,/bin/rm,*/bin/certutil,*/bin/pk12util,*/bin/ssltunnel,*/bin/uname,*/bin/which,*/bin/ps,*/bin/grep" --tool=memcheck --track-origins=yes'") 2>&1 | tee spew-mc-627616-1


Conditional jump or move depends on uninitialised value(s)
   at 0x6315097: nsHttpConnection::CanReuse() (nsHttpConnection.cpp:571)
   by 0x631B839: nsHttpConnectionMgr::OnMsgReclaimConnection(int, void*) (nsHttpConnectionMgr.cpp:1895)
   by 0x6315DB7: nsHttpConnectionMgr::nsConnEvent::Run() (nsHttpConnectionMgr.h:518)
   by 0x6FF0E39: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:656)
   by 0x6FB95A9: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:245)
   by 0x62C303B: nsSocketTransportService::Run() (nsSocketTransportService2.cpp:652)
   by 0x6FF0E39: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:656)
   by 0x6FB95A9: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:245)
   by 0x6FF0B64: nsThread::ThreadFunc(void*) (nsThread.cpp:289)
   by 0x41A3EB2: _pt_root (ptthread.c:187)
   by 0x4A289C9: start_thread (pthread_create.c:300)
   by 0x53B470C: clone (clone.S:112)
 Uninitialised value was created by a heap allocation
   at 0x4029B9A: malloc (vg_replace_malloc.c:263)
   by 0x403CF6B: moz_xmalloc (mozalloc.cpp:87)
   by 0x631BB14: nsHttpConnectionMgr::nsHalfOpenSocket::OnOutputStreamReady(nsIAsyncOutputStream*) (mozalloc.h:229)
   by 0x62BE644: nsSocketOutputStream::OnSocketReady(unsigned int) (nsSocketTransport2.cpp:525)
   by 0x62C062C: nsSocketTransport::OnSocketReady(PRFileDesc*, short) (nsSocketTransport2.cpp:1568)
   by 0x62C2DB2: nsSocketTransportService::DoPollIteration(bool) (nsSocketTransportService2.cpp:762)
   by 0x62C3027: nsSocketTransportService::Run() (nsSocketTransportService2.cpp:645)
   by 0x6FF0E39: nsThread::ProcessNextEvent(bool, bool*) (nsThread.cpp:656)
   by 0x6FB95A9: NS_ProcessNextEvent_P(nsIThread*, bool) (nsThreadUtils.cpp:245)
   by 0x6FF0B64: nsThread::ThreadFunc(void*) (nsThread.cpp:289)
   by 0x41A3EB2: _pt_root (ptthread.c:187)
   by 0x4A289C9: start_thread (pthread_create.c:300)

Updated

5 years ago
Assignee: nobody → mcmanus
Thanks Julian, somehow we lost the initializer when making that PRInterval based.. it matters in this case where you have a transaction that did not finish parsing the headers.

I also noted a problem that the default value that is used when parsing the headers is always the spdy one instead of the http/1 version in this case.

my bug.
status-firefox13: --- → unaffected
status-firefox14: --- → affected
Created attachment 617502 [details] [diff] [review]
patch 0

Initialize mIdleTimeout to 0. This is the way it was before the printerval change - which means that any connection that does not make it to onheadersvailable will not be reused unless the timeout is set explicitly such as by connection manager when it has leftover connections from syn-retry. I considered making it the default http KA value instead of 0, but that should be a separate patch if we do that.

Also fix up one case where the default http/1 KA value should have been used but the spdy value was used instead.
Attachment #617502 - Flags: review?(honzab.moz)
Attachment #617502 - Flags: review?(honzab.moz) → review+
Comment on attachment 617502 [details] [diff] [review]
patch 0

[Approval Request Comment]
Please see https://wiki.mozilla.org/Tree_Rules for information on mozilla-central landings that do not require approval.

Possible risk to Fennec Native 14 (if any):

This fixes a regression in HTTP timeout handling that was introduced on FF14.
Attachment #617502 - Flags: approval-mozilla-central?
Comment on attachment 617502 [details] [diff] [review]
patch 0

[Triage Comment]
Merge of 14 to Aurora has completed.  Please renominate for mozilla-aurora.
Attachment #617502 - Flags: approval-mozilla-central?
This is FF15
https://hg.mozilla.org/integration/mozilla-inbound/rev/946995f07a3f
status-firefox15: --- → fixed
Comment on attachment 617502 [details] [diff] [review]
patch 0

[Approval Request Comment]


This fixes a regression in HTTP timeout handling that was introduced on FF14.
Attachment #617502 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/946995f07a3f
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla15

Comment 8

5 years ago
Comment on attachment 617502 [details] [diff] [review]
patch 0

[Triage Comment]
Regression in 14, approved for Aurora 14.
Attachment #617502 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
ff14
https://hg.mozilla.org/releases/mozilla-aurora/rev/00d9852452e5
status-firefox14: affected → fixed
You need to log in before you can comment on or make changes to this bug.