Closed Bug 968825 Opened 7 years ago Closed 7 years ago

Missing null pointer checks in LayerTransactionParent::RecvUpdate


(Core :: Graphics, defect)

Not set





(Reporter: bjacob, Assigned: bjacob)




(1 file)

See dependent bugs: many crashes found by Gfx IPC fuzzing as plain null pointer dereferences in LayerTransactionParent::RecvUpdate() that could be easily averted by plain old null checks.
Comment on attachment 8372417 [details] [diff] [review]
Add missing null checks in LayerTransactionParent

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

This makes the code a lot more verbose and I'm worried that we might forget one of these null checks in the future. Can we do better?
Attachment #8372417 - Flags: review?(jmuizelaar) → review-
I still don't see an alternative. The pointers we're null-checking here are originally null in the default state of PLayer actors. They only become non-null as a result of earlier messages (Edit::TOpCreate*). So, to reproduce the present null situation, a client just needs to send e.g. a Edit::TOpInsertAfter with a PLayer that hadn't been previously been created by a Edit::TOpCreate*.

In other words, this is just a manifestation of the familiar idiom that stateful APIs drag in compulsory stupid error handling.

Do you see a better alternative?
Flags: needinfo?(jmuizelaar)
Comment on attachment 8372417 [details] [diff] [review]
Add missing null checks in LayerTransactionParent

I would like to either land this or be given a specific other approach to take. Feel free to redirect the review if needed.
Attachment #8372417 - Flags: review- → review?(jmuizelaar)
Jrmuizel told me "r+" offline and I landed without realizing that the r+ was not yet officialized on bugzilla. Don't back me out for now...
Assignee: nobody → bjacob
Attachment #8372417 - Flags: review?(jmuizelaar) → review+
Flags: needinfo?(jmuizelaar)
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.