Closed Bug 966446 Opened 6 years ago Closed 6 years ago

Use after free of PGrallocBufferParent by new texture when the child process is killed

Categories

(Core :: Graphics: Layers, defect)

Product:
Core
Component:
Graphics: Layers
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla30
Project Flags:
blocking-b2g 1.3T+
Tracking Flags:
Tracking Status
b2g-v1.3T --- fixed

People

(Reporter: sotaro, Assigned: sotaro)

References

Details

Attachments

(2 files, 2 obsolete files)

patch v2 - Handle GrallocBufferActor::ActorDestroy for new texture (r=nical)
7.42 KB, patch
sotaro
: review+
Details | Diff | Splinter Review
patch v2 for b2g v1.3t- Handle GrallocBufferActor::ActorDestroy for new texture (r=nical)
7.37 KB, patch
Details | Diff | Splinter Review 
I faced this problem during Bug 946720 development. In the past same problem was fixed by Bug 862324. But it is only for deprecated texture.
OS: Windows 7 → Gonk (Firefox OS)
Hardware: x86_64 → ARM
Attached file stack trace of the crash (obsolete) —
Blocks: 946720
Deprecated texture's problem was actually fixed by Bug 915869.
Assignee: nobody → sotaro.ikeda.g
Status: NEW → ASSIGNED
The problem is that GrallocTextureHostOGL calls PGrallocBufferParent::Send__delete__() after GrallocBufferActor deletion.
There are the following ways to fix the problem.
- [1] Change GrallocBufferActor as reference counted and GrallocTextureHostOGL does not directly call PGrallocBufferParent::Send__delete__().
- [2] GrallocTextureHostOGL forget a pointer to GrallocBufferActor when GrallocBufferActor is destroyed by IPC error.

[1] is ideal way to fix the problem. But the change becomes relatively large. It might be better to put off [1] to MozSurface implementation. [2] is not a better way, but relatively easy to implement. [2] is same way as in  Bug 915869 for deprecated texture.
This is a hacky fix. Confirmed that problem in bug 946720 is fixed by the patch. 
In near future, MozSurface should fix the problem correctly.
Attachment #8370957 - Flags: review?(nical.bugzilla)
Attachment #8370957 - Flags: review?(nical.bugzilla) → review+
Committable patch. Carry 'r=nical'.
Attachment #8368800 - Attachment is obsolete: true
Attachment #8370957 - Attachment is obsolete: true
Attachment #8371982 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/eee6439dac17
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Blocks: 994590
Nominate to "1.3T+". This bug block's bug 994590.
blocking-b2g: --- → 1.3T?
blocking-b2g: 1.3T? → 1.3T+
v1.3 patch conflict
Hi! Sotaro,

Could you help? Thanks

--
Keven
Flags: needinfo?(sotaro.ikeda.g)
A patch for b2g v1.3t.
Flags: needinfo?(sotaro.ikeda.g)
You need to log in before you can comment on or make changes to this bug.