Closed Bug 1553717 Opened 6 years ago Closed 5 years ago

Use Randomization on all arenas in non-Content Processes

Categories

(Core :: Memory Allocator, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox78 --- fixed

People

(Reporter: tjr, Assigned: gcp)

References

Details

Attachments

(1 file, 1 obsolete file)

Assuming I can land Bug 1376408, I'd like to try to enable randomization on all non content processes. Perfherder shows painful regressions on DOM Traversal in Content Processes when we enabled randomization on all arenas (or the JS Runtime arena).

I hacked a build that enabled randomization on the parent process:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=108e1e25f510a9b16de9b91e8a86c844391dd9b9
https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=108e1e25f510a9b16de9b91e8a86c844391dd9b9&selectedTimeRange=172800

The regressions are few and slight. This makes me think I might be able to stick this improvement. And it is in non-Content Processes that we will derive the most benefit: an attacker will be exploiting these over IPC and don't have the benefit of a JS Engine to carefully groom the heap.

To do this, I need to figure out what process we're running as inside of mozglue extremely early in process startup. I believe XRE_GetProcessType() and related are all in xul; so I am thinking I might need to re-implement it as lightweight as possible (and put in assertions to ensure it stays in sync in the future.) Looking for feedback on this approach...

This was also partially re-implemented for the Windows DLL Blocklist with gBlocklistInitFlags - but that's specific to Windows. It seems likely we don't need both gBlocklistInitFlags and my solution so maybe they can be combined... But I think I'd probably try to do that after I got something independent working.

Flags: needinfo?(mh+mozilla)
Flags: needinfo?(aklotz)

You could do a combined solution, but I should point out that the reason why we use gBlocklistInitFlags is because, when bootstrapped, the browser process disables the legacy DLL blocklist. In other words, I don't want (for example) a third party to be able to fool us into disabling our DLL blocking by adding a magic command line flag, or something that is relatively easy to tamper with. If you are going to change this, please keep that in mind.

Flags: needinfo?(aklotz)
Assignee: tom → gpascutto

XRE_SetProcessType + XRE_GetProcessType are both in Bootstrap.cpp/h (so they're externally visible), and are doing simple things like strcmp. So it looks like they would be safe to use even very early on.

So the above ends up not being true. The problem is that the XRE_*Process calls need nsXULApi.h and related headers, which include nscore.h, which includes mozalloc.h and this causes conflicts as we're in mozjemalloc itself. Blocking that include with MOZ_NO_MOZALLOC doesn't work either, as mozmemory gets pulled in...somewhere.

The good news is that the GeckoProcessType list is structured such that it is possible to generate multiple lists from the same header file, so there's a lower risk things get out of sync.

Attempting to make a Linux version of Tom's patch to understand what's going on didn't really work. It looks like malloc is already called before main() is entered which makes inspecting and propagating the command line flags tricky:

#0  arena_t::arena_t (this=0x7ffff6b00000, aParams=0x7fffffffc900)
    at /home/morbo/hg/firefox/memory/build/mozjemalloc.cpp:3531
#1  0x000055555558db21 in ArenaCollection::CreateArena (this=0x5555556398c8 <gArenas>, 
    aIsPrivate=false, aParams=0x7fffffffc900)
    at /home/morbo/hg/firefox/memory/build/mozjemalloc.cpp:3605
#2  0x000055555558f49f in ArenaCollection::Init (this=<optimized out>)
    at /home/morbo/hg/firefox/memory/build/mozjemalloc.cpp:1065
#3  malloc_init_hard () at /home/morbo/hg/firefox/memory/build/mozjemalloc.cpp:3983
#4  malloc_init () at /home/morbo/hg/firefox/memory/build/mozjemalloc.cpp:1219
#5  0x0000555555593e76 in Allocator<MozJemallocBase>::jemalloc_stats (aStats=0x7fffffffd570)
    at /home/morbo/hg/firefox/memory/build/mozjemalloc.cpp:4233
#6  0x00005555555ae991 in phc_init (aMallocTable=0x7fffffffd610, 
    aBridge=0x555555639738 <gReplaceMallocBridge>)
    at /home/morbo/hg/firefox/memory/replace/phc/PHC.cpp:1327
#7  0x000055555558e025 in init () at /home/morbo/hg/firefox/memory/build/mozjemalloc.cpp:4659
#8  0x000055555558e1cc in Allocator<ReplaceMallocBase>::malloc (arg1=72704)
    at /home/morbo/hg/firefox/memory/build/malloc_decls.h:51
#9  malloc (arg1=72704) at /home/morbo/hg/firefox/memory/build/malloc_decls.h:51
#10 0x00007ffff76b8426 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#11 0x00007ffff7de5733 in call_init (env=0x7fffffffd790, argv=0x7fffffffd768, argc=4, 
    l=<optimized out>) at dl-init.c:72
#12 _dl_init (main_map=0x7ffff7ffe170, argc=4, argv=0x7fffffffd768, env=0x7fffffffd790)
    at dl-init.c:119
#13 0x00007ffff7dd60ca in _dl_start_user () from /lib64/ld-linux-x86-64.so.2
Attachment #9125186 - Attachment description: Bug 1553717 - Use Randomization on all arenas in non-Content Processes. → Bug 1553717 - Use Randomization on all arenas in non-Content Processes. r=tjr,glandium
Attachment #9125186 - Attachment is obsolete: true

Performance data indicates some small gains and some small regressions. I suspect that this is simply luck or bad luck with placement in the cache of things?

twinopen ext+twinopen:twinopen.html opt e10s stylo is 32% worse, which seems weird.

Local test with twinopen shows <1% difference.

Attachment #9139265 - Attachment description: Bug 1553717 - Use Randomization on all arenas in non-Content Processes. → Bug 1553717 - Use Randomization on all arenas in non-Content Processes. r?glandium
Pushed by gpascutto@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/83a818c96a40 Use Randomization on all arenas in non-Content Processes. r=glandium
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Flags: needinfo?(mh+mozilla)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: