From e8ba2c61e8508c15e99806303bf13c9fd3afbae9 Mon Sep 17 00:00:00 2001 From: Chris Jones Date: Thu, 17 Dec 2009 18:12:01 -0600 Subject: [PATCH] bug 535298: IPDL unit test for use-after-free crashes after RPC errors. r=test-only --- ipc/ipdl/test/cxx/IPDLUnitTests.h | 7 ++ ipc/ipdl/test/cxx/IPDLUnitTests.template.cpp | 102 ++++++++--------- ipc/ipdl/test/cxx/Makefile.in | 1 + ipc/ipdl/test/cxx/PTestRPCErrorCleanup.ipdl | 11 ++ ipc/ipdl/test/cxx/TestRPCErrorCleanup.cpp | 112 +++++++++++++++++++ ipc/ipdl/test/cxx/TestRPCErrorCleanup.h | 55 +++++++++ ipc/ipdl/test/cxx/ipdl.mk | 1 + 7 files changed, 233 insertions(+), 56 deletions(-) create mode 100644 ipc/ipdl/test/cxx/PTestRPCErrorCleanup.ipdl create mode 100644 ipc/ipdl/test/cxx/TestRPCErrorCleanup.cpp create mode 100644 ipc/ipdl/test/cxx/TestRPCErrorCleanup.h diff --git a/ipc/ipdl/test/cxx/IPDLUnitTests.h b/ipc/ipdl/test/cxx/IPDLUnitTests.h index 3a2e7020f0b1..e08b70514953 100644 --- a/ipc/ipdl/test/cxx/IPDLUnitTests.h +++ b/ipc/ipdl/test/cxx/IPDLUnitTests.h @@ -100,6 +100,11 @@ inline void passed(const char* fmt, ...) //----------------------------------------------------------------------------- // parent process only +class IPDLUnitTestSubprocess; + +extern void* gParentActor; +extern IPDLUnitTestSubprocess* gSubprocess; + void IPDLUnitTestMain(void* aData); void QuitParent(); @@ -107,6 +112,8 @@ void QuitParent(); //----------------------------------------------------------------------------- // child process only +extern void* gChildActor; + void IPDLUnitTestChildInit(IPC::Channel* transport, base::ProcessHandle parent, MessageLoop* worker); diff --git a/ipc/ipdl/test/cxx/IPDLUnitTests.template.cpp b/ipc/ipdl/test/cxx/IPDLUnitTests.template.cpp index bd4241f5a130..56ce18f808f4 100644 --- a/ipc/ipdl/test/cxx/IPDLUnitTests.template.cpp +++ b/ipc/ipdl/test/cxx/IPDLUnitTests.template.cpp @@ -21,6 +21,11 @@ ${INCLUDES} using mozilla::_ipdltest::IPDLUnitTestSubprocess; using mozilla::_ipdltest::IPDLUnitTestThreadChild; +void* mozilla::_ipdltest::gParentActor; +IPDLUnitTestSubprocess* mozilla::_ipdltest::gSubprocess; + +void* mozilla::_ipdltest::gChildActor; + //----------------------------------------------------------------------------- // data/functions accessed by both parent and child processes @@ -111,10 +116,46 @@ IPDLUnitTest() //----------------------------------------------------------------------------- // parent process only -namespace { +namespace mozilla { +namespace _ipdltest { -void* gParentActor = NULL; -IPDLUnitTestSubprocess* gSubprocess; +void +IPDLUnitTestMain(void* aData) +{ + char* testString = reinterpret_cast(aData); + IPDLUnitTestType test = IPDLUnitTestFromString(testString); + if (!test) { + // use this instead of |fail()| because we don't know what the test is + fprintf(stderr, MOZ_IPDL_TESTFAIL_LABEL "| %s | unknown unit test %s\\n", + "<--->", testString); + NS_RUNTIMEABORT("can't continue"); + } + gIPDLUnitTestName = testString; + + std::vector testCaseArgs; + testCaseArgs.push_back(testString); + + gSubprocess = new IPDLUnitTestSubprocess(); + if (!gSubprocess->SyncLaunch(testCaseArgs)) + fail("problem launching subprocess"); + + IPC::Channel* transport = gSubprocess->GetChannel(); + if (!transport) + fail("no transport"); + + base::ProcessHandle child = gSubprocess->GetChildProcessHandle(); + + switch (test) { +//----------------------------------------------------------------------------- +//===== TEMPLATED ===== +${PARENT_MAIN_CASES} +//----------------------------------------------------------------------------- + + default: + fail("not reached"); + return; // unreached + } +} void DeleteParentActor() @@ -158,50 +199,6 @@ DeferredParentShutdown() NewRunnableFunction(DeleteSubprocess, MessageLoop::current())); } -} - - -namespace mozilla { -namespace _ipdltest { - -void -IPDLUnitTestMain(void* aData) -{ - char* testString = reinterpret_cast(aData); - IPDLUnitTestType test = IPDLUnitTestFromString(testString); - if (!test) { - // use this instead of |fail()| because we don't know what the test is - fprintf(stderr, MOZ_IPDL_TESTFAIL_LABEL "| %s | unknown unit test %s\\n", - "<--->", testString); - NS_RUNTIMEABORT("can't continue"); - } - gIPDLUnitTestName = testString; - - std::vector testCaseArgs; - testCaseArgs.push_back(testString); - - gSubprocess = new IPDLUnitTestSubprocess(); - if (!gSubprocess->SyncLaunch(testCaseArgs)) - fail("problem launching subprocess"); - - IPC::Channel* transport = gSubprocess->GetChannel(); - if (!transport) - fail("no transport"); - - base::ProcessHandle child = gSubprocess->GetChildProcessHandle(); - - switch (test) { -//----------------------------------------------------------------------------- -//===== TEMPLATED ===== -${PARENT_MAIN_CASES} -//----------------------------------------------------------------------------- - - default: - fail("not reached"); - return; // unreached - } -} - void QuitParent() { @@ -218,9 +215,8 @@ QuitParent() //----------------------------------------------------------------------------- // child process only -namespace { - -void* gChildActor = NULL; +namespace mozilla { +namespace _ipdltest { void DeleteChildActor() @@ -237,12 +233,6 @@ ${CHILD_DELETE_CASES} } } -} - - -namespace mozilla { -namespace _ipdltest { - void IPDLUnitTestChildInit(IPC::Channel* transport, base::ProcessHandle parent, diff --git a/ipc/ipdl/test/cxx/Makefile.in b/ipc/ipdl/test/cxx/Makefile.in index 4c3b61be2f24..5a21432e6db5 100644 --- a/ipc/ipdl/test/cxx/Makefile.in +++ b/ipc/ipdl/test/cxx/Makefile.in @@ -60,6 +60,7 @@ EXPORT_LIBRARY = 1 # Please keep these organized in the order "easy"-to-"hard" IPDLTESTS = \ TestSanity \ + TestRPCErrorCleanup \ TestLatency \ TestRPCRaces \ TestManyChildAllocs \ diff --git a/ipc/ipdl/test/cxx/PTestRPCErrorCleanup.ipdl b/ipc/ipdl/test/cxx/PTestRPCErrorCleanup.ipdl new file mode 100644 index 000000000000..509ecf8a1c1f --- /dev/null +++ b/ipc/ipdl/test/cxx/PTestRPCErrorCleanup.ipdl @@ -0,0 +1,11 @@ +namespace mozilla { +namespace _ipdltest { + +rpc protocol PTestRPCErrorCleanup { +child: + rpc Error(); + rpc __delete__(); +}; + +} // namespace _ipdltest +} // namespace mozilla diff --git a/ipc/ipdl/test/cxx/TestRPCErrorCleanup.cpp b/ipc/ipdl/test/cxx/TestRPCErrorCleanup.cpp new file mode 100644 index 000000000000..1b685b795353 --- /dev/null +++ b/ipc/ipdl/test/cxx/TestRPCErrorCleanup.cpp @@ -0,0 +1,112 @@ +#include "TestRPCErrorCleanup.h" + +#include "IPDLUnitTests.h" // fail etc. +#include "IPDLUnitTestSubprocess.h" + +namespace mozilla { +namespace _ipdltest { + +//----------------------------------------------------------------------------- +// parent + +// NB: this test does its own shutdown, rather than going through +// QuitParent(), because it's testing degenerate edge cases + +void DeleteTheWorld() +{ + delete static_cast(gParentActor); + gParentActor = NULL; + delete gSubprocess; + gSubprocess = NULL; +} + +void Done() +{ + static NS_DEFINE_CID(kAppShellCID, NS_APPSHELL_CID); + nsCOMPtr appShell (do_GetService(kAppShellCID)); + appShell->Exit(); + + passed(__FILE__); +} + +TestRPCErrorCleanupParent::TestRPCErrorCleanupParent() +{ + MOZ_COUNT_CTOR(TestRPCErrorCleanupParent); +} + +TestRPCErrorCleanupParent::~TestRPCErrorCleanupParent() +{ + MOZ_COUNT_DTOR(TestRPCErrorCleanupParent); +} + +void +TestRPCErrorCleanupParent::Main() +{ + // This test models the following sequence of events + // + // (1) Parent: RPC out-call + // (2) Child: crash + // --[Parent-only hereafter]-- + // (3) RPC out-call return false + // (4) Close() + // --[event loop]-- + // (5) delete parentActor + // (6) delete childProcess + // --[event loop]-- + // (7) Channel::OnError notification + // --[event loop]-- + // (8) Done, quit + // + // See bug 535298 and friends; this seqeunce of events captures + // three differnent potential errors + // - Close()-after-error (semantic error previously) + // - use-after-free of parentActor + // - use-after-free of channel + // + // Because of legacy constraints related to nsNPAPI* code, we need + // to ensure that this sequence of events can occur without + // errors/crashes. + + MessageLoop::current()->PostTask( + FROM_HERE, NewRunnableFunction(DeleteTheWorld)); + + // it's a failure if this *succeeds* + if (CallError()) + fail("expected an error!"); + + // it's OK to Close() a channel after an error, because nsNPAPI* + // wants to do this + Close(); + + // we know that this event *must* be after the MaybeError + // notification enqueued by AsyncChannel, because that event is + // enqueued within the same mutex that ends up signaling the + // wakeup-on-error of |CallError()| above + MessageLoop::current()->PostTask(FROM_HERE, NewRunnableFunction(Done)); +} + + +//----------------------------------------------------------------------------- +// child + +TestRPCErrorCleanupChild::TestRPCErrorCleanupChild() +{ + MOZ_COUNT_CTOR(TestRPCErrorCleanupChild); +} + +TestRPCErrorCleanupChild::~TestRPCErrorCleanupChild() +{ + MOZ_COUNT_DTOR(TestRPCErrorCleanupChild); +} + +bool +TestRPCErrorCleanupChild::AnswerError() +{ + _exit(0); + NS_RUNTIMEABORT("unreached"); + return false; +} + + +} // namespace _ipdltest +} // namespace mozilla diff --git a/ipc/ipdl/test/cxx/TestRPCErrorCleanup.h b/ipc/ipdl/test/cxx/TestRPCErrorCleanup.h new file mode 100644 index 000000000000..3a4294b65bf6 --- /dev/null +++ b/ipc/ipdl/test/cxx/TestRPCErrorCleanup.h @@ -0,0 +1,55 @@ +#ifndef mozilla__ipdltest_TestRPCErrorCleanup_h +#define mozilla__ipdltest_TestRPCErrorCleanup_h 1 + +#include "mozilla/_ipdltest/IPDLUnitTests.h" + +#include "mozilla/_ipdltest/PTestRPCErrorCleanupParent.h" +#include "mozilla/_ipdltest/PTestRPCErrorCleanupChild.h" + +namespace mozilla { +namespace _ipdltest { + + +class TestRPCErrorCleanupParent : + public PTestRPCErrorCleanupParent +{ +public: + TestRPCErrorCleanupParent(); + virtual ~TestRPCErrorCleanupParent(); + + void Main(); + +protected: + NS_OVERRIDE + virtual void ActorDestroy(ActorDestroyReason why) + { + if (AbnormalShutdown != why) + fail("unexpected destruction!"); + } +}; + + +class TestRPCErrorCleanupChild : + public PTestRPCErrorCleanupChild +{ +public: + TestRPCErrorCleanupChild(); + virtual ~TestRPCErrorCleanupChild(); + +protected: + NS_OVERRIDE + virtual bool AnswerError(); + + NS_OVERRIDE + virtual void ActorDestroy(ActorDestroyReason why) + { + fail("should have 'crashed'!"); + } +}; + + +} // namespace _ipdltest +} // namespace mozilla + + +#endif // ifndef mozilla__ipdltest_TestRPCErrorCleanup_h diff --git a/ipc/ipdl/test/cxx/ipdl.mk b/ipc/ipdl/test/cxx/ipdl.mk index 53512dc23b84..e2282b1d139a 100644 --- a/ipc/ipdl/test/cxx/ipdl.mk +++ b/ipc/ipdl/test/cxx/ipdl.mk @@ -7,6 +7,7 @@ IPDLSRCS = \ PTestLatency.ipdl \ PTestManyChildAllocs.ipdl \ PTestManyChildAllocsSub.ipdl \ + PTestRPCErrorCleanup.ipdl \ PTestRPCRaces.ipdl \ PTestSanity.ipdl \ PTestShmem.ipdl \