870652 - crash in mozilla::net::NetAddrToString

Status: RESOLVED FIXED

(Core :: Networking: DNS, defect)

Description

[Mats Palmgren (inactive)]

This bug was filed from the Socorro interface and is 
report bp-3edba9ae-91a6-4b79-b929-bc3fd2130505 .
============================================================= 

http://hg.mozilla.org/mozilla-central/annotate/a6104e0e5a2c/netwerk/dns/nsDNSService2.cpp#l233
  mHostRecord->ReportUnusable(&mIter->mAddress);

http://hg.mozilla.org/mozilla-central/annotate/a6104e0e5a2c/netwerk/dns/DNS.cpp#l99
  if (addr->raw.family == AF_INET) {

The crash data indicates that 'mIter' is NULL.


More crashes:
https://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=contains&query=mozilla%3A%3Anet%3A%3ANetAddrToString&reason_type=contains&range_value=4&range_unit=weeks&hang_type=any&process_type=any&do_query=1&signature=mozilla%3A%3Anet%3A%3ANetAddrToString%28mozilla%3A%3Anet%3A%3ANetAddr%20const*%2C%20char*%2C%20unsigned%20int%29

3,080 crash reports in the past 4 week period, all on Windows.

Comment 1

[Steve Workman [:sworkman] (INACTIVE)]

I see several comments relating to the host machine either going to sleep or coming out of sleep. Suggest to me it could be the addr_info changing. However, the boolean condition (mIterGenCnt == mHostRecord->addr_info_gencnt) should detect that.

Patrick, does this sound reasonable? Any ideas why addr_info_gencnt wouldn't be updated?

A simple null check for mIter in nsDNSRecord::ReportUnusable and/or for aAddress in nsHostRecord::ReportUnusable should avoid the crash. But it would be better to know why this is happening.

Comment 2

[Patrick McManus [:mcmanus]]

I think this is a regression from 807678.. if you look at the changes to ReportUnusable() made in that patch..

there clearly are cases in getnextaddr() where mIter could be nullptr with a valid (and unchanging) gencnt.

I'm not 100% sure but I think the nullptr check is probably the right thing in this case.

Comment 3

[Steve Workman [:sworkman] (INACTIVE)]

Attachment: v1.0 Add null ptr check in nsDNSRecord::ReportUnusable to avoid crashing

Crash comments don't yield great STR, so just did regression testing using network xpcshell tests locally. All passing (except test_protocolproxyservice, but that fails without the patch too).

Comment 4

[Steve Workman [:sworkman] (INACTIVE)]

Thanks, Pat!

https://hg.mozilla.org/integration/mozilla-inbound/rev/1be2d9024641

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1be2d9024641