Closed Bug 1353610 Opened 3 years ago Closed 3 years ago

Add null checks to TaskDispatcher.h

Categories

(Core :: XPCOM, defect, P1, critical)

defect

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: jwwang, Assigned: jwwang)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

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.
Assignee: nobody → jwwang
Blocks: 1342420
Severity: normal → critical
Priority: -- → P1
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.
Attachment #8854676 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/678773021f14
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
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
Attachment #8854676 - Flags: approval-mozilla-aurora?
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-.
Flags: needinfo?(jwwang)
Attachment #8854676 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora-
Yes, the crash is reported in bug 1342420 which however is not a regression I think.
Flags: needinfo?(jwwang)
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')
---
Flags: needinfo?(jwwang)
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
Flags: needinfo?(jwwang)
Attachment #8856366 - Flags: approval-mozilla-aurora?
Comment on attachment 8856366 [details] [diff] [review]
1353610_fix_54_aurora.patch

Help debug null-deref crashes. Aurora54+.
Attachment #8856366 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.