Open Bug 1609882 (forkserver) Opened 4 years ago Updated 1 day ago

Enable the fork server by default in Linux builds

Categories

(Core :: IPC, task, P3)

Desktop
Linux
task

Tracking

()

People

(Reporter: gsvelto, Unassigned)

References

(Depends on 4 open bugs, Blocks 1 open bug)

Details

(Keywords: parity-chrome)

As per title, the idea is to start building the fork server on all Linux builds while having it off by default at runtime.

Priority: -- → P3

All the dependencies on this bug are closed. What other work is necessary for the Fork Server to be turned on by default?

Flags: needinfo?(jld)

I filed bug 1752638 for the main thing I'd like to see fixed. There are also a few oddities I've noticed in the code. For example, I don't think it should be necessary to pad the fork server's file descriptors like this (and if we do, it may be a bug in FileDescriptorShuffle). And this comment about a message loop is probably stale? Also, the way sandboxing state is passed is kind of hacky (variously, a file descriptor mapping that isn't actually given to the child and appended to an env var that was originally a boolean); I don't think this is a functional bug, but it would be nice to clean that up someday (which may need the ForkDelegate interface to be redesigned so that it can be serializable/deserializable in a more normal way).

While scrolling through the code I also see these attempts to prevent data from remaining in the fork server's address space (so that an untrusted child process can't see the args/env of previous launch requests) which is a good idea but it may not actually work; compilers like to delete dead stores, and string buffer resizing could also leave copies in memory.

It's currently limited to content processes (and asserted here) but as far as I know that's not an inherent limitation. (GMP processes are still run via plugin-container instead of firefox -contentproc; I don't think anything depends on that, but I could be wrong.)

And speaking of process types: I don't think we do anything to try to change the kernel's idea of argv (exposed via /proc/N/cmdline etc.), so everything would look like a forkserver in ps; I think Chromium might have some tricks for this. People will probably complain if we ship to release without doing anything about that (and for that matter, Gecko developers may complain if we ship it on Nightly).

Flags: needinfo?(jld)
Severity: normal → S3
Blocks: 1830516
Blocks: 1705217
Blocks: 1772219
Depends on: 1532782
OS: Unspecified → Linux
Hardware: Unspecified → Desktop
Depends on: 1805788
Keywords: parity-chrome
Alias: forkserver
No longer blocks: 1772219
Depends on: 1851271

(In reply to Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧ from comment #2)

I don't think it should be necessary to pad the fork server's file descriptors like this (and if we do, it may be a bug in FileDescriptorShuffle).

I think I understand this a little better now — there's also this hack to exempt existing fds from being closed during remapping, used by refcount logging, so those fds need to not overlap with a dup2 target. It would be nice if refcount logging (and similar) could be completely shut down before the fd remapping and started for the new process afterwards, but I don't know how difficult that would be.

It's currently limited to content processes (and asserted here) but as far as I know that's not an inherent limitation. (GMP processes are still run via plugin-container instead of firefox -contentproc; I don't think anything depends on that, but I could be wrong.)

Fixed in bug 1805788 (and bug 1532782). There's one more vestige of this (I haven't tried it yet but I think setting MOZ_DISABLE_CONTENT_SANDBOX can potentially make sandboxed non-content processes not work) but that's already fixed in another patch that I'm about to file a bug for.

And speaking of process types: I don't think we do anything to try to change the kernel's idea of argv (exposed via /proc/N/cmdline etc.), so everything would look like a forkserver in ps; I think Chromium might have some tricks for this. People will probably complain if we ship to release without doing anything about that (and for that matter, Gecko developers may complain if we ship it on Nightly).

Fixed in bug 1851095.

I have some ideas for how to deal with the other issues….

Depends on: 1874689
Depends on: 1888125
You need to log in before you can comment on or make changes to this bug.