Closed Bug 1415852 Opened 5 years ago Closed 4 years ago

Unconditionally poison chunk trailer on free


(Core :: JavaScript: GC, enhancement, P3)




Tracking Status
firefox61 --- fixed


(Reporter: jonco, Assigned: jonco)



(Keywords: good-first-bug)


(1 file, 1 obsolete file)

To help catch use-after-free bugs we should unconditionally poison the chunk trailer for free chunks.

The trailer (ChunkTrailer) stores things like the associated runtime that are used when working with cells in that chunk.  Poisoning these values could show up UAF bugs much sooner.  There shouldn't be any noticeable performance impact so we should do this all the time.

This could happen in GCRuntime::recycleChunk().
Priority: -- → P3
Assignee: nobody → jcoppeard
Jonco, since it's a good first bug, can I take this? :P

Attached patch bug1415852-poison-chunk-trailer (obsolete) — Splinter Review
Well I already wrote the patch.  There are some test failures with this I'm looking into at the moment.
Depends on: 1419373
Rebased patch to unconditionally poison chunk trailer on free.
Attachment #8969629 - Flags: review?(sphink)
Attachment #8928508 - Attachment is obsolete: true
Comment on attachment 8969629 [details] [diff] [review]
bug1415852-poison-chunk-trailer v2

Review of attachment 8969629 [details] [diff] [review]:

Uh... weird question, but did we have this at some point? Because I went to add this to the mrgiggles knowledge base of magic numbers, and found this already there:

        '8b': '''\                                                                                                                                                
0x8b is JS_FREED_CHUNK_PATTERN, the poison value written to the trailer of freed chunks                                                                           
<...2sec...>this will be accessed when looking up things like runtime, store buffer address, or ChunkLocation                                                     

but I don't see it in the current code, and I can't find it with hg grep.
Attachment #8969629 - Flags: review?(sphink) → review+
Oh, never mind. I looked at when I added it:

changeset:   236:206e54983146
user:        Steve Fink <>
date:        Thu Nov 16 16:49:58 2017 -0800
summary:     Document 0x8b8b8b8b in anticipation of jonco landing bug 1415852
Pushed by
Unconditionally poison chunk trailer on free r=sfink
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla61
You need to log in before you can comment on or make changes to this bug.