diff --git a/dom/indexedDB/PBackgroundIDBSharedTypes.ipdlh b/dom/indexedDB/PBackgroundIDBSharedTypes.ipdlh index 371bf58248cd..aabde3c8e89a 100644 --- a/dom/indexedDB/PBackgroundIDBSharedTypes.ipdlh +++ b/dom/indexedDB/PBackgroundIDBSharedTypes.ipdlh @@ -35,7 +35,7 @@ using class mozilla::dom::indexedDB::KeyPath using mozilla::dom::quota::PersistenceType from "mozilla/dom/quota/PersistenceType.h"; -[MoveOnly=data] using mozilla::SerializedStructuredCloneBuffer +[MoveOnly] using mozilla::SerializedStructuredCloneBuffer from "mozilla/ipc/SerializedStructuredCloneBuffer.h"; namespace mozilla { diff --git a/dom/ipc/ContentChild.cpp b/dom/ipc/ContentChild.cpp index 0c5b6f30f96a..d22da676a6d0 100644 --- a/dom/ipc/ContentChild.cpp +++ b/dom/ipc/ContentChild.cpp @@ -677,7 +677,7 @@ NS_INTERFACE_MAP_END mozilla::ipc::IPCResult ContentChild::RecvSetXPCOMProcessAttributes( XPCOMInitData&& aXPCOMInit, const StructuredCloneData& aInitialData, FullLookAndFeel&& aLookAndFeelData, dom::SystemFontList&& aFontList, - Maybe&& aSharedUASheetHandle, + const Maybe& aSharedUASheetHandle, const uintptr_t& aSharedUASheetAddress, nsTArray&& aSharedFontListBlocks) { if (!sShutdownCanary) { @@ -693,7 +693,7 @@ mozilla::ipc::IPCResult ContentChild::RecvSetXPCOMProcessAttributes( #endif gfx::gfxVars::SetValuesForInitialize(aXPCOMInit.gfxNonDefaultVarUpdates()); - InitSharedUASheets(std::move(aSharedUASheetHandle), aSharedUASheetAddress); + InitSharedUASheets(aSharedUASheetHandle, aSharedUASheetAddress); InitXPCOM(std::move(aXPCOMInit), aInitialData); InitGraphicsDeviceData(aXPCOMInit.contentDeviceData()); @@ -1313,7 +1313,7 @@ void ContentChild::InitGraphicsDeviceData(const ContentDeviceData& aData) { gfxPlatform::InitChild(aData); } -void ContentChild::InitSharedUASheets(Maybe&& aHandle, +void ContentChild::InitSharedUASheets(const Maybe& aHandle, uintptr_t aAddress) { MOZ_ASSERT_IF(!aHandle, !aAddress); @@ -1324,7 +1324,7 @@ void ContentChild::InitSharedUASheets(Maybe&& aHandle, // Map the shared memory storing the user agent style sheets. Do this as // early as possible to maximize the chance of being able to map at the // address we want. - GlobalStyleSheetCache::SetSharedMemory(std::move(*aHandle), aAddress); + GlobalStyleSheetCache::SetSharedMemory(*aHandle, aAddress); } void ContentChild::InitXPCOM( @@ -2520,10 +2520,10 @@ mozilla::ipc::IPCResult ContentChild::RecvRebuildFontList( mozilla::ipc::IPCResult ContentChild::RecvFontListShmBlockAdded( const uint32_t& aGeneration, const uint32_t& aIndex, - base::SharedMemoryHandle&& aHandle) { + const base::SharedMemoryHandle& aHandle) { if (gfxPlatform::Initialized()) { gfxPlatformFontList::PlatformFontList()->ShmBlockAdded(aGeneration, aIndex, - std::move(aHandle)); + aHandle); } return IPC_OK(); } diff --git a/dom/ipc/ContentChild.h b/dom/ipc/ContentChild.h index 2d7d85573ede..cbc1cfb713f1 100644 --- a/dom/ipc/ContentChild.h +++ b/dom/ipc/ContentChild.h @@ -126,7 +126,7 @@ class ContentChild final : public PContentChild, void InitXPCOM(XPCOMInitData&& aXPCOMInit, const mozilla::dom::ipc::StructuredCloneData& aInitialData); - void InitSharedUASheets(Maybe&& aHandle, + void InitSharedUASheets(const Maybe& aHandle, uintptr_t aAddress); void InitGraphicsDeviceData(const ContentDeviceData& aData); @@ -358,7 +358,7 @@ class ContentChild final : public PContentChild, mozilla::ipc::IPCResult RecvRebuildFontList(const bool& aFullRebuild); mozilla::ipc::IPCResult RecvFontListShmBlockAdded( const uint32_t& aGeneration, const uint32_t& aIndex, - base::SharedMemoryHandle&& aHandle); + const base::SharedMemoryHandle& aHandle); mozilla::ipc::IPCResult RecvUpdateAppLocales( nsTArray&& aAppLocales); @@ -542,7 +542,7 @@ class ContentChild final : public PContentChild, mozilla::ipc::IPCResult RecvSetXPCOMProcessAttributes( XPCOMInitData&& aXPCOMInit, const StructuredCloneData& aInitialData, FullLookAndFeel&& aLookAndFeelData, SystemFontList&& aFontList, - Maybe&& aSharedUASheetHandle, + const Maybe& aSharedUASheetHandle, const uintptr_t& aSharedUASheetAddress, nsTArray&& aSharedFontListBlocks); diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index cb160a2e4969..74023c1c57b6 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -1637,8 +1637,7 @@ void ContentParent::BroadcastShmBlockAdded(uint32_t aGeneration, // request the block as needed, at some performance cost. continue; } - Unused << cp->SendFontListShmBlockAdded(aGeneration, aIndex, - std::move(handle)); + Unused << cp->SendFontListShmBlockAdded(aGeneration, aIndex, handle); } } @@ -2977,14 +2976,14 @@ bool ContentParent::InitInternal(ProcessPriority aInitialPriority) { SharedMemoryHandle handle; if (sheetCache->ShareToProcess(OtherPid(), &handle)) { - sharedUASheetHandle.emplace(std::move(handle)); + sharedUASheetHandle.emplace(handle); } else { sharedUASheetAddress = 0; } Unused << SendSetXPCOMProcessAttributes( - xpcomInit, initialData, lnf, fontList, std::move(sharedUASheetHandle), - sharedUASheetAddress, std::move(sharedFontListBlocks)); + xpcomInit, initialData, lnf, fontList, sharedUASheetHandle, + sharedUASheetAddress, sharedFontListBlocks); ipc::WritableSharedMap* sharedData = nsFrameMessageManager::sParentProcessManager->SharedData(); diff --git a/dom/ipc/DOMTypes.ipdlh b/dom/ipc/DOMTypes.ipdlh index ea46c891a905..3c3c253c9064 100644 --- a/dom/ipc/DOMTypes.ipdlh +++ b/dom/ipc/DOMTypes.ipdlh @@ -21,7 +21,7 @@ include ProtocolTypes; using struct mozilla::void_t from "mozilla/ipc/IPCCore.h"; -[MoveOnly=data] using struct mozilla::SerializedStructuredCloneBuffer +[MoveOnly] using struct mozilla::SerializedStructuredCloneBuffer from "mozilla/ipc/SerializedStructuredCloneBuffer.h"; using class mozilla::dom::LoadingSessionHistoryInfo diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 969f5d54220c..a37d332543f0 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -117,14 +117,14 @@ using mozilla::Telemetry::ChildEventData from "mozilla/TelemetryComms.h"; #endif // defined(XP_WIN) using mozilla::Telemetry::DiscardedData from "mozilla/TelemetryComms.h"; -[MoveOnly] using mozilla::CrossProcessMutexHandle from "mozilla/ipc/CrossProcessMutex.h"; +using mozilla::CrossProcessMutexHandle from "mozilla/ipc/CrossProcessMutex.h"; using mozilla::dom::MaybeDiscardedBrowsingContext from "mozilla/dom/BrowsingContext.h"; using mozilla::dom::BrowsingContextTransaction from "mozilla/dom/BrowsingContext.h"; using mozilla::dom::BrowsingContextInitializer from "mozilla/dom/BrowsingContext.h"; using mozilla::dom::PermitUnloadResult from "nsIContentViewer.h"; using mozilla::dom::MaybeDiscardedWindowContext from "mozilla/dom/WindowContext.h"; using mozilla::dom::WindowContextTransaction from "mozilla/dom/WindowContext.h"; -[MoveOnly] using base::SharedMemoryHandle from "base/shared_memory.h"; +using base::SharedMemoryHandle from "base/shared_memory.h"; using mozilla::fontlist::Pointer from "SharedFontList.h"; using gfxSparseBitSet from "gfxFontUtils.h"; using FontVisibility from "gfxFontEntry.h"; diff --git a/dom/media/gmp/ChromiumCDMParent.cpp b/dom/media/gmp/ChromiumCDMParent.cpp index bc7e0c05dfa1..cdf74e1b8820 100644 --- a/dom/media/gmp/ChromiumCDMParent.cpp +++ b/dom/media/gmp/ChromiumCDMParent.cpp @@ -379,7 +379,7 @@ bool ChromiumCDMParent::InitCDMInputBuffer(gmp::CDMInputBuffer& aBuffer, ? crypto.mIV : crypto.mConstantIV; aBuffer = gmp::CDMInputBuffer( - shmem, crypto.mKeyId, iv, aSample->mTime.ToMicroseconds(), + std::move(shmem), crypto.mKeyId, iv, aSample->mTime.ToMicroseconds(), aSample->mDuration.ToMicroseconds(), crypto.mPlainSizes, crypto.mEncryptedSizes, crypto.mCryptByteBlock, crypto.mSkipByteBlock, encryptionScheme); diff --git a/gfx/layers/CanvasDrawEventRecorder.cpp b/gfx/layers/CanvasDrawEventRecorder.cpp index 1c9eec62cbf7..193ee63c2c50 100644 --- a/gfx/layers/CanvasDrawEventRecorder.cpp +++ b/gfx/layers/CanvasDrawEventRecorder.cpp @@ -84,13 +84,13 @@ bool CanvasEventRingBuffer::InitWriter( } bool CanvasEventRingBuffer::InitReader( - ipc::SharedMemoryBasic::Handle aReadHandle, - CrossProcessSemaphoreHandle aReaderSem, - CrossProcessSemaphoreHandle aWriterSem, + const ipc::SharedMemoryBasic::Handle& aReadHandle, + const CrossProcessSemaphoreHandle& aReaderSem, + const CrossProcessSemaphoreHandle& aWriterSem, UniquePtr aReaderServices) { mSharedMemory = MakeAndAddRef(); if (NS_WARN_IF(!mSharedMemory->SetHandle( - std::move(aReadHandle), ipc::SharedMemory::RightsReadWrite)) || + aReadHandle, ipc::SharedMemory::RightsReadWrite)) || NS_WARN_IF(!mSharedMemory->Map(kShmemSize))) { return false; } @@ -100,9 +100,9 @@ bool CanvasEventRingBuffer::InitReader( mBuf = static_cast(mSharedMemory->memory()); mRead = reinterpret_cast(mBuf + kStreamSize); mWrite = reinterpret_cast(mBuf + kStreamSize + kCacheLineSize); - mReaderSemaphore.reset(CrossProcessSemaphore::Create(std::move(aReaderSem))); + mReaderSemaphore.reset(CrossProcessSemaphore::Create(aReaderSem)); mReaderSemaphore->CloseHandle(); - mWriterSemaphore.reset(CrossProcessSemaphore::Create(std::move(aWriterSem))); + mWriterSemaphore.reset(CrossProcessSemaphore::Create(aWriterSem)); mWriterSemaphore->CloseHandle(); mReaderServices = std::move(aReaderServices); diff --git a/gfx/layers/CanvasDrawEventRecorder.h b/gfx/layers/CanvasDrawEventRecorder.h index a88e74cd2e14..7fb69669ec93 100644 --- a/gfx/layers/CanvasDrawEventRecorder.h +++ b/gfx/layers/CanvasDrawEventRecorder.h @@ -82,9 +82,9 @@ class CanvasEventRingBuffer final : public gfx::EventRingBuffer { * @param aReaderServices provides functions required by the reader * @returns true if initialization succeeds */ - bool InitReader(ipc::SharedMemoryBasic::Handle aReadHandle, - CrossProcessSemaphoreHandle aReaderSem, - CrossProcessSemaphoreHandle aWriterSem, + bool InitReader(const ipc::SharedMemoryBasic::Handle& aReadHandle, + const CrossProcessSemaphoreHandle& aReaderSem, + const CrossProcessSemaphoreHandle& aWriterSem, UniquePtr aReaderServices); bool good() const final { return mGood; } diff --git a/gfx/layers/SourceSurfaceSharedData.cpp b/gfx/layers/SourceSurfaceSharedData.cpp index 44484c5defdf..99aca41e23d7 100644 --- a/gfx/layers/SourceSurfaceSharedData.cpp +++ b/gfx/layers/SourceSurfaceSharedData.cpp @@ -30,10 +30,9 @@ using namespace mozilla::layers; namespace mozilla { namespace gfx { -void SourceSurfaceSharedDataWrapper::Init(const IntSize& aSize, int32_t aStride, - SurfaceFormat aFormat, - SharedMemoryBasic::Handle aHandle, - base::ProcessId aCreatorPid) { +void SourceSurfaceSharedDataWrapper::Init( + const IntSize& aSize, int32_t aStride, SurfaceFormat aFormat, + const SharedMemoryBasic::Handle& aHandle, base::ProcessId aCreatorPid) { MOZ_ASSERT(!mBuf); mSize = aSize; mStride = aStride; @@ -42,7 +41,7 @@ void SourceSurfaceSharedDataWrapper::Init(const IntSize& aSize, int32_t aStride, size_t len = GetAlignedDataLength(); mBuf = MakeAndAddRef(); - if (!mBuf->SetHandle(std::move(aHandle), ipc::SharedMemory::RightsReadOnly)) { + if (!mBuf->SetHandle(aHandle, ipc::SharedMemory::RightsReadOnly)) { MOZ_CRASH("Invalid shared memory handle!"); } diff --git a/gfx/layers/SourceSurfaceSharedData.h b/gfx/layers/SourceSurfaceSharedData.h index a77de53510d7..e6670ce6a890 100644 --- a/gfx/layers/SourceSurfaceSharedData.h +++ b/gfx/layers/SourceSurfaceSharedData.h @@ -47,7 +47,8 @@ class SourceSurfaceSharedDataWrapper final : public DataSourceSurface { mCreatorRef(true) {} void Init(const IntSize& aSize, int32_t aStride, SurfaceFormat aFormat, - SharedMemoryBasic::Handle aHandle, base::ProcessId aCreatorPid); + const SharedMemoryBasic::Handle& aHandle, + base::ProcessId aCreatorPid); void Init(SourceSurfaceSharedData* aSurface); diff --git a/gfx/layers/client/TextureClient.cpp b/gfx/layers/client/TextureClient.cpp index 9c8571b7e787..054e90ebfd14 100644 --- a/gfx/layers/client/TextureClient.cpp +++ b/gfx/layers/client/TextureClient.cpp @@ -1039,9 +1039,8 @@ bool TextureClient::InitIPDLActor(CompositableForwarder* aForwarder) { } PTextureChild* actor = aForwarder->GetTextureForwarder()->CreateTexture( - desc, std::move(readLockDescriptor), - aForwarder->GetCompositorBackendType(), GetFlags(), mSerial, - mExternalImageId, nullptr); + desc, readLockDescriptor, aForwarder->GetCompositorBackendType(), + GetFlags(), mSerial, mExternalImageId, nullptr); if (!actor) { gfxCriticalNote << static_cast(desc.type()) << ", " @@ -1106,10 +1105,9 @@ bool TextureClient::InitIPDLActor(KnowsCompositor* aKnowsCompositor) { mReadLock->Serialize(readLockDescriptor, GetAllocator()->GetParentPid()); } - PTextureChild* actor = - fwd->CreateTexture(desc, std::move(readLockDescriptor), - aKnowsCompositor->GetCompositorBackendType(), - GetFlags(), mSerial, mExternalImageId); + PTextureChild* actor = fwd->CreateTexture( + desc, readLockDescriptor, aKnowsCompositor->GetCompositorBackendType(), + GetFlags(), mSerial, mExternalImageId); if (!actor) { gfxCriticalNote << static_cast(desc.type()) << ", " << static_cast( @@ -1506,8 +1504,7 @@ class CrossProcessSemaphoreReadLock : public TextureReadLock { : mSemaphore(CrossProcessSemaphore::Create("TextureReadLock", 1)), mShared(false) {} explicit CrossProcessSemaphoreReadLock(CrossProcessSemaphoreHandle aHandle) - : mSemaphore(CrossProcessSemaphore::Create(std::move(aHandle))), - mShared(false) {} + : mSemaphore(CrossProcessSemaphore::Create(aHandle)), mShared(false) {} bool ReadLock() override { if (!IsValid()) { @@ -1540,7 +1537,7 @@ class CrossProcessSemaphoreReadLock : public TextureReadLock { // static already_AddRefed TextureReadLock::Deserialize( - ReadLockDescriptor&& aDescriptor, ISurfaceAllocator* aAllocator) { + const ReadLockDescriptor& aDescriptor, ISurfaceAllocator* aAllocator) { switch (aDescriptor.type()) { case ReadLockDescriptor::TShmemSection: { const ShmemSection& section = aDescriptor.get_ShmemSection(); @@ -1569,7 +1566,7 @@ already_AddRefed TextureReadLock::Deserialize( } case ReadLockDescriptor::TCrossProcessSemaphoreDescriptor: { return MakeAndAddRef( - std::move(aDescriptor.get_CrossProcessSemaphoreDescriptor().sem())); + aDescriptor.get_CrossProcessSemaphoreDescriptor().sem()); } case ReadLockDescriptor::Tnull_t: { return nullptr; diff --git a/gfx/layers/client/TextureClient.h b/gfx/layers/client/TextureClient.h index cbe17c0e8532..e04f6f394ada 100644 --- a/gfx/layers/client/TextureClient.h +++ b/gfx/layers/client/TextureClient.h @@ -195,7 +195,7 @@ class TextureReadLock { virtual bool IsValid() const = 0; static already_AddRefed Deserialize( - ReadLockDescriptor&& aDescriptor, ISurfaceAllocator* aAllocator); + const ReadLockDescriptor& aDescriptor, ISurfaceAllocator* aAllocator); virtual bool Serialize(ReadLockDescriptor& aOutput, base::ProcessId aOther) = 0; diff --git a/gfx/layers/composite/TextureHost.cpp b/gfx/layers/composite/TextureHost.cpp index 660fc3ca4730..261815c36d39 100644 --- a/gfx/layers/composite/TextureHost.cpp +++ b/gfx/layers/composite/TextureHost.cpp @@ -79,8 +79,8 @@ class TextureParent : public ParentActor { virtual ~TextureParent(); bool Init(const SurfaceDescriptor& aSharedData, - ReadLockDescriptor&& aReadLock, const LayersBackend& aLayersBackend, - const TextureFlags& aFlags); + const ReadLockDescriptor& aReadLock, + const LayersBackend& aLayersBackend, const TextureFlags& aFlags); void NotifyNotUsed(uint64_t aTransactionId); @@ -115,12 +115,12 @@ static bool WrapWithWebRenderTextureHost(ISurfaceAllocator* aDeallocator, //////////////////////////////////////////////////////////////////////////////// PTextureParent* TextureHost::CreateIPDLActor( HostIPCAllocator* aAllocator, const SurfaceDescriptor& aSharedData, - ReadLockDescriptor&& aReadLock, LayersBackend aLayersBackend, + const ReadLockDescriptor& aReadLock, LayersBackend aLayersBackend, TextureFlags aFlags, uint64_t aSerial, const wr::MaybeExternalImageId& aExternalImageId) { TextureParent* actor = new TextureParent(aAllocator, aSerial, aExternalImageId); - if (!actor->Init(aSharedData, std::move(aReadLock), aLayersBackend, aFlags)) { + if (!actor->Init(aSharedData, aReadLock, aLayersBackend, aFlags)) { actor->ActorDestroy(ipc::IProtocol::ActorDestroyReason::FailedConstructor); delete actor; return nullptr; @@ -182,7 +182,7 @@ already_AddRefed CreateDummyBufferTextureHost( } already_AddRefed TextureHost::Create( - const SurfaceDescriptor& aDesc, ReadLockDescriptor&& aReadLock, + const SurfaceDescriptor& aDesc, const ReadLockDescriptor& aReadLock, ISurfaceAllocator* aDeallocator, LayersBackend aBackend, TextureFlags aFlags, wr::MaybeExternalImageId& aExternalImageId) { RefPtr result; @@ -225,9 +225,8 @@ already_AddRefed TextureHost::Create( break; } - result = - TextureHost::Create(*realDesc, std::move(aReadLock), aDeallocator, - aBackend, aFlags, aExternalImageId); + result = TextureHost::Create(*realDesc, aReadLock, aDeallocator, aBackend, + aFlags, aExternalImageId); return result.forget(); } default: @@ -245,7 +244,7 @@ already_AddRefed TextureHost::Create( } if (result) { - result->DeserializeReadLock(std::move(aReadLock), aDeallocator); + result->DeserializeReadLock(aReadLock, aDeallocator); } return result.forget(); @@ -681,13 +680,13 @@ void BufferTextureHost::PushDisplayItems(wr::DisplayListBuilder& aBuilder, } } -void TextureHost::DeserializeReadLock(ReadLockDescriptor&& aDesc, +void TextureHost::DeserializeReadLock(const ReadLockDescriptor& aDesc, ISurfaceAllocator* aAllocator) { if (mReadLock) { return; } - mReadLock = TextureReadLock::Deserialize(std::move(aDesc), aAllocator); + mReadLock = TextureReadLock::Deserialize(aDesc, aAllocator); } void TextureHost::SetReadLocked() { @@ -1225,12 +1224,11 @@ void TextureParent::NotifyNotUsed(uint64_t aTransactionId) { } bool TextureParent::Init(const SurfaceDescriptor& aSharedData, - ReadLockDescriptor&& aReadLock, + const ReadLockDescriptor& aReadLock, const LayersBackend& aBackend, const TextureFlags& aFlags) { - mTextureHost = - TextureHost::Create(aSharedData, std::move(aReadLock), mSurfaceAllocator, - aBackend, aFlags, mExternalImageId); + mTextureHost = TextureHost::Create(aSharedData, aReadLock, mSurfaceAllocator, + aBackend, aFlags, mExternalImageId); if (mTextureHost) { mTextureHost->mActor = this; } diff --git a/gfx/layers/composite/TextureHost.h b/gfx/layers/composite/TextureHost.h index 0299f8e7e1bb..c58e2a65f389 100644 --- a/gfx/layers/composite/TextureHost.h +++ b/gfx/layers/composite/TextureHost.h @@ -409,7 +409,7 @@ class TextureHost : public AtomicRefCountedWithFinalize { * Factory method. */ static already_AddRefed Create( - const SurfaceDescriptor& aDesc, ReadLockDescriptor&& aReadLock, + const SurfaceDescriptor& aDesc, const ReadLockDescriptor& aReadLock, ISurfaceAllocator* aDeallocator, LayersBackend aBackend, TextureFlags aFlags, wr::MaybeExternalImageId& aExternalImageId); @@ -571,7 +571,7 @@ class TextureHost : public AtomicRefCountedWithFinalize { */ static PTextureParent* CreateIPDLActor( HostIPCAllocator* aAllocator, const SurfaceDescriptor& aSharedData, - ReadLockDescriptor&& aDescriptor, LayersBackend aLayersBackend, + const ReadLockDescriptor& aDescriptor, LayersBackend aLayersBackend, TextureFlags aFlags, uint64_t aSerial, const wr::MaybeExternalImageId& aExternalImageId); static bool DestroyIPDLActor(PTextureParent* actor); @@ -643,7 +643,7 @@ class TextureHost : public AtomicRefCountedWithFinalize { void SetLastFwdTransactionId(uint64_t aTransactionId); - void DeserializeReadLock(ReadLockDescriptor&& aDesc, + void DeserializeReadLock(const ReadLockDescriptor& aDesc, ISurfaceAllocator* aAllocator); void SetReadLocked(); diff --git a/gfx/layers/ipc/CanvasChild.cpp b/gfx/layers/ipc/CanvasChild.cpp index b6f2e3800246..11a9ebe6a074 100644 --- a/gfx/layers/ipc/CanvasChild.cpp +++ b/gfx/layers/ipc/CanvasChild.cpp @@ -149,8 +149,7 @@ void CanvasChild::EnsureRecorder(TextureType aTextureType) { } if (CanSend()) { - Unused << SendInitTranslator(mTextureType, std::move(handle), - std::move(readerSem), std::move(writerSem)); + Unused << SendInitTranslator(mTextureType, handle, readerSem, writerSem); } } diff --git a/gfx/layers/ipc/CanvasTranslator.cpp b/gfx/layers/ipc/CanvasTranslator.cpp index 94a2703c27b2..21d8e8da65cc 100644 --- a/gfx/layers/ipc/CanvasTranslator.cpp +++ b/gfx/layers/ipc/CanvasTranslator.cpp @@ -127,16 +127,15 @@ void CanvasTranslator::Bind(Endpoint&& aEndpoint) { mozilla::ipc::IPCResult CanvasTranslator::RecvInitTranslator( const TextureType& aTextureType, - ipc::SharedMemoryBasic::Handle&& aReadHandle, - CrossProcessSemaphoreHandle&& aReaderSem, - CrossProcessSemaphoreHandle&& aWriterSem) { + const ipc::SharedMemoryBasic::Handle& aReadHandle, + const CrossProcessSemaphoreHandle& aReaderSem, + const CrossProcessSemaphoreHandle& aWriterSem) { mTextureType = aTextureType; // We need to initialize the stream first, because it might be used to // communicate other failures back to the writer. mStream = MakeUnique(); - if (!mStream->InitReader(std::move(aReadHandle), std::move(aReaderSem), - std::move(aWriterSem), + if (!mStream->InitReader(aReadHandle, aReaderSem, aWriterSem, MakeUnique(this))) { return IPC_FAIL(this, "Failed to initialize ring buffer reader."); } diff --git a/gfx/layers/ipc/CanvasTranslator.h b/gfx/layers/ipc/CanvasTranslator.h index 99b7f7236f12..532efe4f72f7 100644 --- a/gfx/layers/ipc/CanvasTranslator.h +++ b/gfx/layers/ipc/CanvasTranslator.h @@ -58,9 +58,9 @@ class CanvasTranslator final : public gfx::InlineTranslator, */ ipc::IPCResult RecvInitTranslator( const TextureType& aTextureType, - ipc::SharedMemoryBasic::Handle&& aReadHandle, - CrossProcessSemaphoreHandle&& aReaderSem, - CrossProcessSemaphoreHandle&& aWriterSem); + const ipc::SharedMemoryBasic::Handle& aReadHandle, + const CrossProcessSemaphoreHandle& aReaderSem, + const CrossProcessSemaphoreHandle& aWriterSem); /** * Used to tell the CanvasTranslator to start translating again after it has diff --git a/gfx/layers/ipc/CompositorBridgeChild.cpp b/gfx/layers/ipc/CompositorBridgeChild.cpp index 994572e72117..c8aa3c3c4fb3 100644 --- a/gfx/layers/ipc/CompositorBridgeChild.cpp +++ b/gfx/layers/ipc/CompositorBridgeChild.cpp @@ -423,7 +423,7 @@ bool CompositorBridgeChild::SendStopFrameTimeRecording( } PTextureChild* CompositorBridgeChild::AllocPTextureChild( - const SurfaceDescriptor&, ReadLockDescriptor&, const LayersBackend&, + const SurfaceDescriptor&, const ReadLockDescriptor&, const LayersBackend&, const TextureFlags&, const LayersId&, const uint64_t& aSerial, const wr::MaybeExternalImageId& aExternalImageId) { return TextureClient::CreateIPDLActor(); @@ -555,7 +555,7 @@ CompositorBridgeChild::GetTileLockAllocator() { } PTextureChild* CompositorBridgeChild::CreateTexture( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor&& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, LayersBackend aLayersBackend, TextureFlags aFlags, uint64_t aSerial, wr::MaybeExternalImageId& aExternalImageId, nsISerialEventTarget* aTarget) { PTextureChild* textureChild = @@ -568,7 +568,7 @@ PTextureChild* CompositorBridgeChild::CreateTexture( } return SendPTextureConstructor( - textureChild, aSharedData, std::move(aReadLock), aLayersBackend, aFlags, + textureChild, aSharedData, aReadLock, aLayersBackend, aFlags, LayersId{0} /* FIXME? */, aSerial, aExternalImageId); } diff --git a/gfx/layers/ipc/CompositorBridgeChild.h b/gfx/layers/ipc/CompositorBridgeChild.h index 766beb841cc1..4f52559b9c70 100644 --- a/gfx/layers/ipc/CompositorBridgeChild.h +++ b/gfx/layers/ipc/CompositorBridgeChild.h @@ -93,7 +93,7 @@ class CompositorBridgeChild final : public PCompositorBridgeChild, const LayersId& aLayersId, nsTArray&& aJankedAnimations); PTextureChild* AllocPTextureChild( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, const LayersBackend& aLayersBackend, const TextureFlags& aFlags, const LayersId& aId, const uint64_t& aSerial, const wr::MaybeExternalImageId& aExternalImageId); @@ -103,7 +103,7 @@ class CompositorBridgeChild final : public PCompositorBridgeChild, mozilla::ipc::IPCResult RecvParentAsyncMessages( nsTArray&& aMessages); PTextureChild* CreateTexture(const SurfaceDescriptor& aSharedData, - ReadLockDescriptor&& aReadLock, + const ReadLockDescriptor& aReadLock, LayersBackend aLayersBackend, TextureFlags aFlags, uint64_t aSerial, wr::MaybeExternalImageId& aExternalImageId, diff --git a/gfx/layers/ipc/CompositorBridgeParent.cpp b/gfx/layers/ipc/CompositorBridgeParent.cpp index 99afcba8a237..a179cc2cf028 100644 --- a/gfx/layers/ipc/CompositorBridgeParent.cpp +++ b/gfx/layers/ipc/CompositorBridgeParent.cpp @@ -1877,11 +1877,11 @@ CompositorBridgeParent::GetGeckoContentControllerForRoot( } PTextureParent* CompositorBridgeParent::AllocPTextureParent( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, const LayersBackend& aLayersBackend, const TextureFlags& aFlags, const LayersId& aId, const uint64_t& aSerial, const wr::MaybeExternalImageId& aExternalImageId) { - return TextureHost::CreateIPDLActor(this, aSharedData, std::move(aReadLock), + return TextureHost::CreateIPDLActor(this, aSharedData, aReadLock, aLayersBackend, aFlags, aSerial, aExternalImageId); } diff --git a/gfx/layers/ipc/CompositorBridgeParent.h b/gfx/layers/ipc/CompositorBridgeParent.h index c1a1e6aba8d2..c18044d35ac4 100644 --- a/gfx/layers/ipc/CompositorBridgeParent.h +++ b/gfx/layers/ipc/CompositorBridgeParent.h @@ -195,7 +195,7 @@ class CompositorBridgeParentBase : public PCompositorBridgeParent, PAPZCTreeManagerParent* aActor) = 0; virtual PTextureParent* AllocPTextureParent( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, const LayersBackend& aBackend, const TextureFlags& aTextureFlags, const LayersId& id, const uint64_t& aSerial, const MaybeExternalImageId& aExternalImageId) = 0; @@ -360,7 +360,7 @@ class CompositorBridgeParent final : public CompositorBridgeParentBase, void SetFixedLayerMargins(ScreenIntCoord aTop, ScreenIntCoord aBottom); PTextureParent* AllocPTextureParent( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, const LayersBackend& aLayersBackend, const TextureFlags& aFlags, const LayersId& aId, const uint64_t& aSerial, const wr::MaybeExternalImageId& aExternalImageId) override; diff --git a/gfx/layers/ipc/CompositorManagerParent.cpp b/gfx/layers/ipc/CompositorManagerParent.cpp index b1d94eafa32b..2d621da52a1b 100644 --- a/gfx/layers/ipc/CompositorManagerParent.cpp +++ b/gfx/layers/ipc/CompositorManagerParent.cpp @@ -262,8 +262,8 @@ CompositorManagerParent::AllocPCompositorBridgeParent( } mozilla::ipc::IPCResult CompositorManagerParent::RecvAddSharedSurface( - const wr::ExternalImageId& aId, SurfaceDescriptorShared&& aDesc) { - SharedSurfacesParent::Add(aId, std::move(aDesc), OtherPid()); + const wr::ExternalImageId& aId, const SurfaceDescriptorShared& aDesc) { + SharedSurfacesParent::Add(aId, aDesc, OtherPid()); return IPC_OK(); } diff --git a/gfx/layers/ipc/CompositorManagerParent.h b/gfx/layers/ipc/CompositorManagerParent.h index 4c117b802aaa..2933f4025085 100644 --- a/gfx/layers/ipc/CompositorManagerParent.h +++ b/gfx/layers/ipc/CompositorManagerParent.h @@ -40,8 +40,8 @@ class CompositorManagerParent final : public PCompositorManagerParent { bool aUseExternalSurfaceSize, const gfx::IntSize& aSurfaceSize); - mozilla::ipc::IPCResult RecvAddSharedSurface(const wr::ExternalImageId& aId, - SurfaceDescriptorShared&& aDesc); + mozilla::ipc::IPCResult RecvAddSharedSurface( + const wr::ExternalImageId& aId, const SurfaceDescriptorShared& aDesc); mozilla::ipc::IPCResult RecvRemoveSharedSurface( const wr::ExternalImageId& aId); mozilla::ipc::IPCResult RecvReportSharedSurfacesMemory( diff --git a/gfx/layers/ipc/ContentCompositorBridgeParent.cpp b/gfx/layers/ipc/ContentCompositorBridgeParent.cpp index 9b3a641ad0e6..dfa5ffbf71f2 100644 --- a/gfx/layers/ipc/ContentCompositorBridgeParent.cpp +++ b/gfx/layers/ipc/ContentCompositorBridgeParent.cpp @@ -381,7 +381,7 @@ ContentCompositorBridgeParent::~ContentCompositorBridgeParent() { } PTextureParent* ContentCompositorBridgeParent::AllocPTextureParent( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, const LayersBackend& aLayersBackend, const TextureFlags& aFlags, const LayersId& aId, const uint64_t& aSerial, const wr::MaybeExternalImageId& aExternalImageId) { @@ -408,7 +408,7 @@ PTextureParent* ContentCompositorBridgeParent::AllocPTextureParent( << "Texture backend is wrong"; } - return TextureHost::CreateIPDLActor(this, aSharedData, std::move(aReadLock), + return TextureHost::CreateIPDLActor(this, aSharedData, aReadLock, aLayersBackend, aFlags, aSerial, aExternalImageId); } diff --git a/gfx/layers/ipc/ContentCompositorBridgeParent.h b/gfx/layers/ipc/ContentCompositorBridgeParent.h index cf5f51969939..2657f9bf4321 100644 --- a/gfx/layers/ipc/ContentCompositorBridgeParent.h +++ b/gfx/layers/ipc/ContentCompositorBridgeParent.h @@ -130,7 +130,7 @@ class ContentCompositorBridgeParent final : public CompositorBridgeParentBase { TimeStamp& aCompositeStart, TimeStamp& aCompositeEnd); PTextureParent* AllocPTextureParent( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, const LayersBackend& aLayersBackend, const TextureFlags& aFlags, const LayersId& aId, const uint64_t& aSerial, const wr::MaybeExternalImageId& aExternalImageId) override; diff --git a/gfx/layers/ipc/ISurfaceAllocator.cpp b/gfx/layers/ipc/ISurfaceAllocator.cpp index cb0196a33e90..bf38b22226e1 100644 --- a/gfx/layers/ipc/ISurfaceAllocator.cpp +++ b/gfx/layers/ipc/ISurfaceAllocator.cpp @@ -20,8 +20,13 @@ mozilla::Atomic GfxMemoryImageReporter::sAmount(0); /* static */ uint32_t CompositableForwarder::GetMaxFileDescriptorsPerMessage() { +#if defined(OS_POSIX) static const uint32_t kMaxFileDescriptors = - IPC::Message::MAX_DESCRIPTORS_PER_MESSAGE; + FileDescriptorSet::MAX_DESCRIPTORS_PER_MESSAGE; +#else + // default number that works everywhere else + static const uint32_t kMaxFileDescriptors = 250; +#endif return kMaxFileDescriptors; } @@ -38,8 +43,13 @@ void HostIPCAllocator::SendPendingAsyncMessages() { // one file descriptor (e.g. OpDeliverFence). // A number of file descriptors per gecko ipc message have a limitation // on OS_POSIX (MACOSX or LINUX). +#if defined(OS_POSIX) static const uint32_t kMaxMessageNumber = - IPC::Message::MAX_DESCRIPTORS_PER_MESSAGE; + FileDescriptorSet::MAX_DESCRIPTORS_PER_MESSAGE; +#else + // default number that works everywhere else + static const uint32_t kMaxMessageNumber = 250; +#endif nsTArray messages; messages.SetCapacity(mPendingAsyncMessage.size()); diff --git a/gfx/layers/ipc/ImageBridgeChild.cpp b/gfx/layers/ipc/ImageBridgeChild.cpp index f3d820512486..ca6c26ef4e80 100644 --- a/gfx/layers/ipc/ImageBridgeChild.cpp +++ b/gfx/layers/ipc/ImageBridgeChild.cpp @@ -786,7 +786,7 @@ bool ImageBridgeChild::DeallocShmem(ipc::Shmem& aShmem) { } PTextureChild* ImageBridgeChild::AllocPTextureChild( - const SurfaceDescriptor&, ReadLockDescriptor&, const LayersBackend&, + const SurfaceDescriptor&, const ReadLockDescriptor&, const LayersBackend&, const TextureFlags&, const uint64_t& aSerial, const wr::MaybeExternalImageId& aExternalImageId) { MOZ_ASSERT(CanSend()); @@ -877,13 +877,12 @@ mozilla::ipc::IPCResult ImageBridgeChild::RecvReportFramesDropped( } PTextureChild* ImageBridgeChild::CreateTexture( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor&& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, LayersBackend aLayersBackend, TextureFlags aFlags, uint64_t aSerial, wr::MaybeExternalImageId& aExternalImageId, nsISerialEventTarget* aTarget) { MOZ_ASSERT(CanSend()); - return SendPTextureConstructor(aSharedData, std::move(aReadLock), - aLayersBackend, aFlags, aSerial, - aExternalImageId); + return SendPTextureConstructor(aSharedData, aReadLock, aLayersBackend, aFlags, + aSerial, aExternalImageId); } static bool IBCAddOpDestroy(CompositableTransaction* aTxn, diff --git a/gfx/layers/ipc/ImageBridgeChild.h b/gfx/layers/ipc/ImageBridgeChild.h index 05b70641ec54..dbcff0586016 100644 --- a/gfx/layers/ipc/ImageBridgeChild.h +++ b/gfx/layers/ipc/ImageBridgeChild.h @@ -164,7 +164,7 @@ class ImageBridgeChild final : public PImageBridgeChild, base::ProcessId GetParentPid() const override { return OtherPid(); } PTextureChild* AllocPTextureChild( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, const LayersBackend& aLayersBackend, const TextureFlags& aFlags, const uint64_t& aSerial, const wr::MaybeExternalImageId& aExternalImageId); @@ -299,7 +299,7 @@ class ImageBridgeChild final : public PImageBridgeChild, bool DeallocShmem(mozilla::ipc::Shmem& aShmem) override; PTextureChild* CreateTexture( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor&& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, LayersBackend aLayersBackend, TextureFlags aFlags, uint64_t aSerial, wr::MaybeExternalImageId& aExternalImageId, nsISerialEventTarget* aTarget = nullptr) override; diff --git a/gfx/layers/ipc/ImageBridgeParent.cpp b/gfx/layers/ipc/ImageBridgeParent.cpp index a15123d4ed37..9e74c35e84e1 100644 --- a/gfx/layers/ipc/ImageBridgeParent.cpp +++ b/gfx/layers/ipc/ImageBridgeParent.cpp @@ -289,10 +289,10 @@ mozilla::ipc::IPCResult ImageBridgeParent::RecvReleaseCompositable( } PTextureParent* ImageBridgeParent::AllocPTextureParent( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, const LayersBackend& aLayersBackend, const TextureFlags& aFlags, const uint64_t& aSerial, const wr::MaybeExternalImageId& aExternalImageId) { - return TextureHost::CreateIPDLActor(this, aSharedData, std::move(aReadLock), + return TextureHost::CreateIPDLActor(this, aSharedData, aReadLock, aLayersBackend, aFlags, aSerial, aExternalImageId); } diff --git a/gfx/layers/ipc/ImageBridgeParent.h b/gfx/layers/ipc/ImageBridgeParent.h index 25f38e780d53..d3e39db37abc 100644 --- a/gfx/layers/ipc/ImageBridgeParent.h +++ b/gfx/layers/ipc/ImageBridgeParent.h @@ -80,7 +80,7 @@ class ImageBridgeParent final : public PImageBridgeParent, const uint64_t& aFwdTransactionId); PTextureParent* AllocPTextureParent( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, const LayersBackend& aLayersBackend, const TextureFlags& aFlags, const uint64_t& aSerial, const wr::MaybeExternalImageId& aExternalImageId); diff --git a/gfx/layers/ipc/LayersMessages.ipdlh b/gfx/layers/ipc/LayersMessages.ipdlh index e0e095b92d68..20bd9884c422 100644 --- a/gfx/layers/ipc/LayersMessages.ipdlh +++ b/gfx/layers/ipc/LayersMessages.ipdlh @@ -54,7 +54,7 @@ using mozilla::layers::LayersBackend from "mozilla/layers/LayersTypes.h"; using mozilla::layers::LayerHandle from "mozilla/layers/LayersTypes.h"; using mozilla::layers::CompositableHandle from "mozilla/layers/LayersTypes.h"; using mozilla::layers::CompositionPayload from "mozilla/layers/LayersTypes.h"; -[MoveOnly] using mozilla::CrossProcessSemaphoreHandle from "mozilla/ipc/CrossProcessSemaphore.h"; +using mozilla::CrossProcessSemaphoreHandle from "mozilla/ipc/CrossProcessSemaphore.h"; using struct mozilla::void_t from "mozilla/ipc/IPCCore.h"; using mozilla::layers::LayersId from "mozilla/layers/LayersTypes.h"; using mozilla::layers::TransactionId from "mozilla/layers/LayersTypes.h"; diff --git a/gfx/layers/ipc/LayersSurfaces.ipdlh b/gfx/layers/ipc/LayersSurfaces.ipdlh index 29ab043f7ada..d0a6c6799a72 100644 --- a/gfx/layers/ipc/LayersSurfaces.ipdlh +++ b/gfx/layers/ipc/LayersSurfaces.ipdlh @@ -15,7 +15,7 @@ using mozilla::gfx::ColorRange from "mozilla/gfx/Types.h"; using mozilla::gfx::SurfaceFormat from "mozilla/gfx/Types.h"; using mozilla::gfx::IntRect from "mozilla/gfx/Rect.h"; using mozilla::gfx::IntSize from "mozilla/gfx/Point.h"; -[MoveOnly] using mozilla::ipc::SharedMemoryBasic::Handle from "mozilla/ipc/SharedMemoryBasic.h"; +using mozilla::ipc::SharedMemoryBasic::Handle from "mozilla/ipc/SharedMemoryBasic.h"; using gfxImageFormat from "gfxTypes.h"; using mozilla::layers::MaybeVideoBridgeSource from "mozilla/layers/VideoBridgeUtils.h"; diff --git a/gfx/layers/ipc/PCanvas.ipdl b/gfx/layers/ipc/PCanvas.ipdl index 93c522190b34..c09357c77865 100644 --- a/gfx/layers/ipc/PCanvas.ipdl +++ b/gfx/layers/ipc/PCanvas.ipdl @@ -6,9 +6,9 @@ include "mozilla/layers/LayersMessageUtils.h"; -[MoveOnly] using mozilla::CrossProcessSemaphoreHandle from "mozilla/ipc/CrossProcessSemaphore.h"; +using mozilla::CrossProcessSemaphoreHandle from "mozilla/ipc/CrossProcessSemaphore.h"; using mozilla::layers::TextureType from "mozilla/layers/LayersTypes.h"; -[MoveOnly] using mozilla::ipc::SharedMemoryBasic::Handle from "mozilla/ipc/SharedMemoryBasic.h"; +using mozilla::ipc::SharedMemoryBasic::Handle from "mozilla/ipc/SharedMemoryBasic.h"; namespace mozilla { namespace layers { diff --git a/gfx/layers/ipc/PCompositorManager.ipdl b/gfx/layers/ipc/PCompositorManager.ipdl index 360c114dfb3c..551d6e2d9180 100644 --- a/gfx/layers/ipc/PCompositorManager.ipdl +++ b/gfx/layers/ipc/PCompositorManager.ipdl @@ -15,7 +15,7 @@ using struct mozilla::void_t from "mozilla/ipc/IPCCore.h"; using mozilla::TimeDuration from "mozilla/TimeStamp.h"; using mozilla::CSSToLayoutDeviceScale from "Units.h"; using mozilla::gfx::IntSize from "mozilla/gfx/2D.h"; -[MoveOnly] using mozilla::ipc::SharedMemoryBasic::Handle from "mozilla/ipc/SharedMemoryBasic.h"; +using mozilla::ipc::SharedMemoryBasic::Handle from "mozilla/ipc/SharedMemoryBasic.h"; using mozilla::layers::CompositorOptions from "mozilla/layers/LayersMessageUtils.h"; using mozilla::wr::ExternalImageId from "mozilla/webrender/WebRenderTypes.h"; using mozilla::wr::MemoryReport from "mozilla/webrender/WebRenderTypes.h"; diff --git a/gfx/layers/ipc/SharedSurfacesChild.cpp b/gfx/layers/ipc/SharedSurfacesChild.cpp index 938804b8417e..e302fc7c5134 100644 --- a/gfx/layers/ipc/SharedSurfacesChild.cpp +++ b/gfx/layers/ipc/SharedSurfacesChild.cpp @@ -244,9 +244,8 @@ nsresult SharedSurfacesChild::ShareInternal(SourceSurfaceSharedData* aSurface, data->MarkShared(); manager->SendAddSharedSurface( - data->Id(), - SurfaceDescriptorShared(aSurface->GetSize(), aSurface->Stride(), format, - std::move(handle))); + data->Id(), SurfaceDescriptorShared(aSurface->GetSize(), + aSurface->Stride(), format, handle)); *aUserData = data; return NS_OK; } diff --git a/gfx/layers/ipc/SharedSurfacesParent.cpp b/gfx/layers/ipc/SharedSurfacesParent.cpp index eab2b1c9126e..f05b7d587009 100644 --- a/gfx/layers/ipc/SharedSurfacesParent.cpp +++ b/gfx/layers/ipc/SharedSurfacesParent.cpp @@ -197,7 +197,7 @@ void SharedSurfacesParent::DestroyProcess(base::ProcessId aPid) { /* static */ void SharedSurfacesParent::Add(const wr::ExternalImageId& aId, - SurfaceDescriptorShared&& aDesc, + const SurfaceDescriptorShared& aDesc, base::ProcessId aPid) { MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread()); MOZ_ASSERT(aPid != base::GetCurrentProcId()); @@ -212,8 +212,8 @@ void SharedSurfacesParent::Add(const wr::ExternalImageId& aId, // second, to avoid deadlock. // // Note that the surface wrapper maps in the given handle as read only. - surface->Init(aDesc.size(), aDesc.stride(), aDesc.format(), - std::move(aDesc.handle()), aPid); + surface->Init(aDesc.size(), aDesc.stride(), aDesc.format(), aDesc.handle(), + aPid); StaticMutexAutoLock lock(sMutex); if (!sInstance) { diff --git a/gfx/layers/ipc/SharedSurfacesParent.h b/gfx/layers/ipc/SharedSurfacesParent.h index a42588d717a2..80ac76ba6650 100644 --- a/gfx/layers/ipc/SharedSurfacesParent.h +++ b/gfx/layers/ipc/SharedSurfacesParent.h @@ -48,7 +48,7 @@ class SharedSurfacesParent final { static bool Release(const wr::ExternalImageId& aId, bool aForCreator = false); static void Add(const wr::ExternalImageId& aId, - SurfaceDescriptorShared&& aDesc, base::ProcessId aPid); + const SurfaceDescriptorShared& aDesc, base::ProcessId aPid); static void Remove(const wr::ExternalImageId& aId); diff --git a/gfx/layers/ipc/TextureForwarder.h b/gfx/layers/ipc/TextureForwarder.h index 2ce2e5df523e..bbd9b7215398 100644 --- a/gfx/layers/ipc/TextureForwarder.h +++ b/gfx/layers/ipc/TextureForwarder.h @@ -74,7 +74,7 @@ class TextureForwarder : public LayersIPCChannel { * parent side. */ virtual PTextureChild* CreateTexture( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor&& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, LayersBackend aLayersBackend, TextureFlags aFlags, uint64_t aSerial, wr::MaybeExternalImageId& aExternalImageId, nsISerialEventTarget* aTarget = nullptr) = 0; diff --git a/gfx/layers/ipc/VideoBridgeChild.cpp b/gfx/layers/ipc/VideoBridgeChild.cpp index 248f1683f6ef..d41744291972 100644 --- a/gfx/layers/ipc/VideoBridgeChild.cpp +++ b/gfx/layers/ipc/VideoBridgeChild.cpp @@ -145,7 +145,7 @@ bool VideoBridgeChild::DeallocShmem(ipc::Shmem& aShmem) { } PTextureChild* VideoBridgeChild::AllocPTextureChild(const SurfaceDescriptor&, - ReadLockDescriptor&, + const ReadLockDescriptor&, const LayersBackend&, const TextureFlags&, const uint64_t& aSerial) { @@ -164,12 +164,12 @@ void VideoBridgeChild::ActorDestroy(ActorDestroyReason aWhy) { void VideoBridgeChild::ActorDealloc() { mIPDLSelfRef = nullptr; } PTextureChild* VideoBridgeChild::CreateTexture( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor&& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, LayersBackend aLayersBackend, TextureFlags aFlags, uint64_t aSerial, wr::MaybeExternalImageId& aExternalImageId, nsISerialEventTarget* aTarget) { MOZ_ASSERT(CanSend()); - return SendPTextureConstructor(aSharedData, std::move(aReadLock), - aLayersBackend, aFlags, aSerial); + return SendPTextureConstructor(aSharedData, aReadLock, aLayersBackend, aFlags, + aSerial); } bool VideoBridgeChild::IsSameProcess() const { diff --git a/gfx/layers/ipc/VideoBridgeChild.h b/gfx/layers/ipc/VideoBridgeChild.h index 24dd84de7f0c..9370bab04ebe 100644 --- a/gfx/layers/ipc/VideoBridgeChild.h +++ b/gfx/layers/ipc/VideoBridgeChild.h @@ -29,7 +29,7 @@ class VideoBridgeChild final : public PVideoBridgeChild, // PVideoBridgeChild PTextureChild* AllocPTextureChild(const SurfaceDescriptor& aSharedData, - ReadLockDescriptor& aReadLock, + const ReadLockDescriptor& aReadLock, const LayersBackend& aLayersBackend, const TextureFlags& aFlags, const uint64_t& aSerial); @@ -49,7 +49,7 @@ class VideoBridgeChild final : public PVideoBridgeChild, // TextureForwarder PTextureChild* CreateTexture( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor&& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, LayersBackend aLayersBackend, TextureFlags aFlags, uint64_t aSerial, wr::MaybeExternalImageId& aExternalImageId, nsISerialEventTarget* aTarget = nullptr) override; diff --git a/gfx/layers/ipc/VideoBridgeParent.cpp b/gfx/layers/ipc/VideoBridgeParent.cpp index 954f9bca1801..b2a83e6468c7 100644 --- a/gfx/layers/ipc/VideoBridgeParent.cpp +++ b/gfx/layers/ipc/VideoBridgeParent.cpp @@ -109,12 +109,11 @@ void VideoBridgeParent::ActorDealloc() { } PTextureParent* VideoBridgeParent::AllocPTextureParent( - const SurfaceDescriptor& aSharedData, ReadLockDescriptor& aReadLock, + const SurfaceDescriptor& aSharedData, const ReadLockDescriptor& aReadLock, const LayersBackend& aLayersBackend, const TextureFlags& aFlags, const uint64_t& aSerial) { - PTextureParent* parent = - TextureHost::CreateIPDLActor(this, aSharedData, std::move(aReadLock), - aLayersBackend, aFlags, aSerial, Nothing()); + PTextureParent* parent = TextureHost::CreateIPDLActor( + this, aSharedData, aReadLock, aLayersBackend, aFlags, aSerial, Nothing()); if (!parent) { return nullptr; diff --git a/gfx/layers/ipc/VideoBridgeParent.h b/gfx/layers/ipc/VideoBridgeParent.h index ddfbd0528799..88dbeb174d5f 100644 --- a/gfx/layers/ipc/VideoBridgeParent.h +++ b/gfx/layers/ipc/VideoBridgeParent.h @@ -34,7 +34,7 @@ class VideoBridgeParent final : public PVideoBridgeParent, // PVideoBridgeParent void ActorDestroy(ActorDestroyReason aWhy) override; PTextureParent* AllocPTextureParent(const SurfaceDescriptor& aSharedData, - ReadLockDescriptor& aReadLock, + const ReadLockDescriptor& aReadLock, const LayersBackend& aLayersBackend, const TextureFlags& aFlags, const uint64_t& aSerial); diff --git a/gfx/thebes/SharedFontList-impl.h b/gfx/thebes/SharedFontList-impl.h index 1e1e356fc3a4..764af59f9605 100644 --- a/gfx/thebes/SharedFontList-impl.h +++ b/gfx/thebes/SharedFontList-impl.h @@ -252,10 +252,8 @@ class FontList { if (aIndex >= mReadOnlyShmems.Length()) { // Block index out of range *aOut = base::SharedMemory::NULLHandle(); - return; } - *aOut = mReadOnlyShmems[aIndex]->CloneHandle(); - if (!*aOut) { + if (!mReadOnlyShmems[aIndex]->ShareToProcess(aPid, aOut)) { MOZ_CRASH("failed to share block"); } } diff --git a/gfx/thebes/SharedFontList.cpp b/gfx/thebes/SharedFontList.cpp index 1e867a3de2d6..309641248d85 100644 --- a/gfx/thebes/SharedFontList.cpp +++ b/gfx/thebes/SharedFontList.cpp @@ -649,13 +649,13 @@ FontList::FontList(uint32_t aGeneration) { // Initialize using the list of shmem blocks passed by the parent via // SetXPCOMProcessAttributes. auto& blocks = dom::ContentChild::GetSingleton()->SharedFontListBlocks(); - for (auto& handle : blocks) { + for (auto handle : blocks) { auto newShm = MakeUnique(); if (!newShm->IsHandleValid(handle)) { // Bail out and let UpdateShmBlocks try to do its thing below. break; } - if (!newShm->SetHandle(std::move(handle), true)) { + if (!newShm->SetHandle(handle, true)) { MOZ_CRASH("failed to set shm handle"); } if (!newShm->Map(SHM_BLOCK_SIZE) || !newShm->memory()) { @@ -746,7 +746,7 @@ void FontList::ShmBlockAdded(uint32_t aGeneration, uint32_t aIndex, if (!newShm->IsHandleValid(aHandle)) { return; } - if (!newShm->SetHandle(std::move(aHandle), true)) { + if (!newShm->SetHandle(aHandle, true)) { MOZ_CRASH("failed to set shm handle"); } @@ -795,7 +795,7 @@ FontList::ShmBlock* FontList::GetBlockFromParent(uint32_t aIndex) { if (!newShm->IsHandleValid(handle)) { return nullptr; } - if (!newShm->SetHandle(std::move(handle), true)) { + if (!newShm->SetHandle(handle, true)) { MOZ_CRASH("failed to set shm handle"); } if (!newShm->Map(SHM_BLOCK_SIZE) || !newShm->memory()) { @@ -828,8 +828,9 @@ void FontList::ShareBlocksToProcess(nsTArray* aBlocks, base::ProcessId aPid) { MOZ_RELEASE_ASSERT(mReadOnlyShmems.Length() == mBlocks.Length()); for (auto& shmem : mReadOnlyShmems) { - auto handle = shmem->CloneHandle(); - if (!handle) { + base::SharedMemoryHandle* handle = + aBlocks->AppendElement(base::SharedMemory::NULLHandle()); + if (!shmem->ShareToProcess(aPid, handle)) { // If something went wrong here, we just bail out; the child will need to // request the blocks as needed, at some performance cost. (Although in // practice this may mean resources are so constrained the child process @@ -837,7 +838,6 @@ void FontList::ShareBlocksToProcess(nsTArray* aBlocks, aBlocks->Clear(); return; } - aBlocks->AppendElement(std::move(handle)); } } @@ -847,7 +847,12 @@ base::SharedMemoryHandle FontList::ShareBlockToProcess(uint32_t aIndex, MOZ_RELEASE_ASSERT(mReadOnlyShmems.Length() == mBlocks.Length()); MOZ_RELEASE_ASSERT(aIndex < mReadOnlyShmems.Length()); - return mReadOnlyShmems[aIndex]->CloneHandle(); + base::SharedMemoryHandle handle = base::SharedMemory::NULLHandle(); + if (mReadOnlyShmems[aIndex]->ShareToProcess(aPid, &handle)) { + return handle; + } + + return base::SharedMemory::NULLHandle(); } Pointer FontList::Alloc(uint32_t aSize) { diff --git a/gfx/thebes/gfxPlatformFontList.cpp b/gfx/thebes/gfxPlatformFontList.cpp index ebc5576758c9..86cdedcb3102 100644 --- a/gfx/thebes/gfxPlatformFontList.cpp +++ b/gfx/thebes/gfxPlatformFontList.cpp @@ -2777,7 +2777,7 @@ base::SharedMemoryHandle gfxPlatformFontList::ShareShmBlockToProcess( void gfxPlatformFontList::ShmBlockAdded(uint32_t aGeneration, uint32_t aIndex, base::SharedMemoryHandle aHandle) { if (SharedFontList()) { - SharedFontList()->ShmBlockAdded(aGeneration, aIndex, std::move(aHandle)); + SharedFontList()->ShmBlockAdded(aGeneration, aIndex, aHandle); } } diff --git a/intl/hyphenation/glue/nsHyphenator.cpp b/intl/hyphenation/glue/nsHyphenator.cpp index 7e97ba9b001f..c3b377767e3f 100644 --- a/intl/hyphenation/glue/nsHyphenator.cpp +++ b/intl/hyphenation/glue/nsHyphenator.cpp @@ -78,7 +78,7 @@ static UniquePtr GetHyphDictFromParent(nsIURI* aURI, if (!shm->IsHandleValid(handle)) { return nullptr; } - if (!shm->SetHandle(std::move(handle), true)) { + if (!shm->SetHandle(handle, true)) { return nullptr; } if (!shm->Map(size)) { @@ -496,6 +496,9 @@ void nsHyphenator::ShareToProcess(base::ProcessId aPid, if (!mDict.is>()) { return; } - *aOutHandle = mDict.as>()->CloneHandle(); + if (!mDict.as>()->ShareToProcess(aPid, + aOutHandle)) { + return; + } *aOutSize = mDictSize; } diff --git a/ipc/chromium/moz.build b/ipc/chromium/moz.build index ecd57606fcd9..2a40277d990c 100644 --- a/ipc/chromium/moz.build +++ b/ipc/chromium/moz.build @@ -71,6 +71,7 @@ if os_posix: "src/base/string16.cc", "src/base/thread_local_posix.cc", "src/base/waitable_event_posix.cc", + "src/chrome/common/file_descriptor_set_posix.cc", "src/chrome/common/ipc_channel_posix.cc", "src/chrome/common/process_watcher_posix_sigchld.cc", ] diff --git a/ipc/chromium/src/base/file_descriptor_posix.h b/ipc/chromium/src/base/file_descriptor_posix.h new file mode 100644 index 000000000000..cb76086ef81a --- /dev/null +++ b/ipc/chromium/src/base/file_descriptor_posix.h @@ -0,0 +1,39 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +// Copyright (c) 2006-2009 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef BASE_FILE_DESCRIPTOR_POSIX_H_ +#define BASE_FILE_DESCRIPTOR_POSIX_H_ + +namespace base { + +// ----------------------------------------------------------------------------- +// We introduct a special structure for file descriptors in order that we are +// able to use template specialisation to special-case their handling. +// +// WARNING: (Chromium only) There are subtleties to consider if serialising +// these objects over IPC. See comments in chrome/common/ipc_message_utils.h +// above the template specialisation for this structure. +// ----------------------------------------------------------------------------- +struct FileDescriptor { + FileDescriptor() : fd(-1), auto_close(false) {} + + FileDescriptor(int ifd, bool iauto_close) + : fd(ifd), auto_close(iauto_close) {} + + bool operator==(const FileDescriptor& aOther) const { + return fd == aOther.fd && auto_close == aOther.auto_close; + } + + int fd; + // If true, this file descriptor should be closed after it has been used. For + // example an IPC system might interpret this flag as indicating that the + // file descriptor it has been given should be closed after use. + bool auto_close; +}; + +} // namespace base + +#endif // BASE_FILE_DESCRIPTOR_POSIX_H_ diff --git a/ipc/chromium/src/base/shared_memory.h b/ipc/chromium/src/base/shared_memory.h index 841dc62d3f81..c7a5573bdc08 100644 --- a/ipc/chromium/src/base/shared_memory.h +++ b/ipc/chromium/src/base/shared_memory.h @@ -12,6 +12,7 @@ #if defined(OS_POSIX) # include # include +# include "base/file_descriptor_posix.h" #endif #include @@ -24,7 +25,11 @@ namespace base { // SharedMemoryHandle is a platform specific type which represents // the underlying OS handle to a shared memory segment. -typedef mozilla::UniqueFileHandle SharedMemoryHandle; +#if defined(OS_WIN) +typedef HANDLE SharedMemoryHandle; +#elif defined(OS_POSIX) +typedef FileDescriptor SharedMemoryHandle; +#endif // Platform abstraction for shared memory. Provides a C++ wrapper // around the OS primitive for a memory mapped file. @@ -37,7 +42,7 @@ class SharedMemory { // shared memory file. SharedMemory(SharedMemoryHandle init_handle, bool read_only) : SharedMemory() { - SetHandle(std::move(init_handle), read_only); + SetHandle(init_handle, read_only); } // Move constructor; transfers ownership. @@ -93,20 +98,16 @@ class SharedMemory { // Mapped via Map(). Returns NULL if it is not mapped. void* memory() const { return memory_.get(); } - // Extracts the underlying file handle, returning a RAII type. - // This unmaps the memory as a side-effect (and cleans up any OS-specific - // resources). + // Extracts the underlying file handle; similar to + // GiveToProcess(GetCurrentProcId(), ...) but returns a RAII type. + // Like GiveToProcess, this unmaps the memory as a side-effect (and + // cleans up any OS-specific resources). mozilla::UniqueFileHandle TakeHandle() { mozilla::UniqueFileHandle handle = std::move(mapped_file_); Close(); return handle; } - // Creates a copy of the underlying file handle, returning a RAII type. - // This operation may fail, in which case the returned file handle will be - // invalid. - mozilla::UniqueFileHandle CloneHandle(); - // Make the shared memory object read-only, such that it cannot be // written even if it's sent to an untrusted process. If it was // mapped in this process, it will be unmapped. The object must @@ -149,6 +150,27 @@ class SharedMemory { // something there in the meantime. static void* FindFreeAddressSpace(size_t size); + // Share the shared memory to another process. Attempts + // to create a platform-specific new_handle which can be + // used in a remote process to access the shared memory + // file. new_handle is an ouput parameter to receive + // the handle for use in the remote process. + // Returns true on success, false otherwise. + bool ShareToProcess(base::ProcessId target_pid, + SharedMemoryHandle* new_handle) { + return ShareToProcessCommon(target_pid, new_handle, false); + } + + // Logically equivalent to: + // bool ok = ShareToProcess(process, new_handle); + // Close(); + // return ok; + // Note that the memory is unmapped by calling this method, regardless of the + // return value. + bool GiveToProcess(ProcessId target_pid, SharedMemoryHandle* new_handle) { + return ShareToProcessCommon(target_pid, new_handle, true); + } + #ifdef OS_POSIX // If named POSIX shm is being used, append the prefix (including // the leading '/') that would be used by a process with the given @@ -158,6 +180,9 @@ class SharedMemory { #endif private: + bool ShareToProcessCommon(ProcessId target_pid, + SharedMemoryHandle* new_handle, bool close_self); + bool CreateInternal(size_t size, bool freezeable); // Unmapping shared memory requires the mapped size on Unix but not diff --git a/ipc/chromium/src/base/shared_memory_posix.cc b/ipc/chromium/src/base/shared_memory_posix.cc index b2c5d6f4d229..4c6905f60f28 100644 --- a/ipc/chromium/src/base/shared_memory_posix.cc +++ b/ipc/chromium/src/base/shared_memory_posix.cc @@ -64,7 +64,7 @@ bool SharedMemory::SetHandle(SharedMemoryHandle handle, bool read_only) { #endif freezeable_ = false; - mapped_file_ = std::move(handle); + mapped_file_.reset(handle.fd); read_only_ = read_only; // is_memfd_ only matters for freezing, which isn't possible return true; @@ -72,11 +72,11 @@ bool SharedMemory::SetHandle(SharedMemoryHandle handle, bool read_only) { // static bool SharedMemory::IsHandleValid(const SharedMemoryHandle& handle) { - return handle != nullptr; + return handle.fd >= 0; } // static -SharedMemoryHandle SharedMemory::NULLHandle() { return nullptr; } +SharedMemoryHandle SharedMemory::NULLHandle() { return SharedMemoryHandle(); } #ifdef ANDROID @@ -527,16 +527,24 @@ void* SharedMemory::FindFreeAddressSpace(size_t size) { return memory != MAP_FAILED ? memory : NULL; } -SharedMemoryHandle SharedMemory::CloneHandle() { +bool SharedMemory::ShareToProcessCommon(ProcessId processId, + SharedMemoryHandle* new_handle, + bool close_self) { freezeable_ = false; const int new_fd = dup(mapped_file_.get()); if (new_fd < 0) { CHROMIUM_LOG(WARNING) << "failed to duplicate file descriptor: " << strerror(errno); - return nullptr; + return false; } - return mozilla::UniqueFileHandle(new_fd); + new_handle->fd = new_fd; + new_handle->auto_close = true; + + if (close_self) Close(); + + return true; } + void SharedMemory::Close(bool unmap_view) { if (unmap_view) { Unmap(); diff --git a/ipc/chromium/src/base/shared_memory_win.cc b/ipc/chromium/src/base/shared_memory_win.cc index df4db78daacd..1b156d457493 100644 --- a/ipc/chromium/src/base/shared_memory_win.cc +++ b/ipc/chromium/src/base/shared_memory_win.cc @@ -69,7 +69,7 @@ bool SharedMemory::SetHandle(SharedMemoryHandle handle, bool read_only) { external_section_ = true; freezeable_ = false; // just in case - mapped_file_ = std::move(handle); + mapped_file_.reset(handle); read_only_ = read_only; return true; } @@ -191,16 +191,39 @@ void* SharedMemory::FindFreeAddressSpace(size_t size) { return memory; } -SharedMemoryHandle SharedMemory::CloneHandle() { +bool SharedMemory::ShareToProcessCommon(ProcessId processId, + SharedMemoryHandle* new_handle, + bool close_self) { freezeable_ = false; - HANDLE handle = INVALID_HANDLE_VALUE; - if (DuplicateHandle(GetCurrentProcess(), mapped_file_.get(), - GetCurrentProcess(), &handle, 0, false, - DUPLICATE_SAME_ACCESS)) { - return SharedMemoryHandle(handle); + *new_handle = 0; + DWORD access = FILE_MAP_READ | SECTION_QUERY; + DWORD options = 0; + HANDLE mapped_file; + HANDLE result; + if (!read_only_) { + access |= FILE_MAP_WRITE; } - NS_WARNING("DuplicateHandle Failed!"); - return nullptr; + if (close_self) { + // DUPLICATE_CLOSE_SOURCE causes DuplicateHandle to close mapped_file. + mapped_file = mapped_file_.release(); + options = DUPLICATE_CLOSE_SOURCE; + Unmap(); + } else { + mapped_file = mapped_file_.get(); + } + + if (processId == GetCurrentProcId() && close_self) { + *new_handle = mapped_file; + return true; + } + + if (!mozilla::ipc::DuplicateHandle(mapped_file, processId, &result, access, + options)) { + return false; + } + + *new_handle = result; + return true; } void SharedMemory::Close(bool unmap_view) { diff --git a/ipc/chromium/src/chrome/common/child_process_host.cc b/ipc/chromium/src/chrome/common/child_process_host.cc index e999c3ef6590..8ac2c0ba826c 100644 --- a/ipc/chromium/src/chrome/common/child_process_host.cc +++ b/ipc/chromium/src/chrome/common/child_process_host.cc @@ -29,9 +29,6 @@ bool ChildProcessHost::CreateChannel() { channel_id_ = IPC::Channel::GenerateVerifiedChannelID(); channel_.reset( new IPC::Channel(channel_id_, IPC::Channel::MODE_SERVER, &listener_)); -#if defined(OS_WIN) - channel_->StartAcceptingHandles(IPC::Channel::MODE_SERVER); -#endif if (!channel_->Connect()) return false; opening_channel_ = true; diff --git a/ipc/chromium/src/chrome/common/child_thread.cc b/ipc/chromium/src/chrome/common/child_thread.cc index e8bc55ae7ef6..ba5330a4e5d3 100644 --- a/ipc/chromium/src/chrome/common/child_thread.cc +++ b/ipc/chromium/src/chrome/common/child_thread.cc @@ -31,9 +31,6 @@ ChildThread* ChildThread::current() { void ChildThread::Init() { auto channel = mozilla::MakeUnique( channel_name_, IPC::Channel::MODE_CLIENT, nullptr); -#if defined(OS_WIN) - channel->StartAcceptingHandles(IPC::Channel::MODE_CLIENT); -#endif initial_port_ = mozilla::ipc::NodeController::InitChildProcess(std::move(channel)); diff --git a/ipc/chromium/src/chrome/common/file_descriptor_set_posix.cc b/ipc/chromium/src/chrome/common/file_descriptor_set_posix.cc new file mode 100644 index 000000000000..6c13d2953b66 --- /dev/null +++ b/ipc/chromium/src/chrome/common/file_descriptor_set_posix.cc @@ -0,0 +1,126 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "chrome/common/file_descriptor_set_posix.h" + +#include "base/eintr_wrapper.h" +#include "base/logging.h" + +#include + +FileDescriptorSet::FileDescriptorSet() : consumed_descriptor_highwater_(0) {} + +FileDescriptorSet::~FileDescriptorSet() { + if (consumed_descriptor_highwater_ == descriptors_.size()) return; + + CHROMIUM_LOG(WARNING) + << "FileDescriptorSet destroyed with unconsumed descriptors"; + + // We close all the descriptors where the close flag is set. If this + // message should have been transmitted, then closing those with close + // flags set mirrors the expected behaviour. + // + // If this message was received with more descriptors than expected + // (which could a DOS against the browser by a rogue renderer) then all + // the descriptors have their close flag set and we free all the extra + // kernel resources. + for (unsigned i = consumed_descriptor_highwater_; i < descriptors_.size(); + ++i) { + if (descriptors_[i].auto_close) IGNORE_EINTR(close(descriptors_[i].fd)); + } +} + +void FileDescriptorSet::CopyFrom(const FileDescriptorSet& other) { + for (std::vector::const_iterator i = + other.descriptors_.begin(); + i != other.descriptors_.end(); ++i) { + int fd = IGNORE_EINTR(dup(i->fd)); + AddAndAutoClose(fd); + } +} + +bool FileDescriptorSet::Add(int fd) { + if (descriptors_.size() == MAX_DESCRIPTORS_PER_MESSAGE) return false; + + struct base::FileDescriptor sd; + sd.fd = fd; + sd.auto_close = false; + descriptors_.push_back(sd); + return true; +} + +bool FileDescriptorSet::AddAndAutoClose(int fd) { + if (descriptors_.size() == MAX_DESCRIPTORS_PER_MESSAGE) return false; + + struct base::FileDescriptor sd; + sd.fd = fd; + sd.auto_close = true; + descriptors_.push_back(sd); + DCHECK(descriptors_.size() <= MAX_DESCRIPTORS_PER_MESSAGE); + return true; +} + +int FileDescriptorSet::GetDescriptorAt(unsigned index) const { + if (index >= descriptors_.size()) return -1; + + // We should always walk the descriptors in order, so it's reasonable to + // enforce this. Consider the case where a compromised renderer sends us + // the following message: + // + // ExampleMsg: + // num_fds:2 msg:FD(index = 1) control:SCM_RIGHTS {n, m} + // + // Here the renderer sent us a message which should have a descriptor, but + // actually sent two in an attempt to fill our fd table and kill us. By + // setting the index of the descriptor in the message to 1 (it should be + // 0), we would record a highwater of 1 and then consider all the + // descriptors to have been used. + // + // So we can either track of the use of each descriptor in a bitset, or we + // can enforce that we walk the indexes strictly in order. + // + // There's one more wrinkle: When logging messages, we may reparse them. So + // we have an exception: When the consumed_descriptor_highwater_ is at the + // end of the array and index 0 is requested, we reset the highwater value. + if (index == 0 && consumed_descriptor_highwater_ == descriptors_.size()) + consumed_descriptor_highwater_ = 0; + + if (index != consumed_descriptor_highwater_) return -1; + + consumed_descriptor_highwater_ = index + 1; + return descriptors_[index].fd; +} + +void FileDescriptorSet::GetDescriptors(int* buffer) const { + for (std::vector::const_iterator i = + descriptors_.begin(); + i != descriptors_.end(); ++i) { + *(buffer++) = i->fd; + } +} + +void FileDescriptorSet::CommitAll() { + for (std::vector::iterator i = descriptors_.begin(); + i != descriptors_.end(); ++i) { + if (i->auto_close) IGNORE_EINTR(close(i->fd)); + } + descriptors_.clear(); + consumed_descriptor_highwater_ = 0; +} + +void FileDescriptorSet::SetDescriptors(const int* buffer, unsigned count) { + DCHECK_LE(count, MAX_DESCRIPTORS_PER_MESSAGE); + DCHECK_EQ(descriptors_.size(), 0u); + DCHECK_EQ(consumed_descriptor_highwater_, 0u); + + descriptors_.reserve(count); + for (unsigned i = 0; i < count; ++i) { + struct base::FileDescriptor sd; + sd.fd = buffer[i]; + sd.auto_close = true; + descriptors_.push_back(sd); + } +} diff --git a/ipc/chromium/src/chrome/common/file_descriptor_set_posix.h b/ipc/chromium/src/chrome/common/file_descriptor_set_posix.h new file mode 100644 index 000000000000..4192f5b56b36 --- /dev/null +++ b/ipc/chromium/src/chrome/common/file_descriptor_set_posix.h @@ -0,0 +1,103 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +// Copyright (c) 2006-2009 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef CHROME_COMMON_FILE_DESCRIPTOR_SET_POSIX_H_ +#define CHROME_COMMON_FILE_DESCRIPTOR_SET_POSIX_H_ + +#include + +#include "base/basictypes.h" +#include "base/file_descriptor_posix.h" +#include "nsISupportsImpl.h" + +// ----------------------------------------------------------------------------- +// A FileDescriptorSet is an ordered set of POSIX file descriptors. These are +// associated with IPC messages so that descriptors can be transmitted over a +// UNIX domain socket. +// ----------------------------------------------------------------------------- +class FileDescriptorSet { + public: + NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FileDescriptorSet) + FileDescriptorSet(); + + // Mac and Linux both limit the number of file descriptors per message to + // slightly more than 250. + enum { MAX_DESCRIPTORS_PER_MESSAGE = 200 }; + + void CopyFrom(const FileDescriptorSet& other); + + // --------------------------------------------------------------------------- + // Interfaces for building during message serialisation... + + // Add a descriptor to the end of the set. Returns false iff the set is full. + bool Add(int fd); + // Add a descriptor to the end of the set and automatically close it after + // transmission. Returns false iff the set is full. + bool AddAndAutoClose(int fd); + + // --------------------------------------------------------------------------- + + // --------------------------------------------------------------------------- + // Interfaces for accessing during message deserialisation... + + // Return the number of descriptors + unsigned size() const { return descriptors_.size(); } + // Return true if no unconsumed descriptors remain + bool empty() const { return descriptors_.empty(); } + // Fetch the nth descriptor from the beginning of the set. Code using this + // /must/ access the descriptors in order, except that it may wrap from the + // end to index 0 again. + // + // This interface is designed for the deserialising code as it doesn't + // support close flags. + // returns: file descriptor, or -1 on error + int GetDescriptorAt(unsigned n) const; + + // --------------------------------------------------------------------------- + + // --------------------------------------------------------------------------- + // Interfaces for transmission... + + // Fill an array with file descriptors without 'consuming' them. CommitAll + // must be called after these descriptors have been transmitted. + // buffer: (output) a buffer of, at least, size() integers. + void GetDescriptors(int* buffer) const; + // This must be called after transmitting the descriptors returned by + // GetDescriptors. It marks all the descriptors as consumed and closes those + // which are auto-close. + void CommitAll(); + + // --------------------------------------------------------------------------- + + // --------------------------------------------------------------------------- + // Interfaces for receiving... + + // Set the contents of the set from the given buffer. This set must be empty + // before calling. The auto-close flag is set on all the descriptors so that + // unconsumed descriptors are closed on destruction. + void SetDescriptors(const int* buffer, unsigned count); + + // --------------------------------------------------------------------------- + + private: + ~FileDescriptorSet(); + + // A vector of descriptors and close flags. If this message is sent, then + // these descriptors are sent as control data. After sending, any descriptors + // with a true flag are closed. If this message has been received, then these + // are the descriptors which were received and all close flags are true. + std::vector descriptors_; + + // This contains the index of the next descriptor which should be consumed. + // It's used in a couple of ways. Firstly, at destruction we can check that + // all the descriptors have been read (with GetNthDescriptor). Secondly, we + // can check that they are read in order. + mutable unsigned consumed_descriptor_highwater_; + + DISALLOW_COPY_AND_ASSIGN(FileDescriptorSet); +}; + +#endif // CHROME_COMMON_FILE_DESCRIPTOR_SET_POSIX_H_ diff --git a/ipc/chromium/src/chrome/common/ipc_channel.h b/ipc/chromium/src/chrome/common/ipc_channel.h index 5211c0c6526a..300503b3fd2b 100644 --- a/ipc/chromium/src/chrome/common/ipc_channel.h +++ b/ipc/chromium/src/chrome/common/ipc_channel.h @@ -159,11 +159,6 @@ class Channel { #elif defined(OS_WIN) // Return the server pipe handle. void* GetServerPipeHandle() const; - - // Tell this pipe to accept handles. Exactly one side of the IPC connection - // must be set as `MODE_SERVER`, and that side will be responsible for calling - // `DuplicateHandle` to transfer the handle between processes. - void StartAcceptingHandles(Mode mode); #endif // defined(OS_POSIX) // On Windows: Generates a channel ID that, if passed to the client diff --git a/ipc/chromium/src/chrome/common/ipc_channel_posix.cc b/ipc/chromium/src/chrome/common/ipc_channel_posix.cc index 6955a9452ee1..10cdd7127559 100644 --- a/ipc/chromium/src/chrome/common/ipc_channel_posix.cc +++ b/ipc/chromium/src/chrome/common/ipc_channel_posix.cc @@ -29,6 +29,7 @@ #include "base/process_util.h" #include "base/string_util.h" #include "chrome/common/chrome_switches.h" +#include "chrome/common/file_descriptor_set_posix.h" #include "chrome/common/ipc_channel_utils.h" #include "chrome/common/ipc_message_utils.h" #include "mozilla/ipc/Endpoint.h" @@ -482,17 +483,17 @@ bool Channel::ChannelImpl::ProcessIncomingMessages() { Message& m = incoming_message_.ref(); - if (m.header()->num_handles) { + if (m.header()->num_fds) { // the message has file descriptors const char* error = NULL; - if (m.header()->num_handles > num_fds - fds_i) { + if (m.header()->num_fds > num_fds - fds_i) { // the message has been completely received, but we didn't get // enough file descriptors. error = "Message needs unreceived descriptors"; } - if (m.header()->num_handles > - IPC::Message::MAX_DESCRIPTORS_PER_MESSAGE) { + if (m.header()->num_fds > + FileDescriptorSet::MAX_DESCRIPTORS_PER_MESSAGE) { // There are too many descriptors in this message error = "Message requires an excessive number of descriptors"; } @@ -500,7 +501,7 @@ bool Channel::ChannelImpl::ProcessIncomingMessages() { if (error) { CHROMIUM_LOG(WARNING) << error << " channel:" << this << " message-type:" << m.type() - << " header()->num_handles:" << m.header()->num_handles + << " header()->num_fds:" << m.header()->num_fds << " num_fds:" << num_fds << " fds_i:" << fds_i; // close the existing file descriptors so that we don't leak them for (unsigned i = fds_i; i < num_fds; ++i) @@ -520,12 +521,9 @@ bool Channel::ChannelImpl::ProcessIncomingMessages() { OutputQueuePush(std::move(fdAck)); #endif - nsTArray handles(m.header()->num_handles); - for (unsigned end_i = fds_i + m.header()->num_handles; fds_i < end_i; - ++fds_i) { - handles.AppendElement(mozilla::UniqueFileHandle(fds[fds_i])); - } - m.SetAttachedFileHandles(std::move(handles)); + m.file_descriptor_set()->SetDescriptors(&fds[fds_i], + m.header()->num_fds); + fds_i += m.header()->num_fds; } // Note: We set other_pid_ below when we receive a Hello message (which @@ -593,7 +591,7 @@ bool Channel::ChannelImpl::ProcessOutgoingMessages() { struct msghdr msgh = {0}; static const int tmp = - CMSG_SPACE(sizeof(int[IPC::Message::MAX_DESCRIPTORS_PER_MESSAGE])); + CMSG_SPACE(sizeof(int[FileDescriptorSet::MAX_DESCRIPTORS_PER_MESSAGE])); char buf[tmp]; if (partial_write_iter_.isNothing()) { @@ -612,12 +610,12 @@ bool Channel::ChannelImpl::ProcessOutgoingMessages() { AddIPCProfilerMarker(*msg, other_pid_, MessageDirection::eSending, MessagePhase::TransferStart); - if (!msg->attached_handles_.IsEmpty()) { + if (!msg->file_descriptor_set()->empty()) { // This is the first chunk of a message which has descriptors to send struct cmsghdr* cmsg; - const unsigned num_fds = msg->attached_handles_.Length(); + const unsigned num_fds = msg->file_descriptor_set()->size(); - if (num_fds > IPC::Message::MAX_DESCRIPTORS_PER_MESSAGE) { + if (num_fds > FileDescriptorSet::MAX_DESCRIPTORS_PER_MESSAGE) { MOZ_DIAGNOSTIC_ASSERT(false, "Too many file descriptors!"); CHROMIUM_LOG(FATAL) << "Too many file descriptors!"; // This should not be reached. @@ -630,13 +628,11 @@ bool Channel::ChannelImpl::ProcessOutgoingMessages() { cmsg->cmsg_level = SOL_SOCKET; cmsg->cmsg_type = SCM_RIGHTS; cmsg->cmsg_len = CMSG_LEN(sizeof(int) * num_fds); - for (unsigned i = 0; i < num_fds; ++i) { - reinterpret_cast(CMSG_DATA(cmsg))[i] = - msg->attached_handles_[i].get(); - } + msg->file_descriptor_set()->GetDescriptors( + reinterpret_cast(CMSG_DATA(cmsg))); msgh.msg_controllen = cmsg->cmsg_len; - msg->header()->num_handles = num_fds; + msg->header()->num_fds = num_fds; #if defined(OS_MACOSX) msg->set_fd_cookie(++last_pending_fd_id_); #endif @@ -682,11 +678,9 @@ bool Channel::ChannelImpl::ProcessOutgoingMessages() { HANDLE_EINTR(corrected_sendmsg(pipe_, &msgh, MSG_DONTWAIT)); #if !defined(OS_MACOSX) - // On OSX the attached_handles_ array gets cleared later, once we get the + // On OSX CommitAll gets called later, once we get the // RECEIVED_FDS_MESSAGE_TYPE message. - if (bytes_written > 0) { - msg->attached_handles_.Clear(); - } + if (bytes_written > 0) msg->file_descriptor_set()->CommitAll(); #endif if (bytes_written < 0) { @@ -751,10 +745,9 @@ bool Channel::ChannelImpl::ProcessOutgoingMessages() { partial_write_iter_.reset(); #if defined(OS_MACOSX) - if (!msg->attached_handles_.IsEmpty()) { - pending_fds_.push_back(PendingDescriptors{ - msg->fd_cookie(), std::move(msg->attached_handles_)}); - } + if (!msg->file_descriptor_set()->empty()) + pending_fds_.push_back( + PendingDescriptors(msg->fd_cookie(), msg->file_descriptor_set())); #endif // Message sent OK! @@ -841,6 +834,7 @@ void Channel::ChannelImpl::CloseDescriptors(uint32_t pending_fd_id) { for (std::list::iterator i = pending_fds_.begin(); i != pending_fds_.end(); i++) { if ((*i).id == pending_fd_id) { + (*i).fds->CommitAll(); pending_fds_.erase(i); return; } @@ -910,6 +904,10 @@ void Channel::ChannelImpl::Close() { input_overflow_fds_.clear(); #if defined(OS_MACOSX) + for (std::list::iterator i = pending_fds_.begin(); + i != pending_fds_.end(); i++) { + (*i).fds->CommitAll(); + } pending_fds_.clear(); #endif diff --git a/ipc/chromium/src/chrome/common/ipc_channel_posix.h b/ipc/chromium/src/chrome/common/ipc_channel_posix.h index c0014d71293d..750912a6ae7c 100644 --- a/ipc/chromium/src/chrome/common/ipc_channel_posix.h +++ b/ipc/chromium/src/chrome/common/ipc_channel_posix.h @@ -18,11 +18,11 @@ #include "base/message_loop.h" #include "base/task.h" +#include "chrome/common/file_descriptor_set_posix.h" #include "mozilla/Maybe.h" #include "mozilla/Queue.h" #include "mozilla/UniquePtr.h" -#include "mozilla/UniquePtrExtensions.h" namespace IPC { @@ -123,7 +123,7 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher { // it's big enough. static constexpr size_t kControlBufferHeaderSize = 32; static constexpr size_t kControlBufferSize = - IPC::Message::MAX_DESCRIPTORS_PER_MESSAGE * sizeof(int) + + FileDescriptorSet::MAX_DESCRIPTORS_PER_MESSAGE * sizeof(int) + kControlBufferHeaderSize; // Large incoming messages that span multiple pipe buffers get built-up in the @@ -151,7 +151,11 @@ class Channel::ChannelImpl : public MessageLoopForIO::Watcher { #if defined(OS_MACOSX) struct PendingDescriptors { uint32_t id; - nsTArray handles; + RefPtr fds; + + PendingDescriptors() : id(0) {} + PendingDescriptors(uint32_t id, FileDescriptorSet* fds) + : id(id), fds(fds) {} }; std::list pending_fds_; diff --git a/ipc/chromium/src/chrome/common/ipc_channel_win.cc b/ipc/chromium/src/chrome/common/ipc_channel_win.cc index ae7a254bc0ca..404c189640e2 100644 --- a/ipc/chromium/src/chrome/common/ipc_channel_win.cc +++ b/ipc/chromium/src/chrome/common/ipc_channel_win.cc @@ -61,7 +61,9 @@ Channel::ChannelImpl::ChannelImpl(const ChannelId& channel_id, Mode mode, Listener* listener) : ALLOW_THIS_IN_INITIALIZER_LIST(input_state_(this)), ALLOW_THIS_IN_INITIALIZER_LIST(output_state_(this)), - ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)) { + ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)), + shared_secret_(0), + waiting_for_shared_secret_(false) { Init(mode, listener); if (!CreatePipe(channel_id, mode)) { @@ -77,7 +79,9 @@ Channel::ChannelImpl::ChannelImpl(const ChannelId& channel_id, Listener* listener) : ALLOW_THIS_IN_INITIALIZER_LIST(input_state_(this)), ALLOW_THIS_IN_INITIALIZER_LIST(output_state_(this)), - ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)) { + ALLOW_THIS_IN_INITIALIZER_LIST(factory_(this)), + shared_secret_(0), + waiting_for_shared_secret_(false) { Init(mode, listener); if (mode == MODE_SERVER) { @@ -107,9 +111,6 @@ void Channel::ChannelImpl::Init(Mode mode, Listener* listener) { output_queue_length_ = 0; input_buf_offset_ = 0; input_buf_ = mozilla::MakeUnique(Channel::kReadBufferSize); - accept_handles_ = false; - privileged_ = false; - other_process_ = INVALID_HANDLE_VALUE; } void Channel::ChannelImpl::OutputQueuePush(mozilla::UniquePtr msg) { @@ -140,12 +141,6 @@ void Channel::ChannelImpl::Close() { pipe_ = INVALID_HANDLE_VALUE; } - // If we have a connection to the other process, close the handle. - if (other_process_ != INVALID_HANDLE_VALUE) { - CloseHandle(other_process_); - other_process_ = INVALID_HANDLE_VALUE; - } - while (input_state_.is_pending || output_state_.is_pending) { MessageLoopForIO::current()->WaitForIOCompletion(INFINITE, this); } @@ -458,24 +453,9 @@ bool Channel::ChannelImpl::ProcessIncomingMessages( return false; } waiting_for_shared_secret_ = false; - - // Now that we know the remote pid, open a privileged handle to the - // child process if needed to transfer handles to/from it. - if (privileged_ && other_process_ == INVALID_HANDLE_VALUE) { - other_process_ = OpenProcess(PROCESS_DUP_HANDLE, false, other_pid_); - if (!other_process_) { - other_process_ = INVALID_HANDLE_VALUE; - CHROMIUM_LOG(ERROR) << "Failed to acquire privileged handle to " - << other_pid_ << ", cannot accept handles"; - } - } - listener_->OnChannelConnected(other_pid_); } else { mozilla::LogIPCMessage::Run run(&m); - if (!AcceptHandles(m)) { - return false; - } listener_->OnMessageReceived(std::move(m)); } @@ -532,9 +512,6 @@ bool Channel::ChannelImpl::ProcessOutgoingMessages( if (partial_write_iter_.isNothing()) { AddIPCProfilerMarker(*m, other_pid_, MessageDirection::eSending, MessagePhase::TransferStart); - if (!TransferHandles(*m)) { - return false; - } Pickle::BufferList::IterImpl iter(m->Buffers()); partial_write_iter_.emplace(iter); } @@ -605,171 +582,6 @@ void Channel::ChannelImpl::OnIOCompleted(MessageLoopForIO::IOContext* context, } } -void Channel::ChannelImpl::StartAcceptingHandles(Mode mode) { - if (accept_handles_) { - MOZ_ASSERT(privileged_ == (mode == MODE_SERVER)); - return; - } - accept_handles_ = true; - privileged_ = mode == MODE_SERVER; - - if (privileged_ && other_pid_ != -1 && - other_process_ == INVALID_HANDLE_VALUE) { - other_process_ = OpenProcess(PROCESS_DUP_HANDLE, false, other_pid_); - if (!other_process_) { - other_process_ = INVALID_HANDLE_VALUE; - CHROMIUM_LOG(ERROR) << "Failed to acquire privileged handle to " - << other_pid_ << ", cannot accept handles"; - } - } -} - -static uint32_t HandleToUint32(HANDLE h) { - // Cast through uintptr_t and then unsigned int to make the truncation to - // 32 bits explicit. Handles are size of-pointer but are always 32-bit values. - // https://docs.microsoft.com/en-ca/windows/win32/winprog64/interprocess-communication - // says: 64-bit versions of Windows use 32-bit handles for interoperability. - return static_cast(reinterpret_cast(h)); -} - -static HANDLE Uint32ToHandle(uint32_t h) { - return reinterpret_cast( - static_cast(static_cast(h))); -} - -bool Channel::ChannelImpl::AcceptHandles(Message& msg) { - MOZ_ASSERT(msg.num_handles() == 0); - - uint32_t num_handles = msg.header()->num_handles; - if (num_handles == 0) { - return true; - } - - if (!accept_handles_) { - CHROMIUM_LOG(ERROR) << "invalid message: " << msg.name() - << ". channel is not configured to accept handles"; - return false; - } - - // Seek to the start of our handle payload. - mozilla::CheckedInt handles_payload_size(num_handles); - handles_payload_size *= sizeof(uint32_t); - if (!handles_payload_size.isValid() || - handles_payload_size.value() > msg.header()->payload_size) { - CHROMIUM_LOG(ERROR) << "invalid handle count " << num_handles - << " or payload size: " << msg.header()->payload_size; - return false; - } - uint32_t handles_offset = - msg.header()->payload_size - handles_payload_size.value(); - - PickleIterator handles_start{msg}; - if (!msg.IgnoreBytes(&handles_start, handles_offset)) { - CHROMIUM_LOG(ERROR) << "IgnoreBytes failed"; - return false; - } - - // Read in the handles themselves, transferring ownership as required. - nsTArray handles; - { - PickleIterator iter{handles_start}; - for (uint32_t i = 0; i < num_handles; ++i) { - uint32_t handleValue; - if (!msg.ReadUInt32(&iter, &handleValue)) { - CHROMIUM_LOG(ERROR) << "failed to read handle value"; - return false; - } - HANDLE handle = Uint32ToHandle(handleValue); - - // If we're the privileged process, the remote process will have leaked - // the sent handles in its local address space, and be relying on us to - // duplicate them, otherwise the remote privileged side will have - // transferred the handles to us already. - if (privileged_) { - if (other_process_ == INVALID_HANDLE_VALUE) { - CHROMIUM_LOG(ERROR) << "other_process_ is invalid in AcceptHandles"; - return false; - } - if (!::DuplicateHandle( - other_process_, handle, GetCurrentProcess(), &handle, 0, FALSE, - DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE)) { - CHROMIUM_LOG(ERROR) << "DuplicateHandle failed for handle " << handle - << " in AcceptHandles"; - return false; - } - } - - // The handle is directly owned by this process now, and can be added to - // our `handles` array. - handles.AppendElement(mozilla::UniqueFileHandle(handle)); - } - } - - // We're done with the handle footer, truncate the message at that point. - msg.Truncate(&handles_start); - msg.SetAttachedFileHandles(std::move(handles)); - msg.header()->num_handles = 0; - MOZ_ASSERT(msg.header()->payload_size == handles_offset); - MOZ_ASSERT(msg.num_handles() == num_handles); - return true; -} - -bool Channel::ChannelImpl::TransferHandles(Message& msg) { - MOZ_ASSERT(msg.header()->num_handles == 0); - - uint32_t num_handles = msg.num_handles(); - if (num_handles == 0) { - return true; - } - - if (!accept_handles_) { - CHROMIUM_LOG(ERROR) << "cannot send message: " << msg.name() - << ". channel is not configured to accept handles"; - return false; - } - -#ifdef DEBUG - uint32_t handles_offset = msg.header()->payload_size; -#endif - - // Write handles from `attached_handles_` into the message payload. - for (uint32_t i = 0; i < num_handles; ++i) { - // Release ownership of the handle. It'll be cloned when the parent process - // transfers it with DuplicateHandle either in this process or the remote - // process. - HANDLE handle = msg.attached_handles_[i].release(); - - // If we're the privileged process, transfer the HANDLE to our remote before - // sending the message. Otherwise, the remote privileged process will - // transfer the handle for us, so leak it. - if (privileged_) { - if (other_process_ == INVALID_HANDLE_VALUE) { - CHROMIUM_LOG(ERROR) << "other_process_ is invalid in TransferHandles"; - return false; - } - if (!::DuplicateHandle(GetCurrentProcess(), handle, other_process_, - &handle, 0, FALSE, - DUPLICATE_SAME_ACCESS | DUPLICATE_CLOSE_SOURCE)) { - CHROMIUM_LOG(ERROR) << "DuplicateHandle failed for handle " << handle - << " in TransferHandles"; - return false; - } - } - - if (!msg.WriteUInt32(HandleToUint32(handle))) { - CHROMIUM_LOG(ERROR) << "failed to write handle value " << handle; - return false; - } - } - msg.attached_handles_.Clear(); - - msg.header()->num_handles = num_handles; - MOZ_ASSERT(msg.header()->payload_size == - handles_offset + (sizeof(uint32_t) * num_handles), - "Unexpected number of bytes written for handles footer?"); - return true; -} - bool Channel::ChannelImpl::Unsound_IsClosed() const { return closed_; } uint32_t Channel::ChannelImpl::Unsound_NumQueuedMessages() const { @@ -802,10 +614,6 @@ void* Channel::GetServerPipeHandle() const { return channel_impl_->GetServerPipeHandle(); } -void Channel::StartAcceptingHandles(Mode mode) { - channel_impl_->StartAcceptingHandles(mode); -} - Channel::Listener* Channel::set_listener(Listener* listener) { return channel_impl_->set_listener(listener); } diff --git a/ipc/chromium/src/chrome/common/ipc_channel_win.h b/ipc/chromium/src/chrome/common/ipc_channel_win.h index 0eebdcc3a0e4..c559aa0f747f 100644 --- a/ipc/chromium/src/chrome/common/ipc_channel_win.h +++ b/ipc/chromium/src/chrome/common/ipc_channel_win.h @@ -32,15 +32,13 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler { ChannelImpl(const ChannelId& channel_id, HANDLE server_pipe, Mode mode, Listener* listener); ~ChannelImpl() { - if (pipe_ != INVALID_HANDLE_VALUE || - other_process_ != INVALID_HANDLE_VALUE) { + if (pipe_ != INVALID_HANDLE_VALUE) { Close(); } } bool Connect(); void Close(); HANDLE GetServerPipeHandle() const; - void StartAcceptingHandles(Mode mode); Listener* set_listener(Listener* listener) { Listener* old = listener_; listener_ = listener; @@ -71,11 +69,6 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler { bool ProcessOutgoingMessages(MessageLoopForIO::IOContext* context, DWORD bytes_written); - // Called on a Message immediately before it is sent/recieved to transfer - // handles to the remote process, or accept handles from the remote process. - bool AcceptHandles(Message& msg); - bool TransferHandles(Message& msg); - // MessageLoop::IOHandler implementation. virtual void OnIOCompleted(MessageLoopForIO::IOContext* context, DWORD bytes_transfered, DWORD error); @@ -85,15 +78,15 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler { explicit State(ChannelImpl* channel); ~State(); MessageLoopForIO::IOContext context; - bool is_pending = false; + bool is_pending; }; State input_state_; State output_state_; - HANDLE pipe_ = INVALID_HANDLE_VALUE; + HANDLE pipe_; - Listener* listener_ = nullptr; + Listener* listener_; // Messages to be sent are queued here. mozilla::Queue, 64> output_queue_; @@ -105,7 +98,7 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler { // We read from the pipe into this buffer mozilla::UniquePtr input_buf_; - size_t input_buf_offset_ = 0; + size_t input_buf_offset_; // Large incoming messages that span multiple pipe buffers get built-up in the // buffers of this message. @@ -114,15 +107,15 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler { // In server-mode, we have to wait for the client to connect before we // can begin reading. We make use of the input_state_ when performing // the connect operation in overlapped mode. - bool waiting_connect_ = false; + bool waiting_connect_; // This flag is set when processing incoming messages. It is used to // avoid recursing through ProcessIncomingMessages, which could cause // problems. TODO(darin): make this unnecessary - bool processing_incoming_ = false; + bool processing_incoming_; // This flag is set after Close() is run on the channel. - std::atomic closed_ = false; + std::atomic closed_; // We keep track of the PID of the other side of this channel so that we can // record this when generating logs of IPC messages. @@ -132,7 +125,7 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler { // read output_queue_length_ from any thread (if we're OK getting an // occasional out-of-date or bogus value). We use output_queue_length_ to // implement Unsound_NumQueuedMessages. - std::atomic output_queue_length_ = 0; + std::atomic output_queue_length_; ScopedRunnableMethodFactory factory_; @@ -140,21 +133,11 @@ class Channel::ChannelImpl : public MessageLoopForIO::IOHandler { // a connection. If the value is non-zero, the client passes it in the hello // and the host validates. (We don't send the zero value to preserve IPC // compatibility with existing clients that don't validate the channel.) - int32_t shared_secret_ = 0; + int32_t shared_secret_; // In server-mode, we wait for the channel at the other side of the pipe to // send us back our shared secret, if we are using one. - bool waiting_for_shared_secret_ = false; - - // Whether or not to accept handles from a remote process, and whether this - // process is the privileged side of a IPC::Channel which can transfer - // handles. - bool accept_handles_ = false; - bool privileged_ = false; - - // A privileged process handle used to transfer HANDLEs to and from the remote - // process. This will only be used if `privileged_` is set. - HANDLE other_process_ = INVALID_HANDLE_VALUE; + bool waiting_for_shared_secret_; #ifdef DEBUG mozilla::UniquePtr _mOwningThread; diff --git a/ipc/chromium/src/chrome/common/ipc_message.cc b/ipc/chromium/src/chrome/common/ipc_message.cc index d3ed3c686813..2cc55852edb4 100644 --- a/ipc/chromium/src/chrome/common/ipc_message.cc +++ b/ipc/chromium/src/chrome/common/ipc_message.cc @@ -10,6 +10,10 @@ #include "build/build_config.h" #include "mojo/core/ports/event.h" +#if defined(OS_POSIX) +# include "chrome/common/file_descriptor_set_posix.h" +#endif + #include #include "nsISupportsImpl.h" @@ -26,7 +30,9 @@ Message::Message() : UserMessage(&kUserMessageTypeInfo), Pickle(sizeof(Header)) { MOZ_COUNT_CTOR(IPC::Message); header()->routing = header()->type = 0; - header()->num_handles = 0; +#if defined(OS_POSIX) + header()->num_fds = 0; +#endif } Message::Message(int32_t routing_id, msgid_t type, uint32_t segment_capacity, @@ -37,7 +43,9 @@ Message::Message(int32_t routing_id, msgid_t type, uint32_t segment_capacity, header()->routing = routing_id; header()->type = type; header()->flags = flags; - header()->num_handles = 0; +#if defined(OS_POSIX) + header()->num_fds = 0; +#endif header()->interrupt_remote_stack_depth_guess = static_cast(-1); header()->interrupt_local_stack_depth = static_cast(-1); header()->seqno = 0; @@ -59,9 +67,11 @@ Message::Message(const char* data, int data_len) Message::Message(Message&& other) : UserMessage(&kUserMessageTypeInfo), Pickle(std::move(other)), - attached_handles_(std::move(other.attached_handles_)), attached_ports_(std::move(other.attached_ports_)) { MOZ_COUNT_CTOR(IPC::Message); +#if defined(OS_POSIX) + file_descriptor_set_ = std::move(other.file_descriptor_set_); +#endif } /*static*/ Message* Message::IPDLMessage(int32_t routing_id, msgid_t type, @@ -89,8 +99,10 @@ Message::Message(Message&& other) Message& Message::operator=(Message&& other) { *static_cast(this) = std::move(other); - attached_handles_ = std::move(other.attached_handles_); attached_ports_ = std::move(other.attached_ports_); +#if defined(OS_POSIX) + file_descriptor_set_.swap(other.file_descriptor_set_); +#endif return *this; } @@ -143,38 +155,44 @@ uint32_t Message::FooterSize() const { return 0; } -bool Message::WriteFileHandle(mozilla::UniqueFileHandle handle) { - uint32_t handle_index = attached_handles_.Length(); - WriteUInt32(handle_index); - if (handle_index == MAX_DESCRIPTORS_PER_MESSAGE) { - return false; +#if defined(OS_POSIX) +bool Message::WriteFileDescriptor(const base::FileDescriptor& descriptor) { + // We write the index of the descriptor so that we don't have to + // keep the current descriptor as extra decoding state when deserialising. + // Also, we rely on each file descriptor being accompanied by sizeof(int) + // bytes of data in the message. See the comment for input_cmsg_buf_. + WriteInt(file_descriptor_set()->size()); + if (descriptor.auto_close) { + return file_descriptor_set()->AddAndAutoClose(descriptor.fd); + } else { + return file_descriptor_set()->Add(descriptor.fd); } - attached_handles_.AppendElement(std::move(handle)); - return true; } -bool Message::ConsumeFileHandle(PickleIterator* iter, - mozilla::UniqueFileHandle* handle) const { - uint32_t handle_index; - if (!ReadUInt32(iter, &handle_index)) { - return false; - } - if (handle_index >= attached_handles_.Length()) { - return false; - } - // NOTE: This mutates the underlying array, replacing the handle with an - // invalid handle. - *handle = std::exchange(attached_handles_[handle_index], nullptr); - return true; +bool Message::ReadFileDescriptor(PickleIterator* iter, + base::FileDescriptor* descriptor) const { + int descriptor_index; + if (!ReadInt(iter, &descriptor_index)) return false; + + FileDescriptorSet* file_descriptor_set = file_descriptor_set_.get(); + if (!file_descriptor_set) return false; + + descriptor->fd = file_descriptor_set->GetDescriptorAt(descriptor_index); + descriptor->auto_close = false; + + return descriptor->fd >= 0; } -void Message::SetAttachedFileHandles( - nsTArray handles) { - MOZ_DIAGNOSTIC_ASSERT(attached_handles_.IsEmpty()); - attached_handles_ = std::move(handles); +void Message::EnsureFileDescriptorSet() { + if (file_descriptor_set_.get() == NULL) + file_descriptor_set_ = new FileDescriptorSet; } -uint32_t Message::num_handles() const { return attached_handles_.Length(); } +uint32_t Message::num_fds() const { + return file_descriptor_set() ? file_descriptor_set()->size() : 0; +} + +#endif void Message::WritePort(mozilla::ipc::ScopedPort port) { uint32_t port_index = attached_ports_.Length(); diff --git a/ipc/chromium/src/chrome/common/ipc_message.h b/ipc/chromium/src/chrome/common/ipc_message.h index 7362236618e9..1378f6a77ceb 100644 --- a/ipc/chromium/src/chrome/common/ipc_message.h +++ b/ipc/chromium/src/chrome/common/ipc_message.h @@ -15,7 +15,6 @@ #include "mojo/core/ports/port_ref.h" #include "mozilla/RefPtr.h" #include "mozilla/TimeStamp.h" -#include "mozilla/UniquePtrExtensions.h" #include "mozilla/ipc/ScopedPort.h" #include "nsTArray.h" @@ -23,12 +22,18 @@ # include "mozilla/ipc/Faulty.h" #endif +namespace base { +struct FileDescriptor; +} + namespace mozilla { namespace ipc { class MiniTransceiver; } } // namespace mozilla +class FileDescriptorSet; + namespace IPC { //------------------------------------------------------------------------------ @@ -89,10 +94,6 @@ class Message : public mojo::core::ports::UserMessage, public Pickle { REPLY = 1, }; - // Mac and Linux both limit the number of file descriptors per message to - // slightly more than 250. - enum { MAX_DESCRIPTORS_PER_MESSAGE = 200 }; - class HeaderFlags { friend class Message; @@ -106,7 +107,6 @@ class Message : public mojo::core::ports::UserMessage, public Pickle { COMPRESS_BIT = 0x0200, COMPRESSALL_BIT = 0x0400, CONSTRUCTOR_BIT = 0x0800, - RELAY_BIT = 0x1000, }; public: @@ -147,20 +147,12 @@ class Message : public mojo::core::ports::UserMessage, public Pickle { bool IsReply() const { return (mFlags & REPLY_BIT) != 0; } bool IsReplyError() const { return (mFlags & REPLY_ERROR_BIT) != 0; } - bool IsRelay() const { return (mFlags & RELAY_BIT) != 0; } private: void SetSync() { mFlags |= SYNC_BIT; } void SetInterrupt() { mFlags |= INTERRUPT_BIT; } void SetReply() { mFlags |= REPLY_BIT; } void SetReplyError() { mFlags |= REPLY_ERROR_BIT; } - void SetRelay(bool relay) { - if (relay) { - mFlags |= RELAY_BIT; - } else { - mFlags &= ~RELAY_BIT; - } - } uint32_t mFlags; }; @@ -254,10 +246,9 @@ class Message : public mojo::core::ports::UserMessage, public Pickle { const mozilla::TimeStamp& create_time() const { return create_time_; } - uint32_t num_handles() const; - - bool is_relay() const { return header()->flags.IsRelay(); } - void set_relay(bool new_relay) { header()->flags.SetRelay(new_relay); } +#if defined(OS_POSIX) + uint32_t num_fds() const; +#endif template static bool Dispatch(const Message* msg, T* obj, void (T::*func)()) { @@ -309,21 +300,21 @@ class Message : public mojo::core::ports::UserMessage, public Pickle { return Pickle::MessageSize(HeaderSize(), range_start, range_end); } - bool WriteFileHandle(mozilla::UniqueFileHandle handle); +#if defined(OS_POSIX) + // On POSIX, a message supports reading / writing FileDescriptor objects. + // This is used to pass a file descriptor to the peer of an IPC channel. - // WARNING: This method is marked as `const` so it can be called when - // deserializing the message, but will mutate it, consuming the handle. - bool ConsumeFileHandle(PickleIterator* iter, - mozilla::UniqueFileHandle* handle) const; + // Add a descriptor to the end of the set. Returns false iff the set is full. + bool WriteFileDescriptor(const base::FileDescriptor& descriptor); + // Get a file descriptor from the message. Returns false on error. + // iter: a Pickle iterator to the current location in the message. + bool ReadFileDescriptor(PickleIterator* iter, + base::FileDescriptor* descriptor) const; - // Called when receiving an IPC message to attach file handles which were - // received from IPC. Must only be called when there are no handles on this - // IPC::Message. - void SetAttachedFileHandles(nsTArray handles); - -#if defined(OS_MACOSX) +# if defined(OS_MACOSX) void set_fd_cookie(uint32_t cookie) { header()->cookie = cookie; } uint32_t fd_cookie() const { return header()->cookie; } +# endif #endif void WritePort(mozilla::ipc::ScopedPort port); @@ -353,12 +344,14 @@ class Message : public mojo::core::ports::UserMessage, public Pickle { #endif struct Header : Pickle::Header { - int32_t routing; // ID of the view that this message is destined for - msgid_t type; // specifies the user-defined message type - HeaderFlags flags; // specifies control flags for the message - uint32_t num_handles; // the number of handles included with this message -#if defined(OS_MACOSX) + int32_t routing; // ID of the view that this message is destined for + msgid_t type; // specifies the user-defined message type + HeaderFlags flags; // specifies control flags for the message +#if defined(OS_POSIX) + uint32_t num_fds; // the number of descriptors included with this message +# if defined(OS_MACOSX) uint32_t cookie; // cookie to ACK that the descriptors have been read. +# endif #endif union { // For Interrupt messages, a guess at what the *other* side's stack depth @@ -379,11 +372,21 @@ class Message : public mojo::core::ports::UserMessage, public Pickle { Header* header() { return headerT
(); } const Header* header() const { return headerT
(); } - // The set of file handles which are attached to this message. - // - // Mutable, as this array can be mutated during `ReadHandle` when - // deserializing a message. - mutable nsTArray attached_handles_; +#if defined(OS_POSIX) + // The set of file descriptors associated with this message. + RefPtr file_descriptor_set_; + + // Ensure that a FileDescriptorSet is allocated + void EnsureFileDescriptorSet(); + + FileDescriptorSet* file_descriptor_set() { + EnsureFileDescriptorSet(); + return file_descriptor_set_.get(); + } + const FileDescriptorSet* file_descriptor_set() const { + return file_descriptor_set_.get(); + } +#endif // The set of mojo ports which are attached to this message. // diff --git a/ipc/chromium/src/chrome/common/ipc_message_utils.h b/ipc/chromium/src/chrome/common/ipc_message_utils.h index 7dd841807449..daeb4c8c5acc 100644 --- a/ipc/chromium/src/chrome/common/ipc_message_utils.h +++ b/ipc/chromium/src/chrome/common/ipc_message_utils.h @@ -21,6 +21,9 @@ #include "build/build_config.h" #include "chrome/common/ipc_message.h" +#if defined(OS_POSIX) +# include "chrome/common/file_descriptor_set_posix.h" +#endif #if defined(OS_WIN) # include #endif @@ -383,42 +386,56 @@ struct ParamTraitsWindows { template struct ParamTraitsIPC : ParamTraitsWindows

{}; -// `UniqueFileHandle` may be serialized over IPC channels. On the receiving -// side, the UniqueFileHandle is a valid duplicate of the handle which was -// transmitted. +#if defined(OS_POSIX) +// FileDescriptors may be serialised over IPC channels on POSIX. On the +// receiving side, the FileDescriptor is a valid duplicate of the file +// descriptor which was transmitted: *it is not just a copy of the integer like +// HANDLEs on Windows*. The only exception is if the file descriptor is < 0. In +// this case, the receiving end will see a value of -1. *Zero is a valid file +// descriptor*. // -// When sending a UniqueFileHandle, the handle must be valid at the time of -// transmission. As transmission is asynchronous, this requires passing -// ownership of the handle to IPC. +// The received file descriptor will have the |auto_close| flag set to true. The +// code which handles the message is responsible for taking ownership of it. +// File descriptors are OS resources and must be closed when no longer needed. // -// A UniqueFileHandle may only be read once. After it has been read once, it -// will be consumed, and future reads will return an invalid handle. +// When sending a file descriptor, the file descriptor must be valid at the time +// of transmission. Since transmission is not synchronous, one should consider +// dup()ing any file descriptors to be transmitted and setting the |auto_close| +// flag, which causes the file descriptor to be closed after writing. template <> -struct ParamTraitsIPC { - typedef mozilla::UniqueFileHandle param_type; - static void Write(Message* m, param_type&& p) { - const bool valid = p != nullptr; +struct ParamTraitsIPC { + typedef base::FileDescriptor param_type; + static void Write(Message* m, const param_type& p) { + const bool valid = p.fd >= 0; WriteParam(m, valid); + if (valid) { - if (!m->WriteFileHandle(std::move(p))) { - NOTREACHED() << "Too many file handles for one message!"; + if (!m->WriteFileDescriptor(p)) { + NOTREACHED() << "Too many file descriptors for one message!"; } } } static bool Read(const Message* m, PickleIterator* iter, param_type* r) { bool valid; - if (!ReadParam(m, iter, &valid)) { - return false; - } + if (!ReadParam(m, iter, &valid)) return false; if (!valid) { - *r = nullptr; + r->fd = -1; + r->auto_close = false; return true; } - return m->ConsumeFileHandle(iter, r); + return m->ReadFileDescriptor(iter, r); + } + static void Log(const param_type& p, std::wstring* l) { + if (p.auto_close) { + l->append(StringPrintf(L"FD(%d auto-close)", p.fd)); + } else { + l->append(StringPrintf(L"FD(%d)", p.fd)); + } } }; +#endif // defined(OS_POSIX) // Mozilla-specific types. diff --git a/ipc/glue/AutoTransportDescriptor.cpp b/ipc/glue/AutoTransportDescriptor.cpp index 9ef99cbdcfd5..315d1589592d 100644 --- a/ipc/glue/AutoTransportDescriptor.cpp +++ b/ipc/glue/AutoTransportDescriptor.cpp @@ -33,9 +33,9 @@ AutoTransportDescriptor::~AutoTransportDescriptor() { } Result, nsresult> -AutoTransportDescriptor::Create() { +AutoTransportDescriptor::Create(int32_t aProcIdOne) { TransportDescriptor one, two; - MOZ_TRY(CreateTransport(&one, &two)); + MOZ_TRY(CreateTransport(aProcIdOne, &one, &two)); return std::pair{AutoTransportDescriptor(one), AutoTransportDescriptor(two)}; } diff --git a/ipc/glue/AutoTransportDescriptor.h b/ipc/glue/AutoTransportDescriptor.h index c7770037a0a6..3382d02b4f4a 100644 --- a/ipc/glue/AutoTransportDescriptor.h +++ b/ipc/glue/AutoTransportDescriptor.h @@ -26,7 +26,7 @@ class AutoTransportDescriptor final { static Result, nsresult> - Create(); + Create(int32_t aProcIdOne); AutoTransportDescriptor Duplicate() const; diff --git a/ipc/glue/CrossProcessMutex.h b/ipc/glue/CrossProcessMutex.h index 72b0f1b1b964..6b1e73099e42 100644 --- a/ipc/glue/CrossProcessMutex.h +++ b/ipc/glue/CrossProcessMutex.h @@ -10,9 +10,6 @@ #include "base/process.h" #include "mozilla/Mutex.h" -#if defined(OS_WIN) -# include "mozilla/UniquePtrExtensions.h" -#endif #if !defined(OS_WIN) && !defined(OS_NETBSD) && !defined(OS_OPENBSD) # include # include "mozilla/ipc/SharedMemoryBasic.h" @@ -37,7 +34,7 @@ struct ParamTraits; // namespace mozilla { #if defined(OS_WIN) -typedef mozilla::UniqueFileHandle CrossProcessMutexHandle; +typedef HANDLE CrossProcessMutexHandle; #elif !defined(OS_NETBSD) && !defined(OS_OPENBSD) typedef mozilla::ipc::SharedMemoryBasic::Handle CrossProcessMutexHandle; #else diff --git a/ipc/glue/CrossProcessMutex_posix.cpp b/ipc/glue/CrossProcessMutex_posix.cpp index eb8eea359cd9..f7f5c69536b9 100644 --- a/ipc/glue/CrossProcessMutex_posix.cpp +++ b/ipc/glue/CrossProcessMutex_posix.cpp @@ -75,8 +75,7 @@ CrossProcessMutex::CrossProcessMutex(CrossProcessMutexHandle aHandle) MOZ_CRASH(); } - if (!mSharedBuffer->SetHandle(std::move(aHandle), - ipc::SharedMemory::RightsReadWrite)) { + if (!mSharedBuffer->SetHandle(aHandle, ipc::SharedMemory::RightsReadWrite)) { MOZ_CRASH(); } diff --git a/ipc/glue/CrossProcessMutex_windows.cpp b/ipc/glue/CrossProcessMutex_windows.cpp index 59b5c0a35dc3..b5ed066fde7d 100644 --- a/ipc/glue/CrossProcessMutex_windows.cpp +++ b/ipc/glue/CrossProcessMutex_windows.cpp @@ -30,10 +30,10 @@ CrossProcessMutex::CrossProcessMutex(const char*) { CrossProcessMutex::CrossProcessMutex(CrossProcessMutexHandle aHandle) { DWORD flags; - if (!::GetHandleInformation(aHandle.get(), &flags)) { + if (!::GetHandleInformation(aHandle, &flags)) { MOZ_CRASH("Attempt to construct a mutex from an invalid handle!"); } - mMutex = aHandle.release(); + mMutex = aHandle; MOZ_COUNT_CTOR(CrossProcessMutex); } @@ -56,11 +56,14 @@ void CrossProcessMutex::Unlock() { CrossProcessMutexHandle CrossProcessMutex::ShareToProcess( base::ProcessId aTargetPid) { HANDLE newHandle; - if (!::DuplicateHandle(GetCurrentProcess(), mMutex, GetCurrentProcess(), - &newHandle, 0, false, DUPLICATE_SAME_ACCESS)) { + bool succeeded = ipc::DuplicateHandle(mMutex, aTargetPid, &newHandle, 0, + DUPLICATE_SAME_ACCESS); + + if (!succeeded) { return nullptr; } - return mozilla::UniqueFileHandle(newHandle); + + return newHandle; } } // namespace mozilla diff --git a/ipc/glue/CrossProcessSemaphore.h b/ipc/glue/CrossProcessSemaphore.h index b5d37b27e4c7..8f80e59679a0 100644 --- a/ipc/glue/CrossProcessSemaphore.h +++ b/ipc/glue/CrossProcessSemaphore.h @@ -11,9 +11,6 @@ #include "mozilla/TimeStamp.h" #include "mozilla/Maybe.h" -#if defined(OS_WIN) -# include "mozilla/UniquePtrExtensions.h" -#endif #if !defined(OS_WIN) && !defined(OS_MACOSX) # include # include @@ -38,7 +35,7 @@ inline bool IsHandleValid(const T& handle) { } #if defined(OS_WIN) -typedef mozilla::UniqueFileHandle CrossProcessSemaphoreHandle; +typedef HANDLE CrossProcessSemaphoreHandle; #elif !defined(OS_MACOSX) typedef mozilla::ipc::SharedMemoryBasic::Handle CrossProcessSemaphoreHandle; diff --git a/ipc/glue/CrossProcessSemaphore_posix.cpp b/ipc/glue/CrossProcessSemaphore_posix.cpp index 47b6ab79f603..8c28d897fa54 100644 --- a/ipc/glue/CrossProcessSemaphore_posix.cpp +++ b/ipc/glue/CrossProcessSemaphore_posix.cpp @@ -67,8 +67,7 @@ CrossProcessSemaphore* CrossProcessSemaphore::Create( return nullptr; } - if (!sharedBuffer->SetHandle(std::move(aHandle), - ipc::SharedMemory::RightsReadWrite)) { + if (!sharedBuffer->SetHandle(aHandle, ipc::SharedMemory::RightsReadWrite)) { return nullptr; } diff --git a/ipc/glue/CrossProcessSemaphore_windows.cpp b/ipc/glue/CrossProcessSemaphore_windows.cpp index 899ffbd8ffcb..66da54be615e 100644 --- a/ipc/glue/CrossProcessSemaphore_windows.cpp +++ b/ipc/glue/CrossProcessSemaphore_windows.cpp @@ -35,11 +35,11 @@ CrossProcessSemaphore* CrossProcessSemaphore::Create(const char*, CrossProcessSemaphore* CrossProcessSemaphore::Create( CrossProcessSemaphoreHandle aHandle) { DWORD flags; - if (!::GetHandleInformation(aHandle.get(), &flags)) { + if (!::GetHandleInformation(aHandle, &flags)) { return nullptr; } - return new CrossProcessSemaphore(aHandle.release()); + return new CrossProcessSemaphore(aHandle); } CrossProcessSemaphore::CrossProcessSemaphore(HANDLE aSemaphore) @@ -68,15 +68,14 @@ void CrossProcessSemaphore::Signal() { CrossProcessSemaphoreHandle CrossProcessSemaphore::ShareToProcess( base::ProcessId aTargetPid) { HANDLE newHandle; - bool succeeded = - ::DuplicateHandle(GetCurrentProcess(), mSemaphore, GetCurrentProcess(), - &newHandle, 0, false, DUPLICATE_SAME_ACCESS); + bool succeeded = ipc::DuplicateHandle(mSemaphore, aTargetPid, &newHandle, 0, + DUPLICATE_SAME_ACCESS); if (!succeeded) { return nullptr; } - return UniqueFileHandle(newHandle); + return newHandle; } void CrossProcessSemaphore::CloseHandle() {} diff --git a/ipc/glue/FileDescriptor.cpp b/ipc/glue/FileDescriptor.cpp index a0e4b6f87293..950a0c17a0c2 100644 --- a/ipc/glue/FileDescriptor.cpp +++ b/ipc/glue/FileDescriptor.cpp @@ -35,6 +35,14 @@ FileDescriptor::FileDescriptor(PlatformHandleType aHandle) FileDescriptor::FileDescriptor(UniquePlatformHandle&& aHandle) : mHandle(std::move(aHandle)) {} +FileDescriptor::FileDescriptor(const IPDLPrivate&, const PickleType& aPickle) { +#ifdef XP_WIN + mHandle.reset(aPickle); +#else + mHandle.reset(aPickle.fd); +#endif +} + FileDescriptor::~FileDescriptor() = default; FileDescriptor& FileDescriptor::operator=(const FileDescriptor& aOther) { @@ -51,6 +59,33 @@ FileDescriptor& FileDescriptor::operator=(FileDescriptor&& aOther) { return *this; } +FileDescriptor::PickleType FileDescriptor::ShareTo( + const FileDescriptor::IPDLPrivate&, + FileDescriptor::ProcessId aTargetPid) const { + PlatformHandleType newHandle; +#ifdef XP_WIN + if (IsValid()) { + if (mozilla::ipc::DuplicateHandle(mHandle.get(), aTargetPid, &newHandle, 0, + DUPLICATE_SAME_ACCESS)) { + return newHandle; + } + NS_WARNING("Failed to duplicate file handle for other process!"); + } + return INVALID_HANDLE_VALUE; +#else // XP_WIN + if (IsValid()) { + newHandle = dup(mHandle.get()); + if (newHandle >= 0) { + return base::FileDescriptor(newHandle, /* auto_close */ true); + } + NS_WARNING("Failed to duplicate file handle for other process!"); + } + return base::FileDescriptor(); +#endif + + MOZ_CRASH("Must not get here!"); +} + bool FileDescriptor::IsValid() const { return mHandle != nullptr; } FileDescriptor::UniquePlatformHandle FileDescriptor::ClonePlatformHandle() @@ -95,19 +130,30 @@ FileDescriptor::UniquePlatformHandle FileDescriptor::Clone( void IPDLParamTraits::Write(IPC::Message* aMsg, IProtocol* aActor, const FileDescriptor& aParam) { - WriteIPDLParam(aMsg, aActor, aParam.ClonePlatformHandle()); +#ifdef XP_WIN + FileDescriptor::PickleType pfd = + aParam.ShareTo(FileDescriptor::IPDLPrivate(), aActor->OtherPid()); +#else + // The pid returned by OtherPID() is only required for Windows to + // send file descriptors. For the use case of the fork server, + // aActor is always null. Since it is only for the special case of + // Windows, here we skip it for other platforms. + FileDescriptor::PickleType pfd = + aParam.ShareTo(FileDescriptor::IPDLPrivate(), 0); +#endif + WriteIPDLParam(aMsg, aActor, pfd); } bool IPDLParamTraits::Read(const IPC::Message* aMsg, PickleIterator* aIter, IProtocol* aActor, FileDescriptor* aResult) { - UniqueFileHandle handle; - if (!ReadIPDLParam(aMsg, aIter, aActor, &handle)) { + FileDescriptor::PickleType pfd; + if (!ReadIPDLParam(aMsg, aIter, aActor, &pfd)) { return false; } - *aResult = FileDescriptor(std::move(handle)); + *aResult = FileDescriptor(FileDescriptor::IPDLPrivate(), pfd); if (!aResult->IsValid()) { printf_stderr("IPDL protocol Error: Received an invalid file descriptor\n"); } diff --git a/ipc/glue/FileDescriptor.h b/ipc/glue/FileDescriptor.h index 15160624da7a..0bc5ae569f6b 100644 --- a/ipc/glue/FileDescriptor.h +++ b/ipc/glue/FileDescriptor.h @@ -11,17 +11,25 @@ #include "base/process.h" #include "mozilla/UniquePtrExtensions.h" +#ifdef XP_UNIX +# include "base/file_descriptor_posix.h" +#endif + namespace mozilla { namespace ipc { // This class is used by IPDL to share file descriptors across processes. When -// sending a FileDescriptor, IPDL will transfer a duplicate of the handle into -// the remote process. +// sending a FileDescriptor IPDL will first duplicate a platform-specific file +// handle type ('PlatformHandleType') into a handle that is valid in the other +// process. Then IPDL will convert the duplicated handle into a type suitable +// for pickling ('PickleType') and then send that through the IPC pipe. In the +// receiving process the pickled data is converted into a platform-specific file +// handle and then returned to the receiver. // // To use this class add 'FileDescriptor' as an argument in the IPDL protocol -// and then pass a file descriptor from C++ to the Send method. The Recv method -// will receive a FileDescriptor& on which PlatformHandle() can be called to -// return the platform file handle. +// and then pass a file descriptor from C++ to the Call/Send method. The +// Answer/Recv method will receive a FileDescriptor& on which PlatformHandle() +// can be called to return the platform file handle. class FileDescriptor { public: typedef base::ProcessId ProcessId; @@ -29,6 +37,12 @@ class FileDescriptor { using UniquePlatformHandle = mozilla::UniqueFileHandle; using PlatformHandleType = UniquePlatformHandle::ElementType; +#ifdef XP_WIN + typedef PlatformHandleType PickleType; +#else + typedef base::FileDescriptor PickleType; +#endif + // This should only ever be created by IPDL. struct IPDLPrivate {}; @@ -46,12 +60,21 @@ class FileDescriptor { explicit FileDescriptor(UniquePlatformHandle&& aHandle); + // This constructor WILL NOT duplicate the handle. + // FileDescriptor takes the ownership from IPC message. + FileDescriptor(const IPDLPrivate&, const PickleType& aPickle); + ~FileDescriptor(); FileDescriptor& operator=(const FileDescriptor& aOther); FileDescriptor& operator=(FileDescriptor&& aOther); + // Performs platform-specific actions to duplicate mHandle in the other + // process (e.g. dup() on POSIX, DuplicateHandle() on Windows). Returns a + // pickled value that can be passed to the other process via IPC. + PickleType ShareTo(const IPDLPrivate&, ProcessId aTargetPid) const; + // Tests mHandle against a well-known invalid platform-specific file handle // (e.g. -1 on POSIX, INVALID_HANDLE_VALUE on Windows). bool IsValid() const; diff --git a/ipc/glue/IdleSchedulerChild.cpp b/ipc/glue/IdleSchedulerChild.cpp index 566799b74574..514c65afa472 100644 --- a/ipc/glue/IdleSchedulerChild.cpp +++ b/ipc/glue/IdleSchedulerChild.cpp @@ -32,7 +32,7 @@ void IdleSchedulerChild::Init(IdlePeriodState* aIdlePeriodState) { auto resolve = [&](Tuple, uint32_t>&& aResult) { if (Get<0>(aResult)) { - mActiveCounter.SetHandle(std::move(*Get<0>(aResult)), false); + mActiveCounter.SetHandle(*Get<0>(aResult), false); mActiveCounter.Map(sizeof(int32_t)); mChildId = Get<1>(aResult); if (mChildId && mIdlePeriodState && mIdlePeriodState->IsActive()) { diff --git a/ipc/glue/IdleSchedulerParent.cpp b/ipc/glue/IdleSchedulerParent.cpp index 9e8f60987f61..feb6cd110899 100644 --- a/ipc/glue/IdleSchedulerParent.cpp +++ b/ipc/glue/IdleSchedulerParent.cpp @@ -191,9 +191,10 @@ IPCResult IdleSchedulerParent::RecvInitForIdleUse( } } Maybe activeCounter; - if (SharedMemoryHandle handle = - sActiveChildCounter ? sActiveChildCounter->CloneHandle() : nullptr) { - activeCounter.emplace(std::move(handle)); + SharedMemoryHandle handle; + if (sActiveChildCounter && + sActiveChildCounter->ShareToProcess(OtherPid(), &handle)) { + activeCounter.emplace(handle); } uint32_t unusedId = 0; @@ -208,8 +209,8 @@ IPCResult IdleSchedulerParent::RecvInitForIdleUse( // If there wasn't an empty item, we'll fallback to 0. mChildId = unusedId; - aResolve(Tuple&&, const uint32_t&>( - std::move(activeCounter), mChildId)); + aResolve(Tuple&, const uint32_t&>( + activeCounter, mChildId)); return IPC_OK(); } diff --git a/ipc/glue/MiniTransceiver.cpp b/ipc/glue/MiniTransceiver.cpp index d79745b64821..505655ec6f26 100644 --- a/ipc/glue/MiniTransceiver.cpp +++ b/ipc/glue/MiniTransceiver.cpp @@ -38,18 +38,18 @@ namespace { * Initialize the IO vector for sending data and the control buffer for sending * FDs. */ -static void InitMsgHdr(msghdr* aHdr, int aIOVSize, size_t aMaxNumFds) { +static void InitMsgHdr(msghdr* aHdr, int aIOVSize, int aMaxNumFds) { aHdr->msg_name = nullptr; aHdr->msg_namelen = 0; aHdr->msg_flags = 0; // Prepare the IO vector to receive the content of message. - auto* iov = new iovec[aIOVSize]; + auto iov = new iovec[aIOVSize]; aHdr->msg_iov = iov; aHdr->msg_iovlen = aIOVSize; // Prepare the control buffer to receive file descriptors. - auto* cbuf = new char[CMSG_SPACE(sizeof(int) * aMaxNumFds)]; + auto cbuf = new char[CMSG_SPACE(sizeof(int) * aMaxNumFds)]; aHdr->msg_control = cbuf; aHdr->msg_controllen = CMSG_SPACE(sizeof(int) * aMaxNumFds); } @@ -66,20 +66,18 @@ static void DeinitMsgHdr(msghdr* aHdr) { void MiniTransceiver::PrepareFDs(msghdr* aHdr, IPC::Message& aMsg) { // Set control buffer to send file descriptors of the Message. - size_t num_fds = aMsg.attached_handles_.Length(); + int num_fds = aMsg.file_descriptor_set()->size(); cmsghdr* cmsg = CMSG_FIRSTHDR(aHdr); cmsg->cmsg_level = SOL_SOCKET; cmsg->cmsg_type = SCM_RIGHTS; cmsg->cmsg_len = CMSG_LEN(sizeof(int) * num_fds); - for (size_t i = 0; i < num_fds; ++i) { - reinterpret_cast(CMSG_DATA(cmsg))[i] = - aMsg.attached_handles_[i].get(); - } + aMsg.file_descriptor_set()->GetDescriptors( + reinterpret_cast(CMSG_DATA(cmsg))); // This number will be sent in the header of the message. So, we // can check it at the other side. - aMsg.header()->num_handles = num_fds; + aMsg.header()->num_fds = num_fds; } size_t MiniTransceiver::PrepareBuffers(msghdr* aHdr, IPC::Message& aMsg) { @@ -113,10 +111,11 @@ bool MiniTransceiver::Send(IPC::Message& aMsg) { mState = STATE_SENDING; #endif - auto clean_fdset = MakeScopeExit([&] { aMsg.attached_handles_.Clear(); }); + auto clean_fdset = + MakeScopeExit([&] { aMsg.file_descriptor_set()->CommitAll(); }); - size_t num_fds = aMsg.attached_handles_.Length(); - msghdr hdr{}; + int num_fds = aMsg.file_descriptor_set()->size(); + msghdr hdr; InitMsgHdr(&hdr, kMaxIOVecSize, num_fds); UniquePtr uniq(&hdr, &DeinitMsgHdr); @@ -228,11 +227,7 @@ bool MiniTransceiver::Recv(IPC::Message& aMsg) { // Create Message from databuf & all_fds. UniquePtr msg = MakeUnique(databuf.get(), msgsz); - nsTArray handles(num_all_fds); - for (unsigned i = 0; i < num_all_fds; ++i) { - handles.AppendElement(UniqueFileHandle(all_fds[i])); - } - msg->SetAttachedFileHandles(std::move(handles)); + msg->file_descriptor_set()->SetDescriptors(all_fds, num_all_fds); if (mDataBufClear == DataBufferClear::AfterReceiving) { // Avoid content processes from reading the content of @@ -240,7 +235,7 @@ bool MiniTransceiver::Recv(IPC::Message& aMsg) { memset(databuf.get(), 0, msgsz); } - MOZ_ASSERT(msg->header()->num_handles == msg->attached_handles_.Length(), + MOZ_ASSERT(msg->header()->num_fds == msg->file_descriptor_set()->size(), "The number of file descriptors in the header is different from" " the number actually received"); diff --git a/ipc/glue/NodeController.cpp b/ipc/glue/NodeController.cpp index dbbaf6762119..92eec9d7b82c 100644 --- a/ipc/glue/NodeController.cpp +++ b/ipc/glue/NodeController.cpp @@ -163,7 +163,6 @@ bool NodeController::SendUserMessage(const PortRef& aPort, } auto NodeController::SerializeEventMessage(UniquePtr aEvent, - const NodeName* aRelayTarget, uint32_t aType) -> UniquePtr { UniquePtr message; @@ -177,30 +176,18 @@ auto NodeController::SerializeEventMessage(UniquePtr aEvent, message = MakeUnique(MSG_ROUTING_CONTROL, aType); } - message->set_relay(aRelayTarget != nullptr); - - size_t length = aEvent->GetSerializedSize(); - if (aRelayTarget) { - length += sizeof(NodeName); - } - // Use an intermediate buffer to serialize to avoid potential issues with the // segmented `IPC::Message` bufferlist. This should be fairly cheap, as the // majority of events are fairly small. Vector buffer; - (void)buffer.initLengthUninitialized(length); - if (aRelayTarget) { - memcpy(buffer.begin(), aRelayTarget, sizeof(NodeName)); - aEvent->Serialize(buffer.begin() + sizeof(NodeName)); - } else { - aEvent->Serialize(buffer.begin()); - } + (void)buffer.initLengthUninitialized(aEvent->GetSerializedSize()); + aEvent->Serialize(buffer.begin()); message->WriteFooter(buffer.begin(), buffer.length()); #ifdef DEBUG // Debug-assert that we can read the same data back out of the buffer. - MOZ_ASSERT(message->FooterSize() == length); + MOZ_ASSERT(message->FooterSize() == aEvent->GetSerializedSize()); Vector buffer2; (void)buffer2.initLengthUninitialized(message->FooterSize()); MOZ_ASSERT(message->ReadFooter(buffer2.begin(), buffer2.length(), @@ -211,14 +198,8 @@ auto NodeController::SerializeEventMessage(UniquePtr aEvent, return message; } -auto NodeController::DeserializeEventMessage(UniquePtr aMessage, - NodeName* aRelayTarget) +auto NodeController::DeserializeEventMessage(UniquePtr aMessage) -> UniquePtr { - if (aMessage->is_relay() && !aRelayTarget) { - NODECONTROLLER_WARNING("Unexpected relay message '%s'", aMessage->name()); - return nullptr; - } - Vector buffer; (void)buffer.initLengthUninitialized(aMessage->FooterSize()); // Truncate the message when reading the footer, so that the extra footer data @@ -231,22 +212,7 @@ auto NodeController::DeserializeEventMessage(UniquePtr aMessage, return nullptr; } - UniquePtr event; - if (aRelayTarget) { - MOZ_ASSERT(aMessage->is_relay()); - if (buffer.length() < sizeof(NodeName)) { - NODECONTROLLER_WARNING( - "Insufficient space in message footer for message '%s'", - aMessage->name()); - return nullptr; - } - memcpy(aRelayTarget, buffer.begin(), sizeof(NodeName)); - event = Event::Deserialize(buffer.begin() + sizeof(NodeName), - buffer.length() - sizeof(NodeName)); - } else { - event = Event::Deserialize(buffer.begin(), buffer.length()); - } - + UniquePtr event = Event::Deserialize(buffer.begin(), buffer.length()); if (!event) { NODECONTROLLER_WARNING("Call to Event::Deserialize for message '%s' Failed", aMessage->name()); @@ -306,22 +272,7 @@ void NodeController::ForwardEvent(const NodeName& aNode, if (aNode == mName) { (void)mNode->AcceptEvent(std::move(aEvent)); } else { - // On Windows, messages holding HANDLEs must be relayed via the broker - // process so it can transfer handle ownership. - bool needsRelay = false; -#ifdef XP_WIN - if (!IsBroker() && aNode != kBrokerNodeName && - aEvent->type() == Event::kUserMessage) { - auto* userEvent = static_cast(aEvent.get()); - needsRelay = userEvent->HasMessage() && - userEvent->GetMessage()->num_handles() > 0; - } -#endif - - UniquePtr message = - SerializeEventMessage(std::move(aEvent), needsRelay ? &aNode : nullptr); - MOZ_ASSERT(message->is_relay() == needsRelay, - "Message relay status set incorrectly"); + UniquePtr message = SerializeEventMessage(std::move(aEvent)); RefPtr peer; RefPtr broker; @@ -332,7 +283,7 @@ void NodeController::ForwardEvent(const NodeName& aNode, // Check if we know this peer. If we don't, we'll need to request an // introduction. peer = state->mPeers.Get(aNode); - if (!peer || needsRelay) { + if (!peer) { if (IsBroker()) { NODECONTROLLER_WARNING("Ignoring message '%s' to unknown peer %s", message->name(), ToString(aNode).c_str()); @@ -347,32 +298,18 @@ void NodeController::ForwardEvent(const NodeName& aNode, return; } - if (!needsRelay) { - auto& queue = - state->mPendingMessages.LookupOrInsertWith(aNode, [&]() { - needsIntroduction = true; - return Queue, 64>{}; - }); - queue.Push(std::move(message)); - } + auto& queue = state->mPendingMessages.LookupOrInsertWith(aNode, [&]() { + needsIntroduction = true; + return Queue, 64>{}; + }); + queue.Push(std::move(message)); } } - MOZ_ASSERT(!needsIntroduction || !needsRelay, - "Only one of the two should ever be set"); + // TODO: On windows, we can relay the message through the broker process + // here to handle transferring handles without using sync IPC. - if (needsRelay) { -#ifdef XP_WIN - NODECONTROLLER_LOG(LogLevel::Info, - "Relaying message '%s' for peer %s due to %lu handles", - message->name(), ToString(aNode).c_str(), - message->num_handles()); - MOZ_ASSERT(broker); - broker->SendEventMessage(std::move(message)); -#else - MOZ_ASSERT_UNREACHABLE("relaying messages is only supported on windows"); -#endif - } else if (needsIntroduction) { + if (needsIntroduction) { MOZ_ASSERT(broker); broker->RequestIntroduction(aNode); } else if (peer) { @@ -383,7 +320,7 @@ void NodeController::ForwardEvent(const NodeName& aNode, void NodeController::BroadcastEvent(UniquePtr aEvent) { UniquePtr message = - SerializeEventMessage(std::move(aEvent), nullptr, BROADCAST_MESSAGE_TYPE); + SerializeEventMessage(std::move(aEvent), BROADCAST_MESSAGE_TYPE); if (IsBroker()) { OnBroadcast(mName, std::move(message)); @@ -415,11 +352,7 @@ void NodeController::OnEventMessage(const NodeName& aFromNode, UniquePtr aMessage) { AssertIOThread(); - bool isRelay = aMessage->is_relay(); - NodeName relayTarget; - - UniquePtr event = DeserializeEventMessage( - std::move(aMessage), isRelay ? &relayTarget : nullptr); + UniquePtr event = DeserializeEventMessage(std::move(aMessage)); if (!event) { NODECONTROLLER_WARNING("Invalid EventMessage from peer %s!", ToString(aFromNode).c_str()); @@ -427,70 +360,6 @@ void NodeController::OnEventMessage(const NodeName& aFromNode, return; } - NodeName fromNode = aFromNode; -#ifdef XP_WIN - if (isRelay) { - if (event->type() != Event::kUserMessage) { - NODECONTROLLER_WARNING( - "Unexpected relay of non-UserMessage event from peer %s!", - ToString(aFromNode).c_str()); - DropPeer(aFromNode); - return; - } - - // If we're the broker, then we'll need to forward this message on to the - // true recipient. To do this, we re-serialize the message, passing along - // the original source node, and send it to the final node. - if (IsBroker()) { - UniquePtr message = - SerializeEventMessage(std::move(event), &aFromNode); - if (!message) { - NODECONTROLLER_WARNING( - "Relaying EventMessage from peer %s failed to re-serialize!", - ToString(aFromNode).c_str()); - DropPeer(aFromNode); - return; - } - MOZ_ASSERT(message->is_relay(), "Message stopped being a relay message?"); - - NODECONTROLLER_LOG( - LogLevel::Info, - "Relaying message '%s' from peer %s to peer %s (%lu handles)", - message->name(), ToString(aFromNode).c_str(), - ToString(relayTarget).c_str(), message->num_handles()); - - RefPtr peer; - { - auto state = mState.Lock(); - peer = state->mPeers.Get(relayTarget); - } - if (!peer) { - NODECONTROLLER_WARNING( - "Dropping relayed message from %s to unknown peer %s", - ToString(aFromNode).c_str(), ToString(relayTarget).c_str()); - return; - } - - peer->SendEventMessage(std::move(message)); - return; - } - - // Otherwise, we're the final recipient, so we can continue & process the - // message as usual. - if (aFromNode != kBrokerNodeName) { - NODECONTROLLER_WARNING( - "Unexpected relayed EventMessage from non-broker peer %s!", - ToString(aFromNode).c_str()); - DropPeer(aFromNode); - return; - } - fromNode = relayTarget; - - NODECONTROLLER_LOG(LogLevel::Info, "Got relayed message from peer %s", - ToString(fromNode).c_str()); - } -#endif - // If we're getting a requested port merge from another process, check to make // sure that we're expecting the request, and record that the merge has // arrived so we don't try to close the port on error. @@ -500,8 +369,8 @@ void NodeController::OnEventMessage(const NodeName& aFromNode, if (!targetPort.is_valid()) { NODECONTROLLER_WARNING( "Unexpected MergePortEvent from peer %s for unknown port %s", - ToString(fromNode).c_str(), ToString(event->port_name()).c_str()); - DropPeer(fromNode); + ToString(aFromNode).c_str(), ToString(event->port_name()).c_str()); + DropPeer(aFromNode); return; } @@ -524,8 +393,8 @@ void NodeController::OnEventMessage(const NodeName& aFromNode, if (!expectingMerge) { NODECONTROLLER_WARNING( "Unexpected MergePortEvent from peer %s for port %s", - ToString(fromNode).c_str(), ToString(event->port_name()).c_str()); - DropPeer(fromNode); + ToString(aFromNode).c_str(), ToString(event->port_name()).c_str()); + DropPeer(aFromNode); return; } } @@ -651,7 +520,7 @@ void NodeController::OnRequestIntroduction(const NodeName& aFromNode, } RefPtr peerB = GetNodeChannel(aName); - auto result = AutoTransportDescriptor::Create(); + auto result = AutoTransportDescriptor::Create(peerA->OtherPid()); if (!peerB || result.isErr()) { NODECONTROLLER_WARNING( "Rejecting introduction request from '%s' for unknown peer '%s'", diff --git a/ipc/glue/NodeController.h b/ipc/glue/NodeController.h index 377dba228149..0412271d62d9 100644 --- a/ipc/glue/NodeController.h +++ b/ipc/glue/NodeController.h @@ -105,10 +105,8 @@ class NodeController final : public mojo::core::ports::NodeDelegate, ~NodeController(); UniquePtr SerializeEventMessage( - UniquePtr aEvent, const NodeName* aRelayTarget = nullptr, - uint32_t aType = EVENT_MESSAGE_TYPE); - UniquePtr DeserializeEventMessage(UniquePtr aMessage, - NodeName* aRelayTarget = nullptr); + UniquePtr aEvent, uint32_t aType = EVENT_MESSAGE_TYPE); + UniquePtr DeserializeEventMessage(UniquePtr aMessage); // Get the `NodeChannel` for the named node. already_AddRefed GetNodeChannel(const NodeName& aName); diff --git a/ipc/glue/PIdleScheduler.ipdl b/ipc/glue/PIdleScheduler.ipdl index 19523a2bc3b1..c443fe7affa8 100644 --- a/ipc/glue/PIdleScheduler.ipdl +++ b/ipc/glue/PIdleScheduler.ipdl @@ -5,7 +5,7 @@ include protocol PBackground; using mozilla::TimeDuration from "mozilla/TimeStamp.h"; -[MoveOnly] using base::SharedMemoryHandle from "base/shared_memory.h"; +using base::SharedMemoryHandle from "base/shared_memory.h"; namespace mozilla { namespace ipc { diff --git a/ipc/glue/ProcessUtils.h b/ipc/glue/ProcessUtils.h index 53297089a5bf..f5487a46e1fe 100644 --- a/ipc/glue/ProcessUtils.h +++ b/ipc/glue/ProcessUtils.h @@ -57,10 +57,12 @@ class SharedPreferenceDeserializer final { uint64_t aPrefMapHandle, uint64_t aPrefsLen, uint64_t aPrefMapSize); + const base::SharedMemoryHandle& GetPrefsHandle() const; const FileDescriptor& GetPrefMapHandle() const; private: DISALLOW_COPY_AND_ASSIGN(SharedPreferenceDeserializer); + Maybe mPrefsHandle; Maybe mPrefMapHandle; Maybe mPrefsLen; Maybe mPrefMapSize; diff --git a/ipc/glue/ProcessUtils_common.cpp b/ipc/glue/ProcessUtils_common.cpp index d2108eb9f647..de552f757b0f 100644 --- a/ipc/glue/ProcessUtils_common.cpp +++ b/ipc/glue/ProcessUtils_common.cpp @@ -109,10 +109,8 @@ SharedPreferenceDeserializer::~SharedPreferenceDeserializer() { bool SharedPreferenceDeserializer::DeserializeFromSharedMemory( uint64_t aPrefsHandle, uint64_t aPrefMapHandle, uint64_t aPrefsLen, uint64_t aPrefMapSize) { - Maybe prefsHandle; - #ifdef XP_WIN - prefsHandle = Some(UniqueFileHandle(HANDLE((uintptr_t)(aPrefsHandle)))); + mPrefsHandle = Some(HANDLE((uintptr_t)(aPrefsHandle))); if (!aPrefsHandle) { return false; } @@ -139,16 +137,17 @@ bool SharedPreferenceDeserializer::DeserializeFromSharedMemory( #ifdef ANDROID // Android is different; get the FD via gPrefsFd instead of a fixed fd. MOZ_RELEASE_ASSERT(gPrefsFd != -1); - prefsHandle = Some(UniqueFileHandle(gPrefsFd)); + mPrefsHandle = Some(base::FileDescriptor(gPrefsFd, /* auto_close */ true)); mPrefMapHandle.emplace(UniqueFileHandle(gPrefMapFd)); #elif XP_UNIX - prefsHandle = Some(UniqueFileHandle(kPrefsFileDescriptor)); + mPrefsHandle = Some(base::FileDescriptor(kPrefsFileDescriptor, + /* auto_close */ true)); mPrefMapHandle.emplace(UniqueFileHandle(kPrefMapFileDescriptor)); #endif - if (prefsHandle.isNothing() || mPrefsLen.isNothing() || + if (mPrefsHandle.isNothing() || mPrefsLen.isNothing() || mPrefMapHandle.isNothing() || mPrefMapSize.isNothing()) { return false; } @@ -158,7 +157,7 @@ bool SharedPreferenceDeserializer::DeserializeFromSharedMemory( Preferences::InitSnapshot(mPrefMapHandle.ref(), *mPrefMapSize); // Set up early prefs from the shared memory. - if (!mShmem.SetHandle(std::move(*prefsHandle), /* read_only */ true)) { + if (!mShmem.SetHandle(*mPrefsHandle, /* read_only */ true)) { NS_ERROR("failed to open shared memory in the child"); return false; } @@ -172,6 +171,13 @@ bool SharedPreferenceDeserializer::DeserializeFromSharedMemory( return true; } +const base::SharedMemoryHandle& SharedPreferenceDeserializer::GetPrefsHandle() + const { + MOZ_ASSERT(mPrefsHandle.isSome()); + + return mPrefsHandle.ref(); +} + const FileDescriptor& SharedPreferenceDeserializer::GetPrefMapHandle() const { MOZ_ASSERT(mPrefMapHandle.isSome()); @@ -248,13 +254,14 @@ bool ImportSharedJSInit(uint64_t aJsInitHandle, uint64_t aJsInitLen) { } #ifdef XP_UNIX - auto handle = UniqueFileHandle(kJSInitFileDescriptor); + auto handle = base::FileDescriptor(kJSInitFileDescriptor, + /* auto_close */ true); #endif // Initialize the shared memory with the file handle and size of the content // of the self-hosted Xdr. auto& shmem = xpc::SelfHostedShmem::GetSingleton(); - if (!shmem.InitFromChild(std::move(handle), len)) { + if (!shmem.InitFromChild(handle, len)) { NS_ERROR("failed to open shared memory in the child"); return false; } diff --git a/ipc/glue/ProtocolUtils.cpp b/ipc/glue/ProtocolUtils.cpp index d11f13461d46..68a33e9bedf7 100644 --- a/ipc/glue/ProtocolUtils.cpp +++ b/ipc/glue/ProtocolUtils.cpp @@ -65,6 +65,42 @@ IPCResult IPCResult::Fail(NotNull actor, const char* where, return IPCResult(false); } +#if defined(XP_WIN) +bool DuplicateHandle(HANDLE aSourceHandle, DWORD aTargetProcessId, + HANDLE* aTargetHandle, DWORD aDesiredAccess, + DWORD aOptions) { + // If our process is the target just duplicate the handle. + if (aTargetProcessId == base::GetCurrentProcId()) { + return !!::DuplicateHandle(::GetCurrentProcess(), aSourceHandle, + ::GetCurrentProcess(), aTargetHandle, + aDesiredAccess, false, aOptions); + } + +# if defined(MOZ_SANDBOX) + // Try the broker next (will fail if not sandboxed). + if (SandboxTarget::Instance()->BrokerDuplicateHandle( + aSourceHandle, aTargetProcessId, aTargetHandle, aDesiredAccess, + aOptions)) { + return true; + } +# endif + + // Finally, see if we already have access to the process. + ScopedProcessHandle targetProcess( + OpenProcess(PROCESS_DUP_HANDLE, FALSE, aTargetProcessId)); + if (!targetProcess) { + CrashReporter::AnnotateCrashReport( + CrashReporter::Annotation::IPCTransportFailureReason, + "Failed to open target process."_ns); + return false; + } + + return !!::DuplicateHandle(::GetCurrentProcess(), aSourceHandle, + targetProcess, aTargetHandle, aDesiredAccess, + FALSE, aOptions); +} +#endif + void AnnotateSystemError() { int64_t error = 0; #if defined(XP_WIN) diff --git a/ipc/glue/ProtocolUtils.h b/ipc/glue/ProtocolUtils.h index 24ea2c233d8b..ae562f26a092 100644 --- a/ipc/glue/ProtocolUtils.h +++ b/ipc/glue/ProtocolUtils.h @@ -639,6 +639,16 @@ MOZ_NEVER_INLINE void ArrayLengthReadError(const char* aElementName); MOZ_NEVER_INLINE void SentinelReadError(const char* aElementName); +#if defined(XP_WIN) +// This is a restricted version of Windows' DuplicateHandle() function +// that works inside the sandbox and can send handles but not retrieve +// them. Unlike DuplicateHandle(), it takes a process ID rather than +// a process handle. It returns true on success, false otherwise. +bool DuplicateHandle(HANDLE aSourceHandle, DWORD aTargetProcessId, + HANDLE* aTargetHandle, DWORD aDesiredAccess, + DWORD aOptions); +#endif + /** * Annotate the crash reporter with the error code from the most recent system * call. Returns the system error. diff --git a/ipc/glue/SharedMemory.h b/ipc/glue/SharedMemory.h index cb793c7f0339..9c2f6bf6785e 100644 --- a/ipc/glue/SharedMemory.h +++ b/ipc/glue/SharedMemory.h @@ -121,7 +121,7 @@ class SharedMemoryCommon : public SharedMemory { virtual bool ShareToProcess(base::ProcessId aProcessId, Handle* aHandle) = 0; virtual bool IsHandleValid(const Handle& aHandle) const = 0; - virtual bool SetHandle(Handle aHandle, OpenRights aRights) = 0; + virtual bool SetHandle(const Handle& aHandle, OpenRights aRights) = 0; virtual bool ShareHandle(base::ProcessId aProcessId, IPC::Message* aMessage) override { @@ -129,7 +129,7 @@ class SharedMemoryCommon : public SharedMemory { if (!ShareToProcess(aProcessId, &handle)) { return false; } - IPC::WriteParam(aMessage, std::move(handle)); + IPC::WriteParam(aMessage, handle); return true; } @@ -137,7 +137,7 @@ class SharedMemoryCommon : public SharedMemory { PickleIterator* aIter) override { Handle handle; return IPC::ReadParam(aMessage, aIter, &handle) && IsHandleValid(handle) && - SetHandle(std::move(handle), RightsReadWrite); + SetHandle(handle, RightsReadWrite); } }; diff --git a/ipc/glue/SharedMemoryBasic_android.cpp b/ipc/glue/SharedMemoryBasic_android.cpp index 9eb20ceb8721..30eb169e995b 100644 --- a/ipc/glue/SharedMemoryBasic_android.cpp +++ b/ipc/glue/SharedMemoryBasic_android.cpp @@ -36,9 +36,9 @@ SharedMemoryBasic::~SharedMemoryBasic() { CloseHandle(); } -bool SharedMemoryBasic::SetHandle(Handle aHandle, OpenRights aRights) { +bool SharedMemoryBasic::SetHandle(const Handle& aHandle, OpenRights aRights) { MOZ_ASSERT(-1 == mShmFd, "Already Create()d"); - mShmFd = aHandle.release(); + mShmFd = aHandle.fd; mOpenRights = aRights; return true; } @@ -107,7 +107,8 @@ bool SharedMemoryBasic::ShareToProcess(base::ProcessId /*unused*/, return false; } - *aNewHandle = mozilla::UniqueFileHandle(shmfdDup); + aNewHandle->fd = shmfdDup; + aNewHandle->auto_close = true; return true; } diff --git a/ipc/glue/SharedMemoryBasic_android.h b/ipc/glue/SharedMemoryBasic_android.h index d6108d400b1f..c30e7357cf22 100644 --- a/ipc/glue/SharedMemoryBasic_android.h +++ b/ipc/glue/SharedMemoryBasic_android.h @@ -7,8 +7,9 @@ #ifndef mozilla_ipc_SharedMemoryBasic_android_h #define mozilla_ipc_SharedMemoryBasic_android_h +#include "base/file_descriptor_posix.h" + #include "mozilla/ipc/SharedMemory.h" -#include "mozilla/UniquePtrExtensions.h" #ifdef FUZZING # include "mozilla/ipc/SharedMemoryFuzzer.h" @@ -23,11 +24,11 @@ namespace mozilla { namespace ipc { class SharedMemoryBasic final - : public SharedMemoryCommon { + : public SharedMemoryCommon { public: SharedMemoryBasic(); - virtual bool SetHandle(Handle aHandle, OpenRights aRights) override; + virtual bool SetHandle(const Handle& aHandle, OpenRights aRights) override; virtual bool Create(size_t aNbytes) override; @@ -52,7 +53,7 @@ class SharedMemoryBasic final static void* FindFreeAddressSpace(size_t aSize); virtual bool IsHandleValid(const Handle& aHandle) const override { - return aHandle != nullptr; + return aHandle.fd >= 0; } virtual bool ShareToProcess(base::ProcessId aProcessId, diff --git a/ipc/glue/SharedMemoryBasic_chromium.h b/ipc/glue/SharedMemoryBasic_chromium.h index 4ee1f7c4b506..c9602f01b92c 100644 --- a/ipc/glue/SharedMemoryBasic_chromium.h +++ b/ipc/glue/SharedMemoryBasic_chromium.h @@ -29,9 +29,8 @@ class SharedMemoryBasic final public: SharedMemoryBasic() = default; - virtual bool SetHandle(Handle aHandle, OpenRights aRights) override { - return mSharedMemory.SetHandle(std::move(aHandle), - aRights == RightsReadOnly); + virtual bool SetHandle(const Handle& aHandle, OpenRights aRights) override { + return mSharedMemory.SetHandle(aHandle, aRights == RightsReadOnly); } virtual bool Create(size_t aNbytes) override { @@ -73,8 +72,10 @@ class SharedMemoryBasic final virtual bool ShareToProcess(base::ProcessId aProcessId, Handle* new_handle) override { - *new_handle = mSharedMemory.CloneHandle(); - return base::SharedMemory::IsHandleValid(*new_handle); + base::SharedMemoryHandle handle; + bool ret = mSharedMemory.ShareToProcess(aProcessId, &handle); + if (ret) *new_handle = handle; + return ret; } static void* FindFreeAddressSpace(size_t size) { diff --git a/ipc/glue/SharedMemoryBasic_mach.h b/ipc/glue/SharedMemoryBasic_mach.h index 18ea5e60e338..32c7ae618dfc 100644 --- a/ipc/glue/SharedMemoryBasic_mach.h +++ b/ipc/glue/SharedMemoryBasic_mach.h @@ -7,6 +7,7 @@ #ifndef mozilla_ipc_SharedMemoryBasic_mach_h #define mozilla_ipc_SharedMemoryBasic_mach_h +#include "base/file_descriptor_posix.h" #include "base/process.h" #include "mozilla/ipc/SharedMemory.h" @@ -66,7 +67,7 @@ class SharedMemoryBasic final : public SharedMemoryCommon { SharedMemoryBasic(); - virtual bool SetHandle(Handle aHandle, OpenRights aRights) override; + virtual bool SetHandle(const Handle& aHandle, OpenRights aRights) override; virtual bool Create(size_t aNbytes) override; diff --git a/ipc/glue/SharedMemoryBasic_mach.mm b/ipc/glue/SharedMemoryBasic_mach.mm index 2f6bd5c947e2..306f2504e0d8 100644 --- a/ipc/glue/SharedMemoryBasic_mach.mm +++ b/ipc/glue/SharedMemoryBasic_mach.mm @@ -509,7 +509,7 @@ SharedMemoryBasic::~SharedMemoryBasic() { CloseHandle(); } -bool SharedMemoryBasic::SetHandle(Handle aHandle, OpenRights aRights) { +bool SharedMemoryBasic::SetHandle(const Handle& aHandle, OpenRights aRights) { MOZ_ASSERT(mPort == MACH_PORT_NULL, "already initialized"); mPort = aHandle; diff --git a/ipc/glue/Transport.h b/ipc/glue/Transport.h index e9f9ea4b9ce5..9621d023544f 100644 --- a/ipc/glue/Transport.h +++ b/ipc/glue/Transport.h @@ -20,13 +20,19 @@ namespace mozilla { namespace ipc { +class FileDescriptor; + typedef IPC::Channel Transport; -nsresult CreateTransport(TransportDescriptor* aOne, TransportDescriptor* aTwo); +nsresult CreateTransport(base::ProcessId aProcIdOne, TransportDescriptor* aOne, + TransportDescriptor* aTwo); UniquePtr OpenDescriptor(const TransportDescriptor& aTd, Transport::Mode aMode); +UniquePtr OpenDescriptor(const FileDescriptor& aFd, + Transport::Mode aMode); + TransportDescriptor DuplicateDescriptor(const TransportDescriptor& aTd); void CloseDescriptor(const TransportDescriptor& aTd); diff --git a/ipc/glue/Transport_posix.cpp b/ipc/glue/Transport_posix.cpp index fd562f01bbda..2cf3ab979011 100644 --- a/ipc/glue/Transport_posix.cpp +++ b/ipc/glue/Transport_posix.cpp @@ -20,7 +20,8 @@ using base::ProcessHandle; namespace mozilla { namespace ipc { -nsresult CreateTransport(TransportDescriptor* aOne, TransportDescriptor* aTwo) { +nsresult CreateTransport(base::ProcessId aProcIdOne, TransportDescriptor* aOne, + TransportDescriptor* aTwo) { auto id = IPC::Channel::GenerateVerifiedChannelID(); // Use MODE_SERVER to force creation of the socketpair Transport t(id, Transport::MODE_SERVER, nullptr); @@ -50,27 +51,33 @@ nsresult CreateTransport(TransportDescriptor* aOne, TransportDescriptor* aTwo) { return NS_ERROR_DUPLICATE_HANDLE; } - aOne->mFd = fd1; - aTwo->mFd = fd2; + aOne->mFd = base::FileDescriptor(fd1, true /*close after sending*/); + aTwo->mFd = base::FileDescriptor(fd2, true /*close after sending*/); return NS_OK; } UniquePtr OpenDescriptor(const TransportDescriptor& aTd, Transport::Mode aMode) { - return MakeUnique(aTd.mFd, aMode, nullptr); + return MakeUnique(aTd.mFd.fd, aMode, nullptr); +} + +UniquePtr OpenDescriptor(const FileDescriptor& aFd, + Transport::Mode aMode) { + auto rawFD = aFd.ClonePlatformHandle(); + return MakeUnique(rawFD.release(), aMode, nullptr); } TransportDescriptor DuplicateDescriptor(const TransportDescriptor& aTd) { TransportDescriptor result = aTd; - result.mFd = dup(aTd.mFd); - if (result.mFd == -1) { + result.mFd.fd = dup(aTd.mFd.fd); + if (result.mFd.fd == -1) { AnnotateSystemError(); } - MOZ_RELEASE_ASSERT(result.mFd != -1, "DuplicateDescriptor failed"); + MOZ_RELEASE_ASSERT(result.mFd.fd != -1, "DuplicateDescriptor failed"); return result; } -void CloseDescriptor(const TransportDescriptor& aTd) { close(aTd.mFd); } +void CloseDescriptor(const TransportDescriptor& aTd) { close(aTd.mFd.fd); } } // namespace ipc } // namespace mozilla diff --git a/ipc/glue/Transport_posix.h b/ipc/glue/Transport_posix.h index dceb6e125671..f0dfeccb49ef 100644 --- a/ipc/glue/Transport_posix.h +++ b/ipc/glue/Transport_posix.h @@ -13,7 +13,7 @@ namespace mozilla { namespace ipc { struct TransportDescriptor { - int mFd; + base::FileDescriptor mFd; }; } // namespace ipc @@ -25,16 +25,11 @@ template <> struct ParamTraits { typedef mozilla::ipc::TransportDescriptor paramType; static void Write(Message* aMsg, const paramType& aParam) { - WriteParam(aMsg, mozilla::UniqueFileHandle{aParam.mFd}); + WriteParam(aMsg, aParam.mFd); } static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) { - mozilla::UniqueFileHandle fd; - if (!ReadParam(aMsg, aIter, &fd)) { - return false; - } - aResult->mFd = fd.release(); - return true; + return ReadParam(aMsg, aIter, &aResult->mFd); } }; diff --git a/ipc/glue/Transport_win.cpp b/ipc/glue/Transport_win.cpp index 493dd937d51d..fddd781b41b4 100644 --- a/ipc/glue/Transport_win.cpp +++ b/ipc/glue/Transport_win.cpp @@ -8,48 +8,81 @@ #include "mozilla/ipc/Transport.h" #include "mozilla/ipc/ProtocolUtils.h" -#include using base::ProcessHandle; namespace mozilla { namespace ipc { -nsresult CreateTransport(TransportDescriptor* aOne, TransportDescriptor* aTwo) { +nsresult CreateTransport(base::ProcessId aProcIdOne, TransportDescriptor* aOne, + TransportDescriptor* aTwo) { auto id = IPC::Channel::GenerateVerifiedChannelID(); - // Use MODE_SERVER to force creation of the pipe - // NB: we create the server pipe immediately, instead of just - // grabbing an ID, on purpose. In the current setup, the client - // needs to connect to an existing server pipe, so to prevent race - // conditions, we create the server side here. When we send the pipe - // to the server, we DuplicateHandle it to the server process to give it - // access. Transport t(id, Transport::MODE_SERVER, nullptr); HANDLE serverPipe = t.GetServerPipeHandle(); if (!serverPipe) { return NS_ERROR_TRANSPORT_INIT; } - // Make a copy of the handle owned by the `Transport` which will be - // transferred to the actual server process. - if (!::DuplicateHandle(GetCurrentProcess(), serverPipe, GetCurrentProcess(), - &aOne->mServerPipeHandle, 0, false, - DUPLICATE_SAME_ACCESS)) { + // NB: we create the server pipe immediately, instead of just + // grabbing an ID, on purpose. In the current setup, the client + // needs to connect to an existing server pipe, so to prevent race + // conditions, we create the server side here. When we send the pipe + // to the server, we DuplicateHandle it to the server process to give it + // access. + HANDLE serverDup; + DWORD access = 0; + DWORD options = DUPLICATE_SAME_ACCESS; + if (!DuplicateHandle(serverPipe, base::GetCurrentProcId(), &serverDup, access, + options)) { return NS_ERROR_DUPLICATE_HANDLE; } aOne->mPipeName = aTwo->mPipeName = id; + aOne->mServerPipeHandle = serverDup; + aOne->mDestinationProcessId = aProcIdOne; aTwo->mServerPipeHandle = INVALID_HANDLE_VALUE; + aTwo->mDestinationProcessId = 0; return NS_OK; } +HANDLE +TransferHandleToProcess(HANDLE source, base::ProcessId pid) { + // At this point we're sending the handle to another process. + + if (source == INVALID_HANDLE_VALUE) { + return source; + } + HANDLE handleDup; + DWORD access = 0; + DWORD options = DUPLICATE_SAME_ACCESS; + bool ok = DuplicateHandle(source, pid, &handleDup, access, options); + if (!ok) { + return nullptr; + } + + // Now close our own copy of the handle (we're supposed to be transferring, + // not copying). + CloseHandle(source); + + return handleDup; +} + UniquePtr OpenDescriptor(const TransportDescriptor& aTd, Transport::Mode aMode) { + if (aTd.mServerPipeHandle != INVALID_HANDLE_VALUE) { + MOZ_RELEASE_ASSERT(aTd.mDestinationProcessId == base::GetCurrentProcId()); + } return MakeUnique(aTd.mPipeName, aTd.mServerPipeHandle, aMode, nullptr); } +UniquePtr OpenDescriptor(const FileDescriptor& aFd, + Transport::Mode aMode) { + MOZ_ASSERT_UNREACHABLE("Not implemented!"); + return nullptr; +} + TransportDescriptor DuplicateDescriptor(const TransportDescriptor& aTd) { // We're duplicating this handle in our own process for bookkeeping purposes. @@ -58,9 +91,10 @@ TransportDescriptor DuplicateDescriptor(const TransportDescriptor& aTd) { } HANDLE serverDup; - bool ok = ::DuplicateHandle(GetCurrentProcess(), aTd.mServerPipeHandle, - GetCurrentProcess(), &serverDup, 0, false, - DUPLICATE_SAME_ACCESS); + DWORD access = 0; + DWORD options = DUPLICATE_SAME_ACCESS; + bool ok = DuplicateHandle(aTd.mServerPipeHandle, base::GetCurrentProcId(), + &serverDup, access, options); if (!ok) { AnnotateSystemError(); } diff --git a/ipc/glue/Transport_win.h b/ipc/glue/Transport_win.h index f093a07be2e0..9cc45bcd191a 100644 --- a/ipc/glue/Transport_win.h +++ b/ipc/glue/Transport_win.h @@ -13,7 +13,6 @@ #include "ipc/IPCMessageUtils.h" #include "nsWindowsHelpers.h" #include "nsXULAppAPI.h" -#include "mozilla/UniquePtrExtensions.h" namespace mozilla { namespace ipc { @@ -24,6 +23,9 @@ struct TransportDescriptor { base::ProcessId mDestinationProcessId; }; +HANDLE +TransferHandleToProcess(HANDLE source, base::ProcessId pid); + } // namespace ipc } // namespace mozilla @@ -33,25 +35,71 @@ template <> struct ParamTraits { typedef mozilla::ipc::TransportDescriptor paramType; static void Write(Message* aMsg, const paramType& aParam) { + HANDLE pipe = mozilla::ipc::TransferHandleToProcess( + aParam.mServerPipeHandle, aParam.mDestinationProcessId); + DWORD duplicateFromProcessId = 0; + if (!pipe) { + if (XRE_IsParentProcess()) { + // If we are the parent and failed to transfer then there is no hope, + // just close the handle. + ::CloseHandle(aParam.mServerPipeHandle); + } else { + // We are probably sending to parent so it should be able to duplicate. + pipe = aParam.mServerPipeHandle; + duplicateFromProcessId = ::GetCurrentProcessId(); + } + } + WriteParam(aMsg, aParam.mPipeName); - WriteParam(aMsg, mozilla::UniqueFileHandle(aParam.mServerPipeHandle)); + WriteParam(aMsg, pipe); + WriteParam(aMsg, duplicateFromProcessId); WriteParam(aMsg, aParam.mDestinationProcessId); } static bool Read(const Message* aMsg, PickleIterator* aIter, paramType* aResult) { - mozilla::UniqueFileHandle serverPipeHandle; + DWORD duplicateFromProcessId; bool r = (ReadParam(aMsg, aIter, &aResult->mPipeName) && - ReadParam(aMsg, aIter, &serverPipeHandle) && + ReadParam(aMsg, aIter, &aResult->mServerPipeHandle) && + ReadParam(aMsg, aIter, &duplicateFromProcessId) && ReadParam(aMsg, aIter, &aResult->mDestinationProcessId)); if (!r) { return r; } - if (serverPipeHandle) { - aResult->mServerPipeHandle = serverPipeHandle.release(); - } else { - aResult->mServerPipeHandle = INVALID_HANDLE_VALUE; + MOZ_RELEASE_ASSERT( + aResult->mServerPipeHandle, + "Main process failed to duplicate pipe handle to child."); + + // If this is a not the "server" side descriptor, we have finished. + if (aResult->mServerPipeHandle == INVALID_HANDLE_VALUE) { + return true; } + + MOZ_RELEASE_ASSERT(aResult->mDestinationProcessId == + base::GetCurrentProcId()); + + // If the pipe has already been duplicated to us, we have finished. + if (!duplicateFromProcessId) { + return true; + } + + // Otherwise duplicate the handle to us. + nsAutoHandle sourceProcess( + ::OpenProcess(PROCESS_DUP_HANDLE, FALSE, duplicateFromProcessId)); + if (!sourceProcess) { + return false; + } + + HANDLE ourHandle; + BOOL duped = ::DuplicateHandle( + sourceProcess, aResult->mServerPipeHandle, ::GetCurrentProcess(), + &ourHandle, 0, FALSE, DUPLICATE_CLOSE_SOURCE | DUPLICATE_SAME_ACCESS); + if (!duped) { + aResult->mServerPipeHandle = INVALID_HANDLE_VALUE; + return false; + } + + aResult->mServerPipeHandle = ourHandle; return true; } }; diff --git a/ipc/gtest/TestSharedMemory.cpp b/ipc/gtest/TestSharedMemory.cpp index f1023df21945..1474be0ea0d9 100644 --- a/ipc/gtest/TestSharedMemory.cpp +++ b/ipc/gtest/TestSharedMemory.cpp @@ -46,10 +46,11 @@ TEST(IPCSharedMemory, FreezeAndMapRW) ASSERT_FALSE(shm.memory()); // Re-create as writeable - auto handle = shm.TakeHandle(); + auto handle = base::SharedMemory::NULLHandle(); + ASSERT_TRUE(shm.GiveToProcess(base::GetCurrentProcId(), &handle)); ASSERT_TRUE(shm.IsHandleValid(handle)); ASSERT_FALSE(shm.IsValid()); - ASSERT_TRUE(shm.SetHandle(std::move(handle), /* read-only */ false)); + ASSERT_TRUE(shm.SetHandle(handle, /* read-only */ false)); ASSERT_TRUE(shm.IsValid()); // This should fail @@ -102,10 +103,11 @@ TEST(IPCSharedMemory, Reprotect) *mem = 'A'; // Re-create as read-only - auto handle = shm.TakeHandle(); + auto handle = base::SharedMemory::NULLHandle(); + ASSERT_TRUE(shm.GiveToProcess(base::GetCurrentProcId(), &handle)); ASSERT_TRUE(shm.IsHandleValid(handle)); ASSERT_FALSE(shm.IsValid()); - ASSERT_TRUE(shm.SetHandle(std::move(handle), /* read-only */ true)); + ASSERT_TRUE(shm.SetHandle(handle, /* read-only */ true)); ASSERT_TRUE(shm.IsValid()); // Re-map @@ -139,14 +141,14 @@ TEST(IPCSharedMemory, WinUnfreeze) ASSERT_FALSE(shm.memory()); // Extract handle. - auto handle = shm.TakeHandle(); + auto handle = base::SharedMemory::NULLHandle(); + ASSERT_TRUE(shm.GiveToProcess(base::GetCurrentProcId(), &handle)); ASSERT_TRUE(shm.IsHandleValid(handle)); ASSERT_FALSE(shm.IsValid()); // Unfreeze. - HANDLE newHandle = INVALID_HANDLE_VALUE; bool unfroze = ::DuplicateHandle( - GetCurrentProcess(), handle.release(), GetCurrentProcess(), &newHandle, + GetCurrentProcess(), handle, GetCurrentProcess(), &handle, FILE_MAP_ALL_ACCESS, false, DUPLICATE_CLOSE_SOURCE); ASSERT_FALSE(unfroze); } @@ -233,10 +235,11 @@ TEST(IPCSharedMemory, ROCopyAndMapRW) ASSERT_TRUE(shmRO.IsValid()); // Re-create as writeable - auto handle = shmRO.TakeHandle(); + auto handle = base::SharedMemory::NULLHandle(); + ASSERT_TRUE(shmRO.GiveToProcess(base::GetCurrentProcId(), &handle)); ASSERT_TRUE(shmRO.IsHandleValid(handle)); ASSERT_FALSE(shmRO.IsValid()); - ASSERT_TRUE(shmRO.SetHandle(std::move(handle), /* read-only */ false)); + ASSERT_TRUE(shmRO.SetHandle(handle, /* read-only */ false)); ASSERT_TRUE(shmRO.IsValid()); // This should fail diff --git a/ipc/ipdl/ipdl/ast.py b/ipc/ipdl/ipdl/ast.py index 2c3d964583ab..b100b07ab0a5 100644 --- a/ipc/ipdl/ipdl/ast.py +++ b/ipc/ipdl/ipdl/ast.py @@ -236,13 +236,8 @@ class UsingStmt(Node): def isRefcounted(self): return "RefCounted" in self.attributes - def isSendMoveOnly(self): - moveonly = self.attributes.get("MoveOnly") - return moveonly and moveonly.value in (None, "send") - - def isDataMoveOnly(self): - moveonly = self.attributes.get("MoveOnly") - return moveonly and moveonly.value in (None, "data") + def isMoveonly(self): + return "MoveOnly" in self.attributes # "singletons" diff --git a/ipc/ipdl/ipdl/lower.py b/ipc/ipdl/ipdl/lower.py index 5747f144a7c8..5bc316119ee4 100644 --- a/ipc/ipdl/ipdl/lower.py +++ b/ipc/ipdl/ipdl/lower.py @@ -269,6 +269,10 @@ def _refptr(T): return Type("RefPtr", T=T) +def _uniqueptr(T): + return Type("UniquePtr", T=T) + + def _alreadyaddrefed(T): return Type("already_AddRefed", T=T) @@ -296,8 +300,8 @@ def _makePromise(returns, side, resolver=False): def _resolveType(returns, side): if len(returns) > 1: - return _tuple([d.inType(side) for d in returns]) - return returns[0].inType(side) + return _tuple([d.moveType(side) for d in returns]) + return returns[0].moveType(side) def _makeResolver(returns, side): @@ -559,13 +563,16 @@ def _cxxConstRefType(ipdltype, side): if ipdltype.isIPDL() and ipdltype.isShmem(): t.ref = True return t + if ipdltype.isIPDL() and ipdltype.isByteBuf(): + t.ref = True + return t if ipdltype.isIPDL() and ipdltype.hasBaseType(): # Keep same constness as inner type. inner = _cxxConstRefType(ipdltype.basetype, side) t.const = inner.const or not inner.ref t.ref = True return t - if ipdltype.isCxx() and (ipdltype.isSendMoveOnly() or ipdltype.isDataMoveOnly()): + if ipdltype.isCxx() and ipdltype.isMoveonly(): t.const = True t.ref = True return t @@ -574,46 +581,41 @@ def _cxxConstRefType(ipdltype, side): t = t.T t.ptr = True return t + if ipdltype.isUniquePtr(): + t.ref = True + return t t.const = True t.ref = True return t -def _cxxTypeNeedsMoveForSend(ipdltype, context="root", visited=None): - """Returns `True` if serializing ipdltype requires a mutable reference, e.g. - because the underlying resource represented by the value is being - transferred to another process. This is occasionally distinct from whether - the C++ type exposes a copy constructor, such as for types which are not - cheaply copiable, but are not mutated when serialized.""" +def _cxxTypeCanMoveSend(ipdltype): + return ipdltype.isUniquePtr() - if visited is None: - visited = set() - visited.add(ipdltype) +def _cxxTypeNeedsMove(ipdltype): + if _cxxTypeNeedsMoveForSend(ipdltype): + return True + + if ipdltype.isIPDL(): + return ipdltype.isArray() + + return False + + +def _cxxTypeNeedsMoveForSend(ipdltype): + if ipdltype.isUniquePtr(): + return True if ipdltype.isCxx(): - return ipdltype.isSendMoveOnly() + return ipdltype.isMoveonly() if ipdltype.isIPDL(): if ipdltype.hasBaseType(): - return _cxxTypeNeedsMoveForSend(ipdltype.basetype, "wrapper", visited) - if ipdltype.isStruct() or ipdltype.isUnion(): - return any( - _cxxTypeNeedsMoveForSend(t, "compound", visited) - for t in ipdltype.itercomponents() - if t not in visited - ) - - # For historical reasons, shmem is `const_cast` to a mutable reference - # when being stored in a struct or union (see - # `_StructField.constRefExpr` and `_UnionMember.getConstValue`), meaning - # that they do not cause the containing struct to require move for - # sending. - if ipdltype.isShmem(): - return context != "compound" - + return _cxxTypeNeedsMove(ipdltype.basetype) return ( - ipdltype.isByteBuf() + ipdltype.isShmem() + or ipdltype.isByteBuf() or ipdltype.isEndpoint() or ipdltype.isManagedEndpoint() ) @@ -621,40 +623,28 @@ def _cxxTypeNeedsMoveForSend(ipdltype, context="root", visited=None): return False -def _cxxTypeNeedsMoveForData(ipdltype, context="root", visited=None): - """Returns `True` if the bare C++ type corresponding to ipdltype does not - satisfy std::is_copy_constructible_v. All C++ types supported by IPDL - must support std::is_move_constructible_v, so non-movable types must be - passed behind a `UniquePtr`.""" - +# FIXME Bug 1547019 This should be the same as _cxxTypeNeedsMoveForSend, but +# a lot of existing code needs to be updated and fixed before +# we can do that. +def _cxxTypeCanOnlyMove(ipdltype, visited=None): if visited is None: visited = set() visited.add(ipdltype) - if ipdltype.isUniquePtr(): - return True - if ipdltype.isCxx(): - return ipdltype.isDataMoveOnly() + return ipdltype.isMoveonly() if ipdltype.isIPDL(): - # When nested within a maybe or array, arrays are no longer copyable. - if context == "wrapper" and ipdltype.isArray(): - return True - if ipdltype.hasBaseType(): - return _cxxTypeNeedsMoveForData(ipdltype.basetype, "wrapper", visited) + if ipdltype.isMaybe() or ipdltype.isArray(): + return _cxxTypeCanOnlyMove(ipdltype.basetype, visited) if ipdltype.isStruct() or ipdltype.isUnion(): return any( - _cxxTypeNeedsMoveForData(t, "compound", visited) + _cxxTypeCanOnlyMove(t, visited) for t in ipdltype.itercomponents() if t not in visited ) - return ( - ipdltype.isByteBuf() - or ipdltype.isEndpoint() - or ipdltype.isManagedEndpoint() - ) + return ipdltype.isManagedEndpoint() return False @@ -663,6 +653,14 @@ def _cxxTypeCanMove(ipdltype): return not (ipdltype.isIPDL() and ipdltype.isActor()) +def _cxxMoveRefType(ipdltype, side): + t = _cxxBareType(ipdltype, side) + if _cxxTypeNeedsMove(ipdltype): + t.rvalref = True + return t + return _cxxConstRefType(ipdltype, side) + + def _cxxForceMoveRefType(ipdltype, side): assert _cxxTypeCanMove(ipdltype) t = _cxxBareType(ipdltype, side) @@ -691,23 +689,6 @@ def _cxxConstPtrToType(ipdltype, side): return t -def _cxxInType(ipdltype, side): - t = _cxxBareType(ipdltype, side) - if ipdltype.isIPDL() and ipdltype.isActor(): - return t - if _cxxTypeNeedsMoveForSend(ipdltype): - t.rvalref = True - return t - if ipdltype.isCxx() and ipdltype.isRefcounted(): - # Use T* instead of const RefPtr& - t = t.T - t.ptr = True - return t - t.const = True - t.ref = True - return t - - def _allocMethod(ptype, side): return "Alloc" + ptype.name() + side.title() @@ -748,6 +729,10 @@ class _HybridDecl: """Return this decl's C++ type as a const, 'reference' type.""" return _cxxConstRefType(self.ipdltype, side) + def rvalueRefType(self, side): + """Return this decl's C++ type as an r-value 'reference' type.""" + return _cxxMoveRefType(self.ipdltype, side) + def ptrToType(self, side): return _cxxPtrToType(self.ipdltype, side) @@ -755,8 +740,18 @@ class _HybridDecl: return _cxxConstPtrToType(self.ipdltype, side) def inType(self, side): - """Return this decl's C++ Type with sending inparam semantics.""" - return _cxxInType(self.ipdltype, side) + """Return this decl's C++ Type with inparam semantics.""" + if self.ipdltype.isIPDL() and self.ipdltype.isActor(): + return self.bareType(side) + elif _cxxTypeNeedsMoveForSend(self.ipdltype): + return self.rvalueRefType(side) + return self.constRefType(side) + + def moveType(self, side): + """Return this decl's C++ Type with move semantics.""" + if self.ipdltype.isIPDL() and self.ipdltype.isActor(): + return self.bareType(side) + return self.rvalueRefType(side) def outType(self, side): """Return this decl's C++ Type with outparam semantics.""" @@ -857,6 +852,10 @@ class _StructField(_CompoundTypeComponent): refexpr = self.refExpr(thisexpr) if "Shmem" == self.ipdltype.name(): refexpr = ExprCast(refexpr, Type("Shmem", ref=True), const=True) + if "ByteBuf" == self.ipdltype.name(): + refexpr = ExprCast(refexpr, Type("ByteBuf", ref=True), const=True) + if "FileDescriptor" == self.ipdltype.name(): + refexpr = ExprCast(refexpr, Type("FileDescriptor", ref=True), const=True) return refexpr def argVar(self): @@ -1030,8 +1029,12 @@ class _UnionMember(_CompoundTypeComponent): def getConstValue(self): v = ExprDeref(self.callGetConstPtr()) # sigh + if "ByteBuf" == self.ipdltype.name(): + v = ExprCast(v, Type("ByteBuf", ref=True), const=True) if "Shmem" == self.ipdltype.name(): v = ExprCast(v, Type("Shmem", ref=True), const=True) + if "FileDescriptor" == self.ipdltype.name(): + v = ExprCast(v, Type("FileDescriptor", ref=True), const=True) return v @@ -1124,23 +1127,9 @@ class MessageDecl(ipdl.ast.MessageDecl): return Decl(Type("Tainted", T=d.bareType(side)), d.name) if sems == "in": - t = d.inType(side) - # If this is the `recv` side, and we're not using "move" - # semantics, that means we're an alloc method, and cannot accept - # values by rvalue reference. Downgrade to an lvalue reference. - if direction == "recv" and t.rvalref: - t.rvalref = False - t.ref = True - return Decl(t, d.name) - elif sems == "move": - assert direction == "recv" - # For legacy reasons, use an rvalue reference when generating - # parameters for recv methods which accept arrays. - if d.ipdltype.isIPDL() and d.ipdltype.isArray(): - t = d.bareType(side) - t.rvalref = True - return Decl(t, d.name) return Decl(d.inType(side), d.name) + elif sems == "move": + return Decl(d.moveType(side), d.name) elif sems == "out": return Decl(d.outType(side), d.name) else: @@ -2021,6 +2010,8 @@ class _ParamTraits: @classmethod def write(cls, var, msgvar, actor, ipdltype=None): + # WARNING: This doesn't set AutoForActor for you, make sure this is + # only called when the actor is already correctly set. if ipdltype and _cxxTypeNeedsMoveForSend(ipdltype): var = ExprMove(var) return ExprCall(ExprVar("WriteIPDLParam"), args=[msgvar, actor, var]) @@ -2167,7 +2158,7 @@ class _ParamTraits: ) @classmethod - def generateDecl(cls, fortype, write, read, needsmove=False): + def generateDecl(cls, fortype, write, read, constin=True): # IPDLParamTraits impls are selected ignoring constness, and references. pt = Class( "IPDLParamTraits", @@ -2183,10 +2174,7 @@ class _ParamTraits: iprotocoltype = Type("mozilla::ipc::IProtocol", ptr=True) # static void Write(Message*, const T&); - if needsmove: - intype = Type("paramType", rvalref=True) - else: - intype = Type("paramType", ref=True, const=True) + intype = Type("paramType", ref=True, const=constin) writemthd = MethodDefn( MethodDecl( "Write", @@ -2343,9 +2331,7 @@ class _ParamTraits: read.append(StmtReturn.TRUE) - return cls.generateDecl( - cxxtype, write, read, needsmove=_cxxTypeNeedsMoveForSend(structtype) - ) + return cls.generateDecl(cxxtype, write, read) @classmethod def unionPickling(cls, uniontype): @@ -2445,9 +2431,7 @@ class _ParamTraits: StmtBlock([cls.fatalError("unknown union type"), StmtReturn.FALSE]), ) - return cls.generateDecl( - cxxtype, write, read, needsmove=_cxxTypeNeedsMoveForSend(uniontype) - ) + return cls.generateDecl(cxxtype, write, read) # -------------------------------------------------- @@ -2621,11 +2605,12 @@ def _generateCxxStruct(sd): constreftype = Type(sd.name, const=True, ref=True) def fieldsAsParamList(): + # FIXME Bug 1547019 inType() should do the right thing once + # _cxxTypeCanOnlyMove is replaced with + # _cxxTypeNeedsMoveForSend return [ Decl( - f.forceMoveType() - if _cxxTypeNeedsMoveForData(f.ipdltype) - else f.constRefType(), + f.forceMoveType() if _cxxTypeCanOnlyMove(f.ipdltype) else f.inType(), f.argVar().name, ) for f in sd.fields_ipdl_order() @@ -2657,7 +2642,7 @@ def _generateCxxStruct(sd): valctor.memberinits = [] for f in sd.fields_member_order(): arg = f.argVar() - if _cxxTypeNeedsMoveForData(f.ipdltype): + if _cxxTypeCanOnlyMove(f.ipdltype): arg = ExprMove(arg) valctor.memberinits.append(ExprMemberInit(f.memberVar(), args=[arg])) @@ -2968,9 +2953,9 @@ def _generateCxxUnion(ud): # Union(const T&) copy & Union(T&&) move ctors othervar = ExprVar("aOther") for c in ud.components: - if not _cxxTypeNeedsMoveForData(c.ipdltype): + if not _cxxTypeCanOnlyMove(c.ipdltype): copyctor = ConstructorDefn( - ConstructorDecl(ud.name, params=[Decl(c.constRefType(), othervar.name)]) + ConstructorDecl(ud.name, params=[Decl(c.inType(), othervar.name)]) ) copyctor.addstmts( [ @@ -2980,7 +2965,7 @@ def _generateCxxUnion(ud): ) cls.addstmts([copyctor, Whitespace.NL]) - if not _cxxTypeCanMove(c.ipdltype): + if not _cxxTypeCanMove(c.ipdltype) or _cxxTypeNeedsMoveForSend(c.ipdltype): continue movector = ConstructorDefn( ConstructorDecl(ud.name, params=[Decl(c.forceMoveType(), othervar.name)]) @@ -2993,7 +2978,7 @@ def _generateCxxUnion(ud): ) cls.addstmts([movector, Whitespace.NL]) - unionNeedsMove = any(_cxxTypeNeedsMoveForData(c.ipdltype) for c in ud.components) + unionNeedsMove = any(_cxxTypeCanOnlyMove(c.ipdltype) for c in ud.components) # Union(const Union&) copy ctor if not unionNeedsMove: @@ -3110,13 +3095,11 @@ def _generateCxxUnion(ud): # Union& operator= methods rhsvar = ExprVar("aRhs") for c in ud.components: - if not _cxxTypeNeedsMoveForData(c.ipdltype): + if not _cxxTypeCanOnlyMove(c.ipdltype): # Union& operator=(const T&) opeq = MethodDefn( MethodDecl( - "operator=", - params=[Decl(c.constRefType(), rhsvar.name)], - ret=refClsType, + "operator=", params=[Decl(c.inType(), rhsvar.name)], ret=refClsType ) ) opeq.addstmts( @@ -3131,7 +3114,7 @@ def _generateCxxUnion(ud): cls.addstmts([opeq, Whitespace.NL]) # Union& operator=(T&&) - if not _cxxTypeCanMove(c.ipdltype): + if not _cxxTypeCanMove(c.ipdltype) or _cxxTypeNeedsMoveForSend(c.ipdltype): continue opeq = MethodDefn( @@ -3272,7 +3255,7 @@ def _generateCxxUnion(ud): opeqeq = MethodDefn( MethodDecl( "operator==", - params=[Decl(c.constRefType(), rhsvar.name)], + params=[Decl(c.inType(), rhsvar.name)], ret=Type.BOOL, const=True, ) @@ -4747,11 +4730,7 @@ class _GenerateProtocolActorCode(ipdl.ast.Visitor): helper.addstmts( [ self.callAllocActor(md, retsems="out", side=self.side), - StmtReturn( - ExprCall( - ExprVar(helperdecl.name), args=md.makeCxxArgs(paramsems="move") - ) - ), + StmtReturn(ExprCall(ExprVar(helperdecl.name), args=md.makeCxxArgs())), ] ) return helper diff --git a/ipc/ipdl/ipdl/type.py b/ipc/ipdl/ipdl/type.py index b1dc5e8665a9..426721c9fc60 100644 --- a/ipc/ipdl/ipdl/type.py +++ b/ipc/ipdl/ipdl/type.py @@ -163,13 +163,12 @@ VOID = VoidType() class ImportedCxxType(Type): - def __init__(self, qname, refcounted, sendmoveonly, datamoveonly): + def __init__(self, qname, refcounted, moveonly): assert isinstance(qname, QualifiedId) self.loc = qname.loc self.qname = qname self.refcounted = refcounted - self.sendmoveonly = sendmoveonly - self.datamoveonly = datamoveonly + self.moveonly = moveonly def isCxx(self): return True @@ -180,11 +179,8 @@ class ImportedCxxType(Type): def isRefcounted(self): return self.refcounted - def isSendMoveOnly(self): - return self.sendmoveonly - - def isDataMoveOnly(self): - return self.datamoveonly + def isMoveonly(self): + return self.moveonly def name(self): return self.qname.baseid @@ -1111,7 +1107,7 @@ class GatherDecls(TcheckVisitor): self.checkAttributes( using.attributes, { - "MoveOnly": (None, "data", "send"), + "MoveOnly": None, "RefCounted": None, }, ) @@ -1124,10 +1120,7 @@ class GatherDecls(TcheckVisitor): ipdltype = FDType(using.type.spec) else: ipdltype = ImportedCxxType( - using.type.spec, - using.isRefcounted(), - using.isSendMoveOnly(), - using.isDataMoveOnly(), + using.type.spec, using.isRefcounted(), using.isMoveonly() ) existingType = self.symtab.lookup(ipdltype.fullname()) if existingType and existingType.fullname == ipdltype.fullname(): @@ -1137,10 +1130,7 @@ class GatherDecls(TcheckVisitor): "inconsistent refcounted status of type `%s`", str(using.type), ) - if ( - ipdltype.isSendMoveOnly() != existingType.type.isSendMoveOnly() - or ipdltype.isDataMoveOnly() != existingType.type.isDataMoveOnly() - ): + if ipdltype.isMoveonly() != existingType.type.isMoveonly(): self.error( using.loc, "inconsistent moveonly status of type `%s`", diff --git a/ipc/ipdl/test/ipdl/error/PInconsistentMoveOnly.ipdl b/ipc/ipdl/test/ipdl/error/PInconsistentMoveOnly.ipdl index 0c429221dc71..bb50411e78ad 100644 --- a/ipc/ipdl/test/ipdl/error/PInconsistentMoveOnly.ipdl +++ b/ipc/ipdl/test/ipdl/error/PInconsistentMoveOnly.ipdl @@ -1,12 +1,8 @@ //error: inconsistent moveonly status of type `mozilla::ipc::SomeMoveonlyType` -//error: inconsistent moveonly status of type `mozilla::ipc::SomeMoveonlySendType` [MoveOnly] using class mozilla::ipc::SomeMoveonlyType from "SomeFile.h"; using class mozilla::ipc::SomeMoveonlyType from "SomeFile.h"; -[MoveOnly=send] using class mozilla::ipc::SomeMoveonlySendType from "SomeFile.h"; -[MoveOnly=data] using class mozilla::ipc::SomeMoveonlySendType from "SomeFile.h"; - protocol PInconsistentMoveOnly { child: async SomeMessage(); diff --git a/ipc/ipdl/test/ipdl/ok/PbasicUsing.ipdl b/ipc/ipdl/test/ipdl/ok/PbasicUsing.ipdl index ac4df3a258f9..df2d165718be 100644 --- a/ipc/ipdl/test/ipdl/ok/PbasicUsing.ipdl +++ b/ipc/ipdl/test/ipdl/ok/PbasicUsing.ipdl @@ -14,14 +14,6 @@ using struct SomeStruct from "SomeFile.h"; [RefCounted, MoveOnly] using class SomeRefcountedMoveonlyClass from "SomeFile.h"; [RefCounted, MoveOnly] using struct SomeRefcountedMoveonlyStruct from "SomeFile.h"; -[MoveOnly=data] using SomeMoveonlyDataType from "SomeFile.h"; -[MoveOnly=data] using class SomeMoveonlyDataClass from "SomeFile.h"; -[MoveOnly=data] using struct SomeMoveonlyDataStruct from "SomeFile.h"; - -[MoveOnly=send] using SomeMoveonlySendType from "SomeFile.h"; -[MoveOnly=send] using class SomeMoveonlySendClass from "SomeFile.h"; -[MoveOnly=send] using struct SomeMoveonlySendStruct from "SomeFile.h"; - union SomeUnion { SomeType; @@ -36,12 +28,6 @@ union SomeUnion SomeRefcountedMoveonlyType; SomeRefcountedMoveonlyClass; SomeRefcountedMoveonlyStruct; - SomeMoveonlyDataType; - SomeMoveonlyDataClass; - SomeMoveonlyDataStruct; - SomeMoveonlySendType; - SomeMoveonlySendClass; - SomeMoveonlySendStruct; }; protocol PbasicUsing { diff --git a/js/xpconnect/src/XPCSelfHostedShmem.cpp b/js/xpconnect/src/XPCSelfHostedShmem.cpp index 6464746ea366..3eb0a95d343a 100644 --- a/js/xpconnect/src/XPCSelfHostedShmem.cpp +++ b/js/xpconnect/src/XPCSelfHostedShmem.cpp @@ -71,7 +71,7 @@ bool xpc::SelfHostedShmem::InitFromChild(::base::SharedMemoryHandle aHandle, MOZ_ASSERT(!mLen, "Shouldn't call this more than once"); auto shm = mozilla::MakeUnique(); - if (NS_WARN_IF(!shm->SetHandle(std::move(aHandle), /* read_only */ true))) { + if (NS_WARN_IF(!shm->SetHandle(aHandle, /* read_only */ true))) { return false; } diff --git a/layout/style/GlobalStyleSheetCache.cpp b/layout/style/GlobalStyleSheetCache.cpp index 4cfb8deb1ba8..ae3197d02452 100644 --- a/layout/style/GlobalStyleSheetCache.cpp +++ b/layout/style/GlobalStyleSheetCache.cpp @@ -670,13 +670,13 @@ bool GlobalStyleSheetCache::AffectedByPref(const nsACString& aPref) { } /* static */ void GlobalStyleSheetCache::SetSharedMemory( - base::SharedMemoryHandle aHandle, uintptr_t aAddress) { + const base::SharedMemoryHandle& aHandle, uintptr_t aAddress) { MOZ_ASSERT(!XRE_IsParentProcess()); MOZ_ASSERT(!gStyleCache, "Too late, GlobalStyleSheetCache already created!"); MOZ_ASSERT(!sSharedMemory, "Shouldn't call this more than once"); auto shm = MakeUnique(); - if (!shm->SetHandle(std::move(aHandle), /* read_only */ true)) { + if (!shm->SetHandle(aHandle, /* read_only */ true)) { return; } @@ -688,10 +688,7 @@ bool GlobalStyleSheetCache::AffectedByPref(const nsACString& aPref) { bool GlobalStyleSheetCache::ShareToProcess(base::ProcessId aProcessId, base::SharedMemoryHandle* aHandle) { MOZ_ASSERT(XRE_IsParentProcess()); - if (sSharedMemory) { - *aHandle = sSharedMemory->CloneHandle(); - } - return !!*aHandle; + return sSharedMemory && sSharedMemory->ShareToProcess(aProcessId, aHandle); } StaticRefPtr GlobalStyleSheetCache::gStyleCache; diff --git a/layout/style/GlobalStyleSheetCache.h b/layout/style/GlobalStyleSheetCache.h index 7dff7fbaf0da..a1eeaa797e69 100644 --- a/layout/style/GlobalStyleSheetCache.h +++ b/layout/style/GlobalStyleSheetCache.h @@ -15,7 +15,6 @@ #include "mozilla/PreferenceSheet.h" #include "mozilla/NotNull.h" #include "mozilla/StaticPtr.h" -#include "mozilla/UserAgentStyleSheetID.h" #include "mozilla/css/Loader.h" class nsIFile; @@ -65,7 +64,7 @@ class GlobalStyleSheetCache final : public nsIObserver, // Called early on in a content process' life from // ContentChild::InitSharedUASheets, before the GlobalStyleSheetCache // singleton has been created. - static void SetSharedMemory(base::SharedMemoryHandle aHandle, + static void SetSharedMemory(const base::SharedMemoryHandle& aHandle, uintptr_t aAddress); // Obtain a shared memory handle for the shared UA sheets to pass into a diff --git a/tools/fuzzing/faulty/Faulty.cpp b/tools/fuzzing/faulty/Faulty.cpp index f72fb23c08d8..93d061aeb82b 100644 --- a/tools/fuzzing/faulty/Faulty.cpp +++ b/tools/fuzzing/faulty/Faulty.cpp @@ -22,6 +22,7 @@ #include "FuzzingTraits.h" #include "chrome/common/ipc_channel.h" #include "chrome/common/ipc_message.h" +#include "chrome/common/file_descriptor_set_posix.h" #include "mozilla/ipc/Faulty.h" #include "nsComponentManagerUtils.h" #include "nsNetCID.h" @@ -658,7 +659,14 @@ std::vector Faulty::GetDataFromIPCMessage(IPC::Message* aMsg) { // static void Faulty::CopyFDs(IPC::Message* aDstMsg, IPC::Message* aSrcMsg) { - std::swap(aDstMsg->attached_handles_, aSrcMsg->attached_handles_); +#ifndef _WINDOWS + FileDescriptorSet* dstFdSet = aDstMsg->file_descriptor_set(); + FileDescriptorSet* srcFdSet = aSrcMsg->file_descriptor_set(); + for (size_t i = 0; i < srcFdSet->size(); i++) { + int fd = srcFdSet->GetDescriptorAt(i); + dstFdSet->Add(fd); + } +#endif } UniquePtr Faulty::MutateIPCMessage(const char* aChannel, diff --git a/widget/windows/PCompositorWidget.ipdl b/widget/windows/PCompositorWidget.ipdl index c56cd94a1fdf..a2d8db5896f2 100644 --- a/widget/windows/PCompositorWidget.ipdl +++ b/widget/windows/PCompositorWidget.ipdl @@ -16,9 +16,9 @@ namespace mozilla { namespace widget { struct RemoteBackbufferHandles { - FileDescriptor fileMapping; - FileDescriptor requestReadyEvent; - FileDescriptor responseReadyEvent; + WindowsHandle fileMapping; + WindowsHandle requestReadyEvent; + WindowsHandle responseReadyEvent; }; sync protocol PCompositorWidget diff --git a/widget/windows/RemoteBackbuffer.cpp b/widget/windows/RemoteBackbuffer.cpp index ef44954af92c..7d79fada76d6 100644 --- a/widget/windows/RemoteBackbuffer.cpp +++ b/widget/windows/RemoteBackbuffer.cpp @@ -149,13 +149,12 @@ class SharedImage { 0 /*offset*/); } - HANDLE CreateRemoteFileMapping(HANDLE aTargetProcess) { - MOZ_ASSERT(aTargetProcess); + HANDLE CreateRemoteFileMapping(DWORD aTargetProcessId) { + MOZ_ASSERT(aTargetProcessId); HANDLE fileMapping = nullptr; - if (!::DuplicateHandle(GetCurrentProcess(), mFileMapping, aTargetProcess, - &fileMapping, 0 /*desiredAccess*/, - FALSE /*inheritHandle*/, DUPLICATE_SAME_ACCESS)) { + if (!ipc::DuplicateHandle(mFileMapping, aTargetProcessId, &fileMapping, + 0 /*desiredAccess*/, DUPLICATE_SAME_ACCESS)) { return nullptr; } return fileMapping; @@ -315,8 +314,8 @@ class PresentableSharedImage { return result; } - HANDLE CreateRemoteFileMapping(HANDLE aTargetProcess) { - return mSharedImage.CreateRemoteFileMapping(aTargetProcess); + HANDLE CreateRemoteFileMapping(DWORD aTargetProcessId) { + return mSharedImage.CreateRemoteFileMapping(aTargetProcessId); } already_AddRefed CreateDrawTarget() { @@ -345,7 +344,7 @@ class PresentableSharedImage { Provider::Provider() : mWindowHandle(nullptr), - mTargetProcess(nullptr), + mTargetProcessId(0), mFileMapping(nullptr), mRequestReadyEvent(nullptr), mResponseReadyEvent(nullptr), @@ -378,10 +377,6 @@ Provider::~Provider() { if (mFileMapping) { MOZ_ALWAYS_TRUE(::CloseHandle(mFileMapping)); } - - if (mTargetProcess) { - MOZ_ALWAYS_TRUE(::CloseHandle(mTargetProcess)); - } } bool Provider::Initialize(HWND aWindowHandle, DWORD aTargetProcessId, @@ -390,12 +385,7 @@ bool Provider::Initialize(HWND aWindowHandle, DWORD aTargetProcessId, MOZ_ASSERT(aTargetProcessId); mWindowHandle = aWindowHandle; - - mTargetProcess = ::OpenProcess(PROCESS_DUP_HANDLE, FALSE /*inheritHandle*/, - aTargetProcessId); - if (!mTargetProcess) { - return false; - } + mTargetProcessId = aTargetProcessId; mFileMapping = ::CreateFileMappingW( INVALID_HANDLE_VALUE, nullptr /*secattr*/, PAGE_READWRITE, 0 /*sizeHigh*/, @@ -447,10 +437,30 @@ bool Provider::Initialize(HWND aWindowHandle, DWORD aTargetProcessId, } Maybe Provider::CreateRemoteHandles() { - return Some( - RemoteBackbufferHandles(ipc::FileDescriptor(mFileMapping), - ipc::FileDescriptor(mRequestReadyEvent), - ipc::FileDescriptor(mResponseReadyEvent))); + HANDLE fileMapping = nullptr; + if (!ipc::DuplicateHandle(mFileMapping, mTargetProcessId, &fileMapping, + 0 /*desiredAccess*/, DUPLICATE_SAME_ACCESS)) { + return Nothing(); + } + + HANDLE requestReadyEvent = nullptr; + if (!ipc::DuplicateHandle(mRequestReadyEvent, mTargetProcessId, + &requestReadyEvent, 0 /*desiredAccess*/, + DUPLICATE_SAME_ACCESS)) { + return Nothing(); + } + + HANDLE responseReadyEvent = nullptr; + if (!ipc::DuplicateHandle(mResponseReadyEvent, mTargetProcessId, + &responseReadyEvent, 0 /*desiredAccess*/, + DUPLICATE_SAME_ACCESS)) { + return Nothing(); + } + + return Some(RemoteBackbufferHandles( + reinterpret_cast(fileMapping), + reinterpret_cast(requestReadyEvent), + reinterpret_cast(responseReadyEvent))); } void Provider::UpdateTransparencyMode(nsTransparencyMode aTransparencyMode) { @@ -542,7 +552,7 @@ void Provider::HandleBorrowRequest(BorrowResponseData* aResponseData, } HANDLE remoteFileMapping = - newBackbuffer->CreateRemoteFileMapping(mTargetProcess); + newBackbuffer->CreateRemoteFileMapping(mTargetProcessId); if (!remoteFileMapping) { return; } @@ -604,22 +614,15 @@ Client::~Client() { } bool Client::Initialize(const RemoteBackbufferHandles& aRemoteHandles) { - MOZ_ASSERT(aRemoteHandles.fileMapping().IsValid()); - MOZ_ASSERT(aRemoteHandles.requestReadyEvent().IsValid()); - MOZ_ASSERT(aRemoteHandles.responseReadyEvent().IsValid()); + MOZ_ASSERT(aRemoteHandles.fileMapping()); + MOZ_ASSERT(aRemoteHandles.requestReadyEvent()); + MOZ_ASSERT(aRemoteHandles.responseReadyEvent()); - // FIXME: Due to PCompositorWidget using virtual Recv methods, - // RemoteBackbufferHandles is passed by const reference, and cannot have its - // signature customized, meaning that we need to clone the handles here. - // - // Once PCompositorWidget is migrated to use direct call semantics, the - // signature can be changed to accept RemoteBackbufferHandles by rvalue - // reference or value, and the DuplicateHandle calls here can be avoided. - mFileMapping = aRemoteHandles.fileMapping().ClonePlatformHandle().release(); + mFileMapping = reinterpret_cast(aRemoteHandles.fileMapping()); mRequestReadyEvent = - aRemoteHandles.requestReadyEvent().ClonePlatformHandle().release(); + reinterpret_cast(aRemoteHandles.requestReadyEvent()); mResponseReadyEvent = - aRemoteHandles.responseReadyEvent().ClonePlatformHandle().release(); + reinterpret_cast(aRemoteHandles.responseReadyEvent()); void* mappedFilePtr = ::MapViewOfFile(mFileMapping, FILE_MAP_ALL_ACCESS, 0 /*offsetHigh*/, diff --git a/widget/windows/RemoteBackbuffer.h b/widget/windows/RemoteBackbuffer.h index 090c42fbeed5..14fe0e9d68b1 100644 --- a/widget/windows/RemoteBackbuffer.h +++ b/widget/windows/RemoteBackbuffer.h @@ -50,7 +50,7 @@ class Provider { PresentResponseData* aResponseData); HWND mWindowHandle; - HANDLE mTargetProcess; + DWORD mTargetProcessId; HANDLE mFileMapping; HANDLE mRequestReadyEvent; HANDLE mResponseReadyEvent;