The baseShapes table is keyed based on parent and metadata objects pointers, both of which may be in the nursery. It needs a postbarrier to rekey entries if these objects are moved on generational GC. Forking this issue from bug 1124563.

Add a check to see if the contents of the baseShapes table are valid. This causes our tests to fail if we enable the checks with zeal mode 13.

I had an idea to mark generic barriers first to get rid of the problem where hashtable barriers see an inconsistent view of, where the table's keys have are based on original object locations but their values contain object pointers that have already been updated by other barriers. Sadly this doesn't work, because moving the entry in the generic barrier means that the barriers for any pointers in the value now have the wrong address. I've just fixed this in the same way we use for other barriers, by giving the lookup structure the ability to match an entry for which the key hash and key value contents are based on prior and updated object pointers respectively.

This issue has been present since we enabled GGC in bug 619558.

This sounds like it could result in dead pointers, though it also sounds difficult to exploit.

Comment on attachment 8556481 [details] [diff] [review] post-barrier-base-shapes-table Review of attachment 8556481 [details] [diff] [review]: ----------------------------------------------------------------- Nice! ::: js/public/HashTable.h @@ +252,5 @@ > if (old_key != new_key) > rekeyAs(old_key, new_key, new_key); > } > > // Infallibly rekey one entry, if present. Please extend the comment to explain the bool being returned. @@ +473,5 @@ > if (old_value != new_value) > rekeyAs(old_value, new_value, new_value); > } > > // Infallibly rekey one entry, if present. Ditto.

Combined patch with review comments addressed.

Comment on attachment 8557038 [details] [diff] [review] bug1127246-post-barrier-base-shapes-table [Security approval request comment] How easily could an exploit be constructed based on the patch? Very hard. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No. Which older supported branches are affected by this flaw? Everything back to FF32. If not all supported branches, which bug introduced the flaw? Bug 619558. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? This patch should apply directly or with minimal changes. How likely is this patch to cause regressions; how much testing does it need? Unlikely to cause regressions.

sec-approval+ for trunk. I'd like to see Aurora and Beta patches nominated and then checked in as well to get it in the next beta.

Jon, could you land and fill the uplift requests so that we have it in beta asap? Thanks

Rebased patch for mozilla-beta, carrying r+.

Jon, sorry to bother you again but we would also need the uplift request, thanks!

Comment on attachment 8557906 [details] [diff] [review] baseShapes-patch-aurora Approval Request Comment [Feature/regressing bug #]: Bug 619558. [User impact if declined]: Possible stability/security issue. [Describe test coverage new/current, TreeHerder]: On m-c for 24 hours. [Risks and why]: Low risk. [String/UUID change made/needed]: None.

Comment on attachment 8557907 [details] [diff] [review] baseShapes-patch-beta Approval Request Comment [Feature/regressing bug #]: Bug 619558. [User impact if declined]: Possible stability/security issue. [Describe test coverage new/current, TreeHerder]: On m-c for 24 hours. [Risks and why]: Low risk. [String/UUID change made/needed]: None.

Comment on attachment 8557907 [details] [diff] [review] baseShapes-patch-beta Thanks

https://hg.mozilla.org/releases/mozilla-aurora/rev/b4e61581f650 https://hg.mozilla.org/releases/mozilla-beta/rev/e0a36c5bdf4c