Open Bug 567392 Opened 14 years ago Updated 2 years ago

Marking a NULL object hits a high address (not guaranteed crash).

Categories

(Core :: JavaScript Engine, defect, P3)

defect

Tracking

()

People

(Reporter: gal, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: sec-want, Whiteboard: [sg:want P1])

This is a follow-up for 567387. Not marking dependency as I am about to open that bug.

If we accidentally mark an object that is null, we reach into the mark bitmap, which is after the arenas in the GC chunk. Since we increased the size of the GC chunks, the bitmap is now at a pretty high address for NULL, no longer guaranteeing a crash. We might want to flip the bitmap and arena order around just as an additional safe-guard.

+--------+--------------+-------+--------+------------+-----------------+
| arenas | mark bitmaps | infos | delays | chunk info | free arena bits |
+--------+--------------+-------+--------+------------+-----------------+

This is not a bug in itself without someone accidentally trying to mark NULL or other invalid low addresses.
Priority: -- → P2
Whiteboard: [sg:high]
Andreas, do you have a plan on when you are going to get to this?
I just filed this bug, I am not owning it. I don't think we have to block on this. Its more a defense in depth thing.
This isn't really sg:high without another bug actually active. I don't think that we have to be too worried about marking null, so bumping this to sg:want.
Whiteboard: [sg:high] → [sg:want P1]
No objections.
Does this bug need to remain hidden?
Group: core-security
Terrence: is Andreas' concern about marking NULL still relevant in GGC's implementation?
Flags: needinfo?(terrence)
If we put the mark bitmap before a nullptr, the address will be a small negative signed integer, e.g. a large positive unsigned integer, e.g. at the end of the address range.

In my currently running instance of firefox, my lowest mapping is:
00400000-00419000 r-xp 00000000 09:00 1476550259                         /usr/lib64/firefox/firefox (deleted)

And my highest mapping is:
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]

On x64 linux, it looks like both arrangements are pretty much equally dangerous. I'd be interested to see what the mappings look like on 32bit and in windows. Additionally, looking at comments in js/src/gc/Memory.cpp, on ia64, linux actively maps memory at very high address ranges. On this platform, the situation would be markedly worse with bitmap-at-the-front.

I think a better protection here would be to try to map the memory at the low offsets where the mark bitmap resides and explicitly make them non-writable. This shouldn't be too hard to implement, but I wouldn't prioritize it very highly:

(1) All but non-rope JSString will mark pointers in the object's internals: very small addresses.
(2) The relevant low mappings are very likely to be non-writable (although we should verify this on more platforms).
(3) We assert non-null in DEBUG builds, which quickly catches most bugs where this could happen. However, it would probably be worth making this a release assertion on nightly too, to at least see if it happens at all in the wild.
Flags: needinfo?(terrence)
Actually, scratch that. The way we currently do masking for the bitmap would be safe with bitmap-at-the-front. It is only the trailer computation that would need to be adjusted, and then only in some places.

On the other hand, if we mask the trailer with PROT_NONE, that would also get us protection from some accidental uses of a NULL Cell*, so the technique I proposed could actually be more useful in practice than I gave it credit for. I'll try and whip something up for our ongoing diagnostics work.
Blocks: 994253
Assignee: general → nobody
Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information
Priority: P2 → P3
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.