Crashes in _invalid_parameter, inside of nsNotifyAddrListener::calculateNetworkId, on Windows Vista

RESOLVED FIXED in Firefox 49

Status

()

Core
Networking
--
critical
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: dbaron, Assigned: bagder)

Tracking

({crash, topcrash})

Trunk
mozilla49
x86
Windows Vista
crash, topcrash
Points:
---

Firefox Tracking Flags

(firefox49+ fixed)

Details

(Whiteboard: [necko-active], crash signature)

Attachments

(1 attachment)

(Reporter)

Description

a year ago
[Tracking Requested - why for this release]:

This bug was filed from the Socorro interface and is 
report bp-aecf27e1-fc4f-455f-bea3-f15c62160507.
=============================================================

A few crashes like this have started showing up in crash-stats:
https://crash-stats.mozilla.com/signature/?product=Firefox&release_channel=nightly&platform=Windows&date=%3E%3D2016-04-01

So far, all 5 crashes have been on Windows Vista.

Presumably a regression from:
https://hg.mozilla.org/mozilla-central/rev/aa730410c52c
which landed a few days ago.

Top of stack is:
0 	ucrtbase.dll 	_invalid_parameter 	
1 	ucrtbase.dll 	_invalid_parameter_noinfo 	
2 	ucrtbase.dll 	strcpy_s 	
3 	xul.dll 	defaultgw 	netwerk/system/win32/nsNotifyAddrListener.cpp:249
4 	xul.dll 	nsNotifyAddrListener::calculateNetworkId() 	netwerk/system/win32/nsNotifyAddrListener.cpp:274
5 	xul.dll 	nsNotifyAddrListener::Run() 	netwerk/system/win32/nsNotifyAddrListener.cpp:311
Flags: needinfo?(daniel)
(Assignee)

Updated

a year ago
Assignee: nobody → daniel
Flags: needinfo?(daniel)
(Assignee)

Comment 1

a year ago
Out of the three arguments to strcpy_s(), only the third changes between invokes so clearly inet_ntoa() returns NULL in this case. I'll make a patch that makes sure only non-NULL inet_ntoa() results are used.
(Assignee)

Comment 2

a year ago
Created attachment 8750117 [details] [diff] [review]
0001-Bug-1271131-check-inet_ntoa-return-code-before-use-r.patch

1. Check the inet_ntoa() return value before blindly using it, even if I can't really understand why it would fail but evidence suggests that it happens.

2. I modified the logic slightly to avoid the superfluous first strcpy_s()
Attachment #8750117 - Flags: review?(mcmanus)
(Assignee)

Updated

a year ago
Whiteboard: [necko-active]
Attachment #8750117 - Flags: review?(mcmanus) → review+
(Assignee)

Updated

a year ago
Keywords: checkin-needed

Comment 3

a year ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/8389ea050328
Keywords: checkin-needed
tracking-firefox49: ? → +

Comment 4

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8389ea050328
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox49: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in before you can comment on or make changes to this bug.