Closed Bug 1423281 Opened 2 years ago Closed 2 years ago

SourceSurfaceCapture::ResolveImpl assumed SourceSurfaces keep their DrawTargets alive

Categories

(Core :: Graphics, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox58 --- fixed
firefox59 --- fixed

People

(Reporter: bas.schouten, Assigned: bas.schouten)

References

Details

Attachments

(1 file)

This happened to work with the source surfaces we ended up using here in practice. However as a general rule this would potentially cause a UAF.
Comment on attachment 8934630 [details]
Bug 1423281: Store the userdata for freeing our memory on the longer living snapshot.

https://reviewboard.mozilla.org/r/205498/#review211160

How is this affected by the ownership changes we're making? If for example the DrawTarget goes away, will that free the underlying storage that SourceSurfaceSkia::mSurface uses? Should the key be associated with the Skia surface instead?
Well, this function already destroys the DrawTarget upon exciting, so the SourceSurface is the only object that points to the data, so I believe this is a safe change.
Flags: needinfo?(bas)
Comment on attachment 8934630 [details]
Bug 1423281: Store the userdata for freeing our memory on the longer living snapshot.

Sorry, misread the patch - looks good!
Attachment #8934630 - Flags: review?(dvander) → review+
Pushed by bschouten@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/62f80084da60
Store the userdata for freeing our memory on the longer living snapshot. r=dvander
https://hg.mozilla.org/mozilla-central/rev/62f80084da60
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment on attachment 8934630 [details]
Bug 1423281: Store the userdata for freeing our memory on the longer living snapshot.

Approval Request Comment
See bug 1416862.
Attachment #8934630 - Flags: approval-mozilla-beta?
Comment on attachment 8934630 [details]
Bug 1423281: Store the userdata for freeing our memory on the longer living snapshot.

gfx fix, beta58+
Attachment #8934630 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Needs a rebased patch for Beta uplift.
Flags: needinfo?(bas)
Flags: needinfo?(bas)
Depends on: 1493080
You need to log in before you can comment on or make changes to this bug.