Closed Bug 968825 Opened 6 years ago Closed 6 years ago
Missing null pointer checks in Layer
Transaction Parent::Recv Update
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?
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... https://hg.mozilla.org/integration/mozilla-inbound/rev/e508a0767ec0
Attachment #8372417 - Flags: review?(jmuizelaar) → review+
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in before you can comment on or make changes to this bug.