1550037 - Shared memory blocks used by the font list should be impossible for content processes to map with write access

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, enhancement, P3)

Description

[Jonathan Kew [:jfkthame]]

To make the font list more robust against a potential rogue content process, we'd like to "freeze" the shared memory blocks once support for this (bug 1479960) is available.

As the initial version of the shared font list code relies on incrementally updating the shared data, we'll need to refactor the allocation strategy a bit in order to freeze each piece of data as it's added.

Comment 1

[Jonathan Kew [:jfkthame]]

Currently, when shmem blocks are shared to the content process, they're mapped there as read-only, but AFAIK there's nothing in principle that prevents a rogue content process re-mapping the block with write access. Once bug 1550900 is done, we should configure these blocks such that they cannot ever be made writable from the content process.

Comment 2

[Jonathan Watt [:jwatt]]

I'm about to punt bug 1533462 which was targeted at M4 because we can't land without this. I'm guessing this should also be targeting a milestone given that.

Comment 3

[Jonathan Kew [:jfkthame]]

Probably bug 1550900 should also have the same target, given that this is dependent on it.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1550037 - patch 1 - Migrate shared font-list code from mozilla::ipc::SharedMemoryBasic to base::SharedMemory APIs. r=jwatt

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1550037 - patch 2 - Ensure the font-list memory blocks passed to content processes are shared as readonly copies. r=jwatt

Depends on D68778

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1550037 - patch 3 - Remove mAddr from the ShmBlock struct, as mShmem->memory() is now a trivial inline accessor. r=jwatt

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/71fdead8eecb
patch 1 - Migrate shared font-list code from mozilla::ipc::SharedMemoryBasic to base::SharedMemory APIs. r=jwatt
https://hg.mozilla.org/integration/autoland/rev/7571e5bc19e7
patch 2 - Ensure the font-list memory blocks passed to content processes are shared as readonly copies. r=jwatt
https://hg.mozilla.org/integration/autoland/rev/34ebd6260867
patch 3 - Remove mAddr from the ShmBlock struct, as mShmem->memory() is now a trivial inline accessor. r=jwatt

Comment 8

[Alexandru Michis [:malexandru]]

Backed out 3 changesets (Bug 1550037) for causing bustages in SharedFontList-impl.h

Backout link: https://hg.mozilla.org/integration/autoland/rev/d850c01b531f5ec42d195cfb4ef92fcdb272c2d5

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&collapsedPushes=676749&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&tochange=d850c01b531f5ec42d195cfb4ef92fcdb272c2d5&fromchange=34ebd6260867364e46f95de72538e4c655aa8869

Failure log:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=296570245&repo=autoland&lineNumber=43785

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/005a8977f8fc
patch 1 - Migrate shared font-list code from mozilla::ipc::SharedMemoryBasic to base::SharedMemory APIs. r=jwatt
https://hg.mozilla.org/integration/autoland/rev/0e5c6a1e1bee
patch 2 - Ensure the font-list memory blocks passed to content processes are shared as readonly copies. r=jwatt
https://hg.mozilla.org/integration/autoland/rev/1394bc3c6261
patch 3 - Remove mAddr from the ShmBlock struct, as mShmem->memory() is now a trivial inline accessor. r=jwatt

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/005a8977f8fc
https://hg.mozilla.org/mozilla-central/rev/0e5c6a1e1bee
https://hg.mozilla.org/mozilla-central/rev/1394bc3c6261