920361 - [B2G] Network Manager: passes null string to stringToIP() while enabling/disabling WiFi

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Ethan Tseng [:ethan]]

This bug is generated from bug 915529.

https://bugzilla.mozilla.org/show_bug.cgi?id=915529#c1
As the comment said, stringToIP() in systemlibs.js would receive null string when we enable or disable WiFi.

We decided to only add defensive programming for stringToIP() in bug 915529.
This new bug is dedicated to locate the origin of null string, discover any possible impact and resolve it.

Comment 1

[Ethan Tseng [:ethan]]

Debug Notes - Reproduce null string problem while enalbing/disabling Wifi on Unagi 1.3.

Device: Unagi
OS version: 1.3.0.0-prerelease
Platform version: 27.0a1
Build ID: 20130927151304

Log:
/* Turn off Wifi */
09-27 17:12:10.255 I/Gecko   (  973): -*- NetworkManager: NetworkManager received message from worker: {"id":"broadcast","topic":"netd-interface-change","reason":"Iface linkstate wlan0 down"}
09-27 17:12:10.275 I/Gecko   (  973): -*- NetworkManager: Network wlan0 changed state to 3
09-27 17:12:10.275 I/Gecko   (  973): *** stringToIP is called ***
09-27 17:12:10.275 I/Gecko   (  973): stringToIP: string is null
09-27 17:12:10.275 I/Gecko   (  973): stringToIP@resource://gre/modules/systemlibs.js:461,removeNetworkRoute@resource://gre/modules/net_worker.js:286,onmessage@resource://gre/modules/net_worker.js:210
09-27 17:12:10.285 I/Gecko   (  973): -*- NetworkManager: Evaluating whether active network needs to be changed.
09-27 17:12:10.375 I/Gecko   (  973): -*- NetworkManager: Received error from worker: resource://gre/modules/systemlibs.js:472: string is null
09-27 17:12:10.385 I/Gecko   (  973): -*- NetworkManager: NetworkManager received message from worker: {"id":"broadcast","topic":"netd-interface-change","reason":"Iface linkstate wlan0 down"}

/* Turn on Wifi */
09-27 17:12:16.881 I/Gecko   (  973): -*- NetworkManager: Network wlan0 changed state to 3
09-27 17:12:16.881 I/Gecko   (  973): *** stringToIP is called ***
09-27 17:12:16.881 I/Gecko   (  973): stringToIP: string is null
09-27 17:12:16.881 I/Gecko   (  973): stringToIP@resource://gre/modules/systemlibs.js:461,removeNetworkRoute@resource://gre/modules/net_worker.js:286,onmessage@resource://gre/modules/net_worker.js:210
09-27 17:12:16.891 I/Gecko   (  973): -*- NetworkManager: Evaluating whether active network needs to be changed.
09-27 17:12:16.891 I/Gecko   (  973): -*- NetworkManager: setWifiOperationMode on wlan0 to STA
09-27 17:12:16.941 I/Gecko   (  973): -*- NetworkManager: Received error from worker: resource://gre/modules/systemlibs.js:472: string is null
09-27 17:12:16.941 I/Gecko   (  973): -*- NetworkManager: NetworkManager received message from worker: {"cmd":"setWifiOperationMode","ifname":"wlan0","mode":"STA","report":true,"isAsync":true,"id":3,"resultCode":200,"resultReason":"Softap operation succeeded"}
...
09-27 17:12:20.385 I/Gecko   (  973): -*- NetworkManager: NetworkManager received message from worker: {"id":"broadcast","topic":"netd-interface-change","reason":"Iface linkstate wlan0 up"}
09-27 17:12:20.855 I/Gecko   (  973): -*- NetworkManager: Network wlan0 changed state to 1
09-27 17:12:20.855 I/Gecko   (  973): -*- NetworkManager: Remove default route for wlan0
09-27 17:12:20.855 I/Gecko   (  973): -*- NetworkManager: Evaluating whether active network needs to be changed.
09-27 17:12:20.855 I/Gecko   (  973): -*- NetworkManager: Found our preferred type of network: wlan0
09-27 17:12:20.855 I/Gecko   (  973): -*- NetworkManager: Going to change route and DNS to wlan0
09-27 17:12:20.855 I/Gecko   (  973): *** stringToIP is called ***
09-27 17:12:20.855 I/Gecko   (  973): stringToIP: string = [10.247.24.1]
09-27 17:12:20.855 I/Gecko   (  973): stringToIP: return ip = 18413322
09-27 17:12:20.865 I/Gecko   (  973): -*- NetworkManager: No proxy support for wlan0 network interface.
09-27 17:12:20.885 I/Gecko   (  973): -*- NetworkManager: Usb tethering settings is not enabled
...
09-27 17:12:25.369 I/Gecko   (  973): -*- NetworkManager: NetworkManager received message from worker: {"id":"broadcast","topic":"netd-interface-change","reason":"Iface linkstate wlan0 up"}

Comment 2

[Ethan Tseng [:ethan]]

When we turn off Wifi, the call flow within NetworkManager.js is:
1. Network Manager was notified by the event "network-interface-state-changed". State was changed from connected to disconnected.
2. Calls member function of NetworkManager.js "resetRoutingTable()", which in turn posts message "removeNetworkRoute" to net_worker.js
3. net_work.js::removeNetworkRoute() calls system-level functions exposed from systemlibs.js, including:
   netHelpers.stringToIP
   netHelpers.getMaskLength
   libnetutils.ifc_remove_default_route
   libnetutils.ifc_remove_route
In this case, ip and netmask arguments for resetRoutingTable() are both null, which would be passed into systemlibs.js::stringToIP() and caused JavaScript exception.

When we turn on Wifi, the stringToIP() would be invoked twice.
The first time is invoked from NetworkManager.js::resetRoutingTable(), just like the case of turning off Wifi. This would cause the same exception again.
The second time is invoked from NetworkManager.js::setDefaultRouteAndDNS(). This is okay.

Comment 3

[Ethan Tseng [:ethan]]

Attachment: Add error handle in NetworkManager.js::resetRoutingTable

Comment 4

[Vincent Chang[:vchang][changyihsin]]

Comment on attachment 812489 [details] [diff] [review]
Add error handle in NetworkManager.js::resetRoutingTable

Review of attachment 812489 [details] [diff] [review]:
-----------------------------------------------------------------

Look good. Thank you.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/e4c283055041

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e4c283055041