Closed
Bug 1000104
Opened 10 years ago
Closed 10 years ago
faulty: null ptr deref in LayerTransactionParent::RecvSetAsyncScrollOffset
Categories
(Core :: Graphics: Layers, defect)
Core
Graphics: Layers
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: bjacob, Assigned: bjacob)
References
Details
Attachments
(3 files)
14.99 KB,
text/plain
|
Details | |
2.98 KB,
patch
|
kats
:
review+
|
Details | Diff | Splinter Review |
1.30 KB,
patch
|
bjacob
:
review+
|
Details | Diff | Splinter Review |
Found while fuzzing.
Assignee | ||
Comment 2•10 years ago
|
||
This patch should fix the null ptr deref; it also changes some 'return true' to 'return false' as these were returning true on error cases, which seemed unintentional (I suppose that if intentional, they would have been explained in a comment).
Attachment #8410975 -
Flags: review?(bugmail.mozilla)
Comment 3•10 years ago
|
||
Comment on attachment 8410975 [details] [diff] [review] fix Review of attachment 8410975 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/ipc/LayerTransactionParent.cpp @@ +678,2 @@ > if (!layer) { > + return false; Returning false in IPDL's Recv*** method means "hey IPDL, I am in an invalid state, please kill me now, so long and thanks for the fish". So I am not sure that returning true was unintentional (but I don't know that code so I can't say for sure).
Assignee | ||
Comment 4•10 years ago
|
||
Returning false on the parent side doesn't mean "kill me now", it means "kill my child now". Killing children can be a great relief for parents.
Comment 5•10 years ago
|
||
Comment on attachment 8410975 [details] [diff] [review] fix Review of attachment 8410975 [details] [diff] [review]: ----------------------------------------------------------------- I think returning false in case of error here and killing something is fine, because this function is only exercised in test scenarios anyway. Please make sure to remote the .gdbinit and all.js changes from this patch before landing as I assume they got accidentally included here.
Attachment #8410975 -
Flags: review?(bugmail.mozilla) → review+
Comment 6•10 years ago
|
||
(Although it's worth doing a try push to make sure none of the existing tests rely on this behaviour)
Assignee | ||
Comment 7•10 years ago
|
||
Sure, and I'll also make sure to water the plants :-P
Assignee | ||
Comment 8•10 years ago
|
||
Oh wait sorry, I just realized what you were talking about... thanks for catching that.
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8411016 -
Flags: review+
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/663c5751c29e
Assignee: nobody → bjacob
Comment 11•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/663c5751c29e
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•