581282 - Invalid memory usage in nsHttpHeaderArray serialization

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect)

Description

[Josh Matthews [:jdm]]

==14272== Thread 2:
==14272== Syscall param socketcall.sendmsg(msg.msg_iov[i]) points to uninitialised byte(s)
==14272==    at 0xC33208: sendmsg (in /lib/libpthread-2.11.2.so)
==14272==    by 0x58277DD: IPC::Channel::ChannelImpl::Send(IPC::Message*) (ipc_channel_posix.cc:678)
==14272==    by 0x5827E04: IPC::Channel::Send(IPC::Message*) (ipc_channel_posix.cc:821)
==14272==    by 0x55B6181: void DispatchToMethod<IPC::Channel, bool (IPC::Channel::*)(IPC::Message*), IPC::Message*>(IPC::Channel*, bool (IPC::Channel::*)(IPC::Message*), Tuple1<IPC::Message*> const&) (tuple.h:393)
==14272==    by 0x55B5FC6: RunnableMethod<IPC::Channel, bool (IPC::Channel::*)(IPC::Message*), Tuple1<IPC::Message*> >::Run() (task.h:307)
==14272==    by 0x57C4C0B: MessageLoop::RunTask(Task*) (message_loop.cc:339)
==14272==    by 0x57C4C74: MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) (message_loop.cc:347)
==14272==    by 0x57C5034: MessageLoop::DoWork() (message_loop.cc:447)
==14272==    by 0x5816BC9: base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) (message_pump_libevent.cc:309)
==14272==    by 0x57C4768: MessageLoop::RunInternal() (message_loop.cc:219)
==14272==    by 0x57C46E8: MessageLoop::RunHandler() (message_loop.cc:202)
==14272==    by 0x57C468C: MessageLoop::Run() (message_loop.cc:176)
==14272==  Address 0xe7a493c is 436 bytes inside a block of size 512 alloc'd
==14272==    at 0x4005CD2: realloc (vg_replace_malloc.c:476)
==14272==    by 0x4036C7A: moz_realloc (mozalloc.cpp:140)
==14272==    by 0x57D1CBD: Pickle::Resize(unsigned int) (pickle.cc:423)
==14272==    by 0x57D17BD: Pickle::BeginWrite(unsigned int) (pickle.cc:325)
==14272==    by 0x57D18CA: Pickle::WriteBytes(void const*, int) (pickle.cc:346)
==14272==    by 0x43BFDA6: IPC::ParamTraits<nsACString_internal>::Write(IPC::Message*, nsACString_internal const&) (IPCMessageUtils.h:134)
==14272==    by 0x5608C57: void IPC::WriteParam<nsCAutoString>(IPC::Message*, nsCAutoString const&) (ipc_message_utils.h:124)
==14272==    by 0x5609036: IPC::ParamTraits<nsHttpAtom>::Write(IPC::Message*, nsHttpAtom const&) (PHttpChannelParams.h:104)
==14272==    by 0x5608C92: void IPC::WriteParam<nsHttpAtom>(IPC::Message*, nsHttpAtom const&) (ipc_message_utils.h:124)
==14272==    by 0x5609138: IPC::ParamTraits<nsHttpHeaderArray::nsEntry>::Write(IPC::Message*, nsHttpHeaderArray::nsEntry const&) (PHttpChannelParams.h:126)
==14272==    by 0x5608E07: void IPC::WriteParam<nsHttpHeaderArray::nsEntry>(IPC::Message*, nsHttpHeaderArray::nsEntry const&) (ipc_message_utils.h:124)
==14272==    by 0x5609C66: IPC::ParamTraits<nsTArray<nsHttpHeaderArray::nsEntry> >::Write(IPC::Message*, nsTArray<nsHttpHeaderArray::nsEntry> const&) (IPCMessageUtils.h:259)
==14272==  Uninitialised value was created by a stack allocation
==14272==    at 0x4480982: mozilla::net::HttpChannelParent::OnStartRequest(nsIRequest*, nsISupports*) (HttpChannelParent.cpp:200)
==14272==

I don't have this figured out yet, but I suspect it's somehow coming from the nsHttpResponseHead being constructed in the event of no response head being present.

Comment 1

[Steffen Imhof]

How would I trigger this bug? Just running with valgrind and browsing does not show this for me.

Comment 2

[Josh Matthews [:jdm]]

I suspect I saw it on programming.reddit.com, but I'm not positive about that.  Not every page triggers it.

Comment 3

[Jae-Seong Lee-Russo]

I see it on programming.reddit.com.  Let me investigate.

Comment 4

[Jae-Seong Lee-Russo]

I built firefox with the recommended setting (by https://developer.mozilla.org/en/Debugging_Mozilla_with_valgrind), and I got the different result on programming.reddit.com:

==13261== Thread 2:
==13261== Syscall param socketcall.sendmsg(msg.msg_iov[i]) points to uninitialised byte(s)
==13261==    at 0x404F468: sendmsg (socket.S:100)
==13261==    by 0x5A880E5: IPC::Channel::ChannelImpl::Send(IPC::Message*) (ipc_channel_posix.cc:678)
==13261==    by 0x5A8814C: IPC::Channel::Send(IPC::Message*) (ipc_channel_posix.cc:821)
==13261==    by 0x57F38AA: RunnableMethod<IPC::Channel, bool (IPC::Channel::*)(IPC::Message*), Tuple1<IPC::Message*> >::Run() (tuple.h:393)
==13261==    by 0x5A44F63: MessageLoop::RunTask(Task*) (message_loop.cc:339)
==13261==    by 0x5A45731: MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) (message_loop.cc:347)
==13261==    by 0x5A45ED4: MessageLoop::DoWork() (message_loop.cc:447)
==13261==    by 0x5A7D230: base::MessagePumpLibevent::Run(base::MessagePump::Delegate*) (message_pump_libevent.cc:309)
==13261==    by 0x5A45568: MessageLoop::RunInternal() (message_loop.cc:219)
==13261==    by 0x5A45584: MessageLoop::RunHandler() (message_loop.cc:202)
==13261==    by 0x5A455EF: MessageLoop::Run() (message_loop.cc:176)
==13261==    by 0x5A5A2E2: base::Thread::ThreadMain() (thread.cc:156)
==13261==  Address 0xc3e18a4 is 436 bytes inside a block of size 512 alloc'd
==13261==    at 0x4025016: realloc (vg_replace_malloc.c:525)
==13261==    by 0x6458EDE: moz_realloc (mozalloc.cpp:140)
==13261==    by 0x5A4BE5C: Pickle::Resize(unsigned int) (pickle.cc:423)
==13261==    by 0x5A4C301: Pickle::BeginWrite(unsigned int) (pickle.cc:325)
==13261==    by 0x5A4C341: Pickle::WriteBytes(void const*, int) (pickle.cc:346)
==13261==    by 0x58B2433: mozilla::net::PHttpChannelParent::SendOnStartRequest(nsHttpResponseHead const&, int const&, int const&, int const&, unsigned int const&, nsCString const&) (IPCMessageUtils.h:134)
==13261==    by 0x43B29FA: mozilla::net::HttpChannelParent::OnStartRequest(nsIRequest*, nsISupports*) (HttpChannelParent.cpp:217)
==13261==    by 0x439DF85: nsHttpChannel::CallOnStartRequest() (nsHttpChannel.cpp:732)
==13261==    by 0x43A31F1: nsHttpChannel::ContinueProcessNormal(unsigned int) (nsHttpChannel.cpp:1087)
==13261==    by 0x43A33FD: nsHttpChannel::ProcessNormal() (nsHttpChannel.cpp:1024)
==13261==    by 0x43A7D2D: nsHttpChannel::ProcessResponse() (nsHttpChannel.cpp:974)
==13261==    by 0x43A8566: nsHttpChannel::OnStartRequest(nsIRequest*, nsISupports*) (nsHttpChannel.cpp:3564)
==13261==  Uninitialised value was created by a stack allocation
==13261==    at 0x43B2706: mozilla::net::HttpChannelParent::OnStartRequest(nsIRequest*, nsISupports*) (HttpChannelParent.cpp:200)
==13261==

Comment 5

[Josh Matthews [:jdm]]

I don't see what's different about it at all except that the stack under OnStartRequest has changed, I assume because of the async redirection patch that landed.

Comment 6

[Jae-Seong Lee-Russo]

Attachment: patch

GetCacheTokenExpirationTime (http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/HttpChannelParent.cpp#208) returned NS_ERROR_NOT_AVAILABLE because !mCacheEntry.

Comment 7

[Josh Matthews [:jdm]]

Comment on attachment 463039 [details] [diff] [review]
patch

I'm passing this off to somebody who knows what a sensible default is here.

Comment 8

[Jason Duell]

Setting the HttpChannelChild's mCacheExpiration time in OnStartRequest to nsICache::NO_EXPIRATION_TIME looks right (that's the default value in the child at construction, at least, so it's as if we just left it alone).

Cc-ing Michal and Byron in case that's deeply wrong.

Comment 9

[Jason Duell]

Hmm.

The NO_EXPIRATION_TIME fix looks correct, in that we'd give a bogus expirationTime to the child otherwise.  

But sending a random expirationTime shouldn't cause invalid memory usage in the IPDL serialization code.  I don't see how this patch actually fixes this bug.

Comment 10

[Jae-Seong Lee-Russo]

valgrind is pointing to the wrong place.  After applying the patch, I didn't see the valgrind warning anymore.

Comment 11

[Michal Novotny [:michal]]

I'm trying to reproduce it but I'm out of luck. Are there any known steps to reproduce? Browsing through programming.reddit.com is stable for me.

Comment 12

[Jae-Seong Lee-Russo]

mozconfig: enable-debug, disable-optimize, disable-jemalloc, enable-valgrind

valgrind --tool=memcheck --trace-children=yes (optional: --track-origins=yes) ~~~ test-ipc.xul

Load http://www.reddit.com, and you should see the output in the description.  Go to the site & wait, there is no need to browse.

Comment 13

[Jason Duell]

Landed as part of 

http://hg.mozilla.org/projects/electrolysis/rev/5936477a4dc8

Note that this wasn't an actual behavior bug--just a use before init that valgrind didn't like.