1353610 - Add null checks to TaskDispatcher.h

Status: RESOLVED FIXED

(Core :: XPCOM, defect, P1)

Description

[JW Wang [:jwwang] [:jw_wang]]

We have
https://crash-stats.mozilla.com/report/index/978fff99-50b0-406f-9d39-a0ecf2170329
https://crash-stats.mozilla.com/report/index/6bfbd5fb-b02f-4292-8549-9db5b2170329
https://crash-stats.mozilla.com/report/index/9edc3dbe-9217-485d-ae27-4f7da2170330

which look like a null deref and crashed on the main thread.

I will add some null checks in the hope of reducing null deref crash so other UAF issues can stand out.

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1353610_add_null_checks.patch

Per suggestion of bug 1342420 comment 10, I file a new bug for null checks. This patch has been r+ed by bobbyholley in bug 1342420.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/678773021f14
Add null checks. r=bobbyholley

Comment 3

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/678773021f14

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8854676 [details] [diff] [review]
1353610_add_null_checks.patch

Approval Request Comment
[Feature/Bug causing the regression]:none
[User impact if declined]:A patch to debug null-deref crashes.
[Is this code covered by automated tests?]:yes
[Has the fix been verified in Nightly?]:yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:none
[Is the change risky?]:low
[Why is the change risky/not risky?]:The change is simple which add some null checks.
[String changes made/needed]:none

Comment 5

[Gerry Chang [:gchang]]

Comment on attachment 8854676 [details] [diff] [review]
1353610_add_null_checks.patch

Hi JW,
If [feature/bug causing the regression] is none, why do we need this to be uplift? Do we have any crashes reported? Aurora54-.

Comment 6

[JW Wang [:jwwang] [:jw_wang]]

Yes, the crash is reported in bug 1342420 which however is not a regression I think.

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Seems that it doesn't apply:
---
grafting 390911:678773021f14 "Bug 1353610 - Add null checks. r=bobbyholley"
merging xpcom/threads/TaskDispatcher.h
 warning: conflicts while merging xpcom/threads/TaskDispatcher.h! (edit, then use 'hg resolve --mark')
abort: unresolved conflicts, can't continue
(use 'hg resolve' and 'hg graft --continue')
---

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Attachment: 1353610_fix_54_aurora.patch

Approval Request Comment
[Feature/Bug causing the regression]:none
[User impact if declined]:A patch to debug null-deref crashes.
[Is this code covered by automated tests?]:yes
[Has the fix been verified in Nightly?]:yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]:none
[Is the change risky?]:low
[Why is the change risky/not risky?]:The change is simple which add some null checks.
[String changes made/needed]:none

Comment 9

[Gerry Chang [:gchang]]

Comment on attachment 8856366 [details] [diff] [review]
1353610_fix_54_aurora.patch

Help debug null-deref crashes. Aurora54+.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/c189e40723ec