Remove "channel IDs" on Unix
Categories
(Core :: IPC, task, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox80 | --- | fixed |
People
(Reporter: jld, Assigned: jld)
References
(Blocks 1 open bug)
Details
Attachments
(4 files)
The Chromium-derived IPC code was, as far as I can tell, originally designed for Windows and assumed that channels would be named pipes, managed and connected to via std::wstring
paths. The port to Unix, however, used unnamed socketpair()
and passed them directly from process to process, so it has no use for these channel IDs... but it still computes and propagates them, even though they're not used, using deprecated wide-string APIs.
This also indirectly causes bug 1556599, because using wide strings causes glibc locale initialization which reads the environment on the I/O thread, presumably racing with something on the main thread changing the environment. Removing channel IDs isn't a proper fix for the underlying problem there — we'd need to refrain from modifying the environment while multithreaded, which is a large task and outside the scope of IPC — but it will prevent this particular instance of it.
Assignee | ||
Comment 1•5 years ago
|
||
The PipeMap class tries to simulate the Windows channel model (named
pipes that the client opens by a pathname) on Unix. However, it's
effectively dead code -- the map is empty except in some unit tests that
we never imported.
What we do is generate a "channel ID" with string formatting, then don't
pass it to the child or ever insert anything into the map, then the child
looks up an empty string and doesn't find it, so it uses the hard-coded
fixed fd for the initial channel.
Basically, it does nothing except maybe confuse unfamiliar readers, so
let's get rid of it.
Assignee | ||
Comment 2•5 years ago
|
||
This "create a pipe" operation has a mode where, on Unix, it doesn't
create a new transport but rather uses a hard-coded fd for the initial
IPC channel in a child process. (It was originally written for Windows
and the assumption of using named pipes and pathnames for everything.)
That seems like a footgun, so this patch checks for trying to "create"
that pipe twice. However, it doesn't check for accidentally calling it
in the parent process.
Assignee | ||
Comment 3•5 years ago
|
||
The Chromium-derived IPC code was, as far as I can tell, originally
designed for Windows and assumed that channels would be named pipes,
managed and connected to via std::wstring
paths. The port to Unix,
however, used unnamed socketpair()
and passed them directly from
process to process, so it has no use for these channel IDs... but it
still computes and propagates them, even though they're not used, using
deprecated wide-string APIs.
This patch introduces a typedef for an abstract channel ID, which is a
wstring
on Windows and an empty struct on Unix, to allow removing the
string code where it's not needed without needing to completely redesign
the channel abstraction.
Updated•4 years ago
|
Assignee | ||
Comment 4•4 years ago
|
||
Chromium's fix for CVE-2011-3079 added an optional prefix parameter for
channel IDs, but we've never used it and have no plans to. (Chromium
itself doesn't appear to have used it except with the prefixes "gpu"
and "nacl", and the code has since been removed completely in favor of
Mojo.) So let's simplify things and remove it.
Comment 6•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1706ed209e22
https://hg.mozilla.org/mozilla-central/rev/9cdc37802d12
https://hg.mozilla.org/mozilla-central/rev/66bfa0dd136d
https://hg.mozilla.org/mozilla-central/rev/d87f5e311934
Description
•