Closed Bug 1000104 Opened 6 years ago Closed 6 years ago

faulty: null ptr deref in LayerTransactionParent::RecvSetAsyncScrollOffset


(Core :: Graphics: Layers, defect)

Not set





(Reporter: bjacob, Assigned: bjacob)




(3 files)

Attached file Faulty session
Found while fuzzing.
Code added in bug 975931
Blocks: 975931
Attached patch fixSplinter Review
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 on attachment 8410975 [details] [diff] [review]

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).
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 on attachment 8410975 [details] [diff] [review]

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+
(Although it's worth doing a try push to make sure none of the existing tests rely on this behaviour)
Sure, and I'll also make sure to water the plants :-P
Oh wait sorry, I just realized what you were talking about... thanks for catching that.
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.