Skip to content

Commit

Permalink
Bug 1779792 - Part 5: Add a unique nsID field to each MessageChannel …
Browse files Browse the repository at this point in the history
…pair, r=ipc-reviewers,mccr8

This won't be used for any security or routing purposes, but can be useful for
debugging. It will be used in the future by the profiler to correlate sent and
received message events across processes.

Differential Revision: https://phabricator.services.mozilla.com/D153621
  • Loading branch information
mystor committed Aug 10, 2022
1 parent 45397cb commit 96d76af
Show file tree
Hide file tree
Showing 15 changed files with 127 additions and 51 deletions.
20 changes: 20 additions & 0 deletions ipc/glue/Endpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,3 +145,23 @@ bool IPDLParamTraits<UntypedManagedEndpoint>::Read(IPC::MessageReader* aReader,
}

} // namespace mozilla::ipc

namespace IPC {

void ParamTraits<mozilla::ipc::UntypedEndpoint>::Write(MessageWriter* aWriter,
paramType&& aParam) {
IPC::WriteParam(aWriter, std::move(aParam.mPort));
IPC::WriteParam(aWriter, aParam.mMessageChannelId);
IPC::WriteParam(aWriter, aParam.mMyPid);
IPC::WriteParam(aWriter, aParam.mOtherPid);
}

bool ParamTraits<mozilla::ipc::UntypedEndpoint>::Read(MessageReader* aReader,
paramType* aResult) {
return IPC::ReadParam(aReader, &aResult->mPort) &&
IPC::ReadParam(aReader, &aResult->mMessageChannelId) &&
IPC::ReadParam(aReader, &aResult->mMyPid) &&
IPC::ReadParam(aReader, &aResult->mOtherPid);
}

} // namespace IPC
27 changes: 19 additions & 8 deletions ipc/glue/Endpoint.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,13 @@ class UntypedEndpoint {
UntypedEndpoint() = default;

UntypedEndpoint(const PrivateIPDLInterface&, ScopedPort aPort,
const nsID& aMessageChannelId,
ProcessId aMyPid = base::kInvalidProcessId,
ProcessId aOtherPid = base::kInvalidProcessId)
: mPort(std::move(aPort)), mMyPid(aMyPid), mOtherPid(aOtherPid) {}
: mPort(std::move(aPort)),
mMessageChannelId(aMessageChannelId),
mMyPid(aMyPid),
mOtherPid(aOtherPid) {}

UntypedEndpoint(const UntypedEndpoint&) = delete;
UntypedEndpoint(UntypedEndpoint&& aOther) = default;
Expand All @@ -74,7 +78,8 @@ class UntypedEndpoint {
MOZ_RELEASE_ASSERT(mMyPid == base::kInvalidProcessId ||
mMyPid == base::GetCurrentProcId());
MOZ_RELEASE_ASSERT(!aEventTarget || aEventTarget->IsOnCurrentThread());
return aActor->Open(std::move(mPort), mOtherPid, aEventTarget);
return aActor->Open(std::move(mPort), mMessageChannelId, mOtherPid,
aEventTarget);
}

bool IsValid() const { return mPort.IsValid(); }
Expand All @@ -83,6 +88,7 @@ class UntypedEndpoint {
friend struct IPC::ParamTraits<UntypedEndpoint>;

ScopedPort mPort;
nsID mMessageChannelId{};
ProcessId mMyPid = base::kInvalidProcessId;
ProcessId mOtherPid = base::kInvalidProcessId;
};
Expand Down Expand Up @@ -157,8 +163,11 @@ nsresult CreateEndpoints(const PrivateIPDLInterface& aPrivate,

auto [parentPort, childPort] =
NodeController::GetSingleton()->CreatePortPair();
*aParentEndpoint = Endpoint<PFooParent>(aPrivate, std::move(parentPort));
*aChildEndpoint = Endpoint<PFooChild>(aPrivate, std::move(childPort));
nsID channelId = nsID::GenerateUUID();
*aParentEndpoint =
Endpoint<PFooParent>(aPrivate, std::move(parentPort), channelId);
*aChildEndpoint =
Endpoint<PFooChild>(aPrivate, std::move(childPort), channelId);
return NS_OK;
}

Expand All @@ -173,10 +182,12 @@ nsresult CreateEndpoints(const PrivateIPDLInterface& aPrivate,

auto [parentPort, childPort] =
NodeController::GetSingleton()->CreatePortPair();
*aParentEndpoint = Endpoint<PFooParent>(aPrivate, std::move(parentPort),
aParentDestPid, aChildDestPid);
*aChildEndpoint = Endpoint<PFooChild>(aPrivate, std::move(childPort),
aChildDestPid, aParentDestPid);
nsID channelId = nsID::GenerateUUID();
*aParentEndpoint =
Endpoint<PFooParent>(aPrivate, std::move(parentPort), channelId,
aParentDestPid, aChildDestPid);
*aChildEndpoint = Endpoint<PFooChild>(
aPrivate, std::move(childPort), channelId, aChildDestPid, aParentDestPid);
return NS_OK;
}

Expand Down
8 changes: 8 additions & 0 deletions ipc/glue/GeckoChildProcessHost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ class BaseProcessLauncher {
mTmpDirName(aHost->mTmpDirName),
mChildId(++gChildCounter) {
SprintfLiteral(mPidString, "%" PRIPID, base::GetCurrentProcId());
aHost->mInitialChannelId.ToProvidedString(mInitialChannelIdString);

// Compute the serial event target we'll use for launching.
nsCOMPtr<nsIEventTarget> threadOrPool = GetIPCLauncher();
Expand Down Expand Up @@ -237,6 +238,7 @@ class BaseProcessLauncher {
int32_t mChildId;
TimeStamp mStartTimeStamp = TimeStamp::Now();
char mPidString[32];
char mInitialChannelIdString[NSID_LENGTH];

// Set during launch.
IPC::Channel::ChannelId mChannelId;
Expand Down Expand Up @@ -386,6 +388,7 @@ GeckoChildProcessHost::GeckoChildProcessHost(GeckoProcessType aProcessType,
mIsFileContent(aIsFileContent),
mMonitor("mozilla.ipc.GeckChildProcessHost.mMonitor"),
mLaunchOptions(MakeUnique<base::LaunchOptions>()),
mInitialChannelId(nsID::GenerateUUID()),
mProcessState(CREATING_CHANNEL),
#ifdef XP_WIN
mGroupId(u"-"),
Expand Down Expand Up @@ -1209,6 +1212,8 @@ bool PosixProcessLauncher::DoSetup() {
# endif
}

mChildArgv.push_back(mInitialChannelIdString);

mChildArgv.push_back(mPidString);

if (!CrashReporter::IsDummy()) {
Expand Down Expand Up @@ -1488,6 +1493,9 @@ bool WindowsProcessLauncher::DoSetup() {
// Win app model id
mCmdLine->AppendLooseValue(mGroupId.get());

// Initial MessageChannel id
mCmdLine->AppendLooseValue(UTF8ToWide(mInitialChannelIdString));

// Process id
mCmdLine->AppendLooseValue(UTF8ToWide(mPidString));

Expand Down
3 changes: 2 additions & 1 deletion ipc/glue/GeckoChildProcessHost.h
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ class GeckoChildProcessHost : public ChildProcessHost,

UntypedEndpoint TakeInitialEndpoint() {
return UntypedEndpoint{PrivateIPDLInterface{}, std::move(mInitialPort),
base::GetCurrentProcId(),
mInitialChannelId, base::GetCurrentProcId(),
base::GetProcId(mChildProcessHandle)};
}

Expand Down Expand Up @@ -203,6 +203,7 @@ class GeckoChildProcessHost : public ChildProcessHost,
// is set to null to free the options after the child is launched.
UniquePtr<base::LaunchOptions> mLaunchOptions;
ScopedPort mInitialPort;
nsID mInitialChannelId;
RefPtr<NodeController> mNodeController;
RefPtr<NodeChannel> mNodeChannel;

Expand Down
14 changes: 10 additions & 4 deletions ipc/glue/MessageChannel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,7 @@ void MessageChannel::Clear() {
}

bool MessageChannel::Open(ScopedPort aPort, Side aSide,
const nsID& aMessageChannelId,
nsISerialEventTarget* aEventTarget) {
nsCOMPtr<nsISerialEventTarget> eventTarget =
aEventTarget ? aEventTarget : GetCurrentSerialEventTarget();
Expand Down Expand Up @@ -669,6 +670,7 @@ bool MessageChannel::Open(ScopedPort aPort, Side aSide,
MOZ_RELEASE_ASSERT(ChannelClosed == mChannelState, "Not currently closed");
MOZ_ASSERT(mSide == UnknownSide);

mMessageChannelId = aMessageChannelId;
mWorkerThread = eventTarget;
mShutdownTask = shutdownTask;
mLink = MakeUnique<PortLink>(this, std::move(aPort));
Expand Down Expand Up @@ -706,6 +708,8 @@ bool MessageChannel::Open(MessageChannel* aTargetChan,

MOZ_ASSERT(aTargetChan, "Need a target channel");

nsID channelId = nsID::GenerateUUID();

std::pair<ScopedPort, ScopedPort> ports =
NodeController::GetSingleton()->CreatePortPair();

Expand All @@ -717,28 +721,30 @@ bool MessageChannel::Open(MessageChannel* aTargetChan,
/* initially_signaled */ false);
MOZ_ALWAYS_SUCCEEDS(aEventTarget->Dispatch(NS_NewCancelableRunnableFunction(
"ipc::MessageChannel::OpenAsOtherThread", [&]() {
aTargetChan->Open(std::move(ports.second), GetOppSide(aSide),
aTargetChan->Open(std::move(ports.second), GetOppSide(aSide), channelId,
aEventTarget);
event.Signal();
})));
bool ok = event.Wait();
MOZ_RELEASE_ASSERT(ok);

// Now that the other side has connected, open the port on our side.
return Open(std::move(ports.first), aSide);
return Open(std::move(ports.first), aSide, channelId);
}

bool MessageChannel::OpenOnSameThread(MessageChannel* aTargetChan,
mozilla::ipc::Side aSide) {
auto [porta, portb] = NodeController::GetSingleton()->CreatePortPair();

nsID channelId = nsID::GenerateUUID();

aTargetChan->mIsSameThreadChannel = true;
mIsSameThreadChannel = true;

auto* currentThread = GetCurrentSerialEventTarget();
return aTargetChan->Open(std::move(portb), GetOppSide(aSide),
return aTargetChan->Open(std::move(portb), GetOppSide(aSide), channelId,
currentThread) &&
Open(std::move(porta), aSide, currentThread);
Open(std::move(porta), aSide, channelId, currentThread);
}

bool MessageChannel::Send(UniquePtr<Message> aMsg) {
Expand Down
14 changes: 13 additions & 1 deletion ipc/glue/MessageChannel.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ class MessageChannel : HasResultCodes {
// valid and connected to a remote.
//
// The `aEventTarget` parameter must be on the current thread.
bool Open(ScopedPort aPort, Side aSide,
bool Open(ScopedPort aPort, Side aSide, const nsID& aMessageChannelId,
nsISerialEventTarget* aEventTarget = nullptr);

// "Open" a connection to another thread in the same process.
Expand Down Expand Up @@ -314,6 +314,11 @@ class MessageChannel : HasResultCodes {
bool IsCrossProcess() const MOZ_REQUIRES(*mMonitor);
void SetIsCrossProcess(bool aIsCrossProcess) MOZ_REQUIRES(*mMonitor);

nsID GetMessageChannelId() const {
MonitorAutoLock lock(*mMonitor);
return mMessageChannelId;
}

#ifdef FUZZING_SNAPSHOT
Maybe<mojo::core::ports::PortName> GetPortName() {
MonitorAutoLock lock(*mMonitor);
Expand Down Expand Up @@ -584,6 +589,13 @@ class MessageChannel : HasResultCodes {
// This will be a string literal, so lifetime is not an issue.
const char* const mName;

// ID for each MessageChannel. Set when it is opened, and never cleared
// afterwards.
//
// This ID is only intended for diagnostics, debugging, and reporting
// purposes, and shouldn't be used for message routing or permissions checks.
nsID mMessageChannelId MOZ_GUARDED_BY(*mMonitor) = {};

// Based on presumption the listener owns and overlives the channel,
// this is never nullified.
IToplevelProtocol* const mListener;
Expand Down
7 changes: 4 additions & 3 deletions ipc/glue/ProcessChild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,11 @@ ProcessChild* ProcessChild::gProcessChild;

static Atomic<bool> sExpectingShutdown(false);

ProcessChild::ProcessChild(ProcessId aParentPid)
ProcessChild::ProcessChild(ProcessId aParentPid, const nsID& aMessageChannelId)
: ChildProcess(new IOThreadChild()),
mUILoop(MessageLoop::current()),
mParentPid(aParentPid) {
mParentPid(aParentPid),
mMessageChannelId(aMessageChannelId) {
MOZ_ASSERT(mUILoop, "UILoop should be created by now");
MOZ_ASSERT(!gProcessChild, "should only be one ProcessChild");
gProcessChild = this;
Expand Down Expand Up @@ -86,7 +87,7 @@ void ProcessChild::QuickExit() { AppShutdown::DoImmediateExit(); }

UntypedEndpoint ProcessChild::TakeInitialEndpoint() {
return UntypedEndpoint{PrivateIPDLInterface{},
child_thread()->TakeInitialPort(),
child_thread()->TakeInitialPort(), mMessageChannelId,
base::GetCurrentProcId(), mParentPid};
}

Expand Down
3 changes: 2 additions & 1 deletion ipc/glue/ProcessChild.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class ProcessChild : public ChildProcess {
typedef base::ProcessId ProcessId;

public:
explicit ProcessChild(ProcessId aParentPid);
explicit ProcessChild(ProcessId aParentPid, const nsID& aMessageChannelId);

ProcessChild(const ProcessChild&) = delete;
ProcessChild& operator=(const ProcessChild&) = delete;
Expand Down Expand Up @@ -66,6 +66,7 @@ class ProcessChild : public ChildProcess {

MessageLoop* mUILoop;
ProcessId mParentPid;
nsID mMessageChannelId;
};

} // namespace ipc
Expand Down
12 changes: 2 additions & 10 deletions ipc/glue/ProtocolMessageUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,17 +71,9 @@ template <>
struct ParamTraits<mozilla::ipc::UntypedEndpoint> {
using paramType = mozilla::ipc::UntypedEndpoint;

static void Write(MessageWriter* aWriter, paramType&& aParam) {
IPC::WriteParam(aWriter, std::move(aParam.mPort));
IPC::WriteParam(aWriter, aParam.mMyPid);
IPC::WriteParam(aWriter, aParam.mOtherPid);
}
static void Write(MessageWriter* aWriter, paramType&& aParam);

static bool Read(MessageReader* aReader, paramType* aResult) {
return IPC::ReadParam(aReader, &aResult->mPort) &&
IPC::ReadParam(aReader, &aResult->mMyPid) &&
IPC::ReadParam(aReader, &aResult->mOtherPid);
}
static bool Read(MessageReader* aReader, paramType* aResult);

static void Log(const paramType& aParam, std::wstring* aLog) {
aLog->append(StringPrintf(L"Endpoint"));
Expand Down
6 changes: 4 additions & 2 deletions ipc/glue/ProtocolUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -597,10 +597,12 @@ void IToplevelProtocol::SetOtherProcessId(base::ProcessId aOtherPid) {
mOtherPid = aOtherPid;
}

bool IToplevelProtocol::Open(ScopedPort aPort, base::ProcessId aOtherPid,
bool IToplevelProtocol::Open(ScopedPort aPort, const nsID& aMessageChannelId,
base::ProcessId aOtherPid,
nsISerialEventTarget* aEventTarget) {
SetOtherProcessId(aOtherPid);
return GetIPCChannel()->Open(std::move(aPort), mSide, aEventTarget);
return GetIPCChannel()->Open(std::move(aPort), mSide, aMessageChannelId,
aEventTarget);
}

bool IToplevelProtocol::Open(IToplevelProtocol* aTarget,
Expand Down
3 changes: 2 additions & 1 deletion ipc/glue/ProtocolUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,8 @@ class IToplevelProtocol : public IProtocol {
virtual void OnChannelError() = 0;
virtual void ProcessingError(Result aError, const char* aMsgName) {}

bool Open(ScopedPort aPort, base::ProcessId aOtherPid,
bool Open(ScopedPort aPort, const nsID& aMessageChannelId,
base::ProcessId aOtherPid,
nsISerialEventTarget* aEventTarget = nullptr);

bool Open(IToplevelProtocol* aTarget, nsISerialEventTarget* aEventTarget,
Expand Down
17 changes: 10 additions & 7 deletions ipc/ipdl/test/gtest/IPDLUnitTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,14 @@ IPDLUnitTestParent::~IPDLUnitTestParent() {

bool IPDLUnitTestParent::Start(const char* aName,
ipc::IToplevelProtocol* aActor) {
nsID channelId = nsID::GenerateUUID();
auto [parentPort, childPort] =
ipc::NodeController::GetSingleton()->CreatePortPair();
if (!SendStart(nsDependentCString(aName), std::move(childPort))) {
if (!SendStart(nsDependentCString(aName), std::move(childPort), channelId)) {
ADD_FAILURE() << "IPDLUnitTestParent::SendStart failed";
return false;
}
if (!aActor->Open(std::move(parentPort), OtherPid())) {
if (!aActor->Open(std::move(parentPort), channelId, OtherPid())) {
ADD_FAILURE() << "Unable to open parent actor";
return false;
}
Expand Down Expand Up @@ -160,7 +161,8 @@ void IPDLUnitTestParent::KillHard() {
}

ipc::IPCResult IPDLUnitTestChild::RecvStart(const nsCString& aName,
ipc::ScopedPort aPort) {
ipc::ScopedPort aPort,
const nsID& aMessageChannelId) {
auto* allocChildActor =
sAllocChildActorRegistry[std::string_view{aName.get()}];
if (!allocChildActor) {
Expand All @@ -174,7 +176,7 @@ ipc::IPCResult IPDLUnitTestChild::RecvStart(const nsCString& aName,
mojo::core::ports::PortRef port = aPort.Port();

auto* child = allocChildActor();
if (!child->Open(std::move(aPort), OtherPid())) {
if (!child->Open(std::move(aPort), aMessageChannelId, OtherPid())) {
ADD_FAILURE() << "Unable to open child actor";
return IPC_FAIL(this, "Unable to open child actor");
}
Expand Down Expand Up @@ -291,13 +293,14 @@ class IPDLUnitTestProcessChild : public ipc::ProcessChild {

// Defined in nsEmbedFunctions.cpp
extern UniquePtr<ipc::ProcessChild> (*gMakeIPDLUnitTestProcessChild)(
base::ProcessId);
base::ProcessId, const nsID&);

// Initialize gMakeIPDLUnitTestProcessChild in a static constructor.
int _childProcessEntryPointStaticConstructor = ([] {
gMakeIPDLUnitTestProcessChild =
[](base::ProcessId aParentPid) -> UniquePtr<ipc::ProcessChild> {
return MakeUnique<IPDLUnitTestProcessChild>(aParentPid);
[](base::ProcessId aParentPid,
const nsID& aMessageChannelId) -> UniquePtr<ipc::ProcessChild> {
return MakeUnique<IPDLUnitTestProcessChild>(aParentPid, aMessageChannelId);
};
return 0;
})();
Expand Down
3 changes: 2 additions & 1 deletion ipc/ipdl/test/gtest/IPDLUnitTestChild.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ class IPDLUnitTestChild : public PIPDLUnitTestChild {
private:
friend class PIPDLUnitTestChild;

ipc::IPCResult RecvStart(const nsCString& aName, ipc::ScopedPort aPort);
ipc::IPCResult RecvStart(const nsCString& aName, ipc::ScopedPort aPort,
const nsID& aChannelId);

void ActorDestroy(ActorDestroyReason aReason) override;

Expand Down
Loading

0 comments on commit 96d76af

Please sign in to comment.