780778 - Make sure we always release stuff on the right thread.

Status: RESOLVED FIXED

(Core :: Storage: IndexedDB, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Since some of the StructuredClone*Info structs can hold onto CCd objects, we need to be sure we always empty them on the main thread.

Comment 1

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 649478 [details] [diff] [review]
Patch

Review of attachment 649478 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/indexedDB/IDBCursor.cpp
@@ -79,5 @@
>    }
>  
>    ~ContinueHelper()
>    {
> -    IDBObjectStore::ClearStructuredCloneBuffer(mCloneReadInfo.mCloneBuffer);

We still need this here and in all the other destructors (if something fails and we don't actually dispatch to the threadpool then we still end up never calling ReleaseMainThreadObjects).

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

If that happens, the only safe thing to do is to leak all the StructuredCloneFiles ... perhaps we should just abort?

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

No, if that happens then we're already on the main thread and the destructors for those objects will release everything just fine.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Attachment: Patch

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5c2b826588d3

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5c2b826588d3

Comment 7

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Comment on attachment 666658 [details] [diff] [review]
Patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 661877
User impact if declined: Potential security problem
Testing completed (on m-c, etc.): On m-c, simple enough to understand easily.
Risk to taking this patch (and alternatives if risky): Low risk.
String or UUID changes made by this patch: None.

Comment 8

[Alex Keybl [:akeybl]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #7)
> [Approval Request Comment]
> User impact if declined: Potential security problem

I'm assuming this isn't an sec-high or sec-critical since this landed on central without sec-approval. Sec-moderate?

Comment 9

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

That's probably reasonable.  This would be really hard to exploit.

Comment 10

[Alex Keybl [:akeybl]]

Comment on attachment 666658 [details] [diff] [review]
Patch

[Triage Comment]
Approving for Aurora 17 - please land early on Monday to make the merge.

Comment 11

[Justin Wood (:Callek)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/b1e06ca302c2