Closed Bug 1246850 Opened 4 years ago Closed 4 years ago

Missing check for return code in call to NotifyIpInterfaceChange

Categories

(Core :: Networking, defect)

Unspecified
Windows Vista
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox45 --- fixed
firefox46 --- fixed
firefox47 --- fixed

People

(Reporter: aklotz, Assigned: bagder)

Details

(Keywords: crash, Whiteboard: [44dll][necko-active])

Crash Data

Attachments

(1 file)

We're not checking the return code from NotifyIpInterfaceChange, resulting in us potentially passing a bad handle to CancelMibChangeNotify2.

I'm trying to eliminate possible triggers of crash stacks such as:
https://crash-stats.mozilla.com/report/index/889b5884-fdbe-43c2-800d-2984f2160201

and this one look like a candidate. Yes, it has been in the code longer than 44, but nonetheless...
Flags: needinfo?(daniel)
Ack. Patch pending.
Assignee: nobody → daniel
Component: Networking: DNS → Networking
Flags: needinfo?(daniel)
Check the return code from NotifyIpInterfaceChange(). It probably indicates a pretty serious error somewhere if it returns error here - but right now this will only log about it to avoid crashes and return.
Attachment #8717357 - Flags: review?(mcmanus)
Attachment #8717357 - Flags: review?(mcmanus) → review+
thanks aaron
Whiteboard: [44dll]
Whiteboard: [44dll] → [44dll[necko-active]
Whiteboard: [44dll[necko-active] → [44dll][necko-active]
https://hg.mozilla.org/mozilla-central/rev/0a47d0927f3d
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Can you please nominate this fix for uplift?
Flags: needinfo?(daniel)
Comment on attachment 8717357 [details] [diff] [review]
0001-bug-1246850-check-the-NotifyIpInterfaceChange-return.patch

Approval Request Comment
[Feature/regressing bug #]: 1246850
[User impact if declined]: This is suspected to be the source of a reported crash: https://crash-stats.mozilla.com/report/index/889b5884-fdbe-43c2-800d-2984f2160201
[Describe test coverage new/current, TreeHerder]: There's no particular test for this code branch.
[Risks and why]: If this code path is triggered, it rather points to an error condition elsewhere. This patch could then be a first whack-a-mole that makes the problem hit someplace else. The patch is tiny.
[String/UUID change made/needed]:
Flags: needinfo?(daniel)
Attachment #8717357 - Flags: approval-mozilla-release?
Attachment #8717357 - Flags: approval-mozilla-beta?
Attachment #8717357 - Flags: approval-mozilla-aurora?
Comment on attachment 8717357 [details] [diff] [review]
0001-bug-1246850-check-the-NotifyIpInterfaceChange-return.patch

Fix a crash, taking it.
Should be in 45 beta 5.
Attachment #8717357 - Flags: approval-mozilla-beta?
Attachment #8717357 - Flags: approval-mozilla-beta+
Attachment #8717357 - Flags: approval-mozilla-aurora?
Attachment #8717357 - Flags: approval-mozilla-aurora+
Crash Signature: [@ RtlFreeHeap | FreeWrapper]
Keywords: crash
Comment on attachment 8717357 [details] [diff] [review]
0001-bug-1246850-check-the-NotifyIpInterfaceChange-return.patch

No longer relevant
Attachment #8717357 - Flags: approval-mozilla-release? → approval-mozilla-release-
You need to log in before you can comment on or make changes to this bug.