768394 - B2G: Uninitialised value uses after "ifconfig wlan0 down ; ifconfig wlan0 up"

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect, P1)

Description

[Julian Seward [:jseward]]

Attachment: error log from valgrind/memcheck

Pretty sure XPCOM is the wrong component, but not sure what the right
one is.

This is completely repeatable.  Bring up b2g, let it idle, then on the
phone do "ifconfig wlan0 down ; ifconfig wlan0 up".  Valgrind
immediately reports the errors in the attachment.  This can also
happen by itself sometimes -- I think if the WiFi connection is flaky,
then Gecko messes with the iface of its own accord.  Or something.

I peered at ffi_call_SYSV but was immediately lost in a maze of
FFIery.

Note that some of the stacks -- those containing ffi_call_SYSV -- have
been obtained by stack scanning rather than by the usual CFI based
unwinding, and so may contain bogus frames -- they should be regarded
with some suspicion.  Stacks not including ffi_call_SYSV are reliable.

Comment 1

[Julian Seward [:jseward]]

There's an inconsistency between this, in gecko/dom/system/gonk/systemlibs.js

    ifc_reset_connections: library.declare("ifc_reset_connections",
                                           ctypes.default_abi,
                                           ctypes.int,
                                           ctypes.char.ptr),

and the function it refers to, in system/core/libnetutils/ifc_utils.c

  int ifc_reset_connections(const char *ifname, const int reset_mask)

The JS bit omits the second arg (const int reset_mask) and presumably
passes junk, which is consistent with the complaints valgrind produces
-- it complains about tests on that second arg:

    if (reset_mask & RESET_IPV4_ADDRESSES) {

and

    if (reset_mask & RESET_IPV6_ADDRESSES) {

Fixing the decl above is obviously trivial, but not sure where to look
for the JS call sites.  The only place I can find is in
gecko/dom/wifi/WifiWorker.js

  function resetConnections(ifname, callback) {
    controlMessage({ cmd: "ifc_reset_connections", ifname: ifname }, function(data) {
      callback(!data.status);
    });
  }

I don't want to pass zero as the new second arg though, because that
will make ifc_reset_connections do nothing -- it is of the form

    if (reset_mask & RESET_IPV4_ADDRESSES) {
       // nuke IPV4 connections
    }
    if (reset_mask & RESET_IPV6_ADDRESSES) {
       // nuke IPV6 connections
    }

Please advise.  (I'd be willing to test a patch!)

I also checked the JS-vs-C consistency of the rest of the ifc_*
functions mentioned in systemlib.js, and they look OK.

Comment 2

[Dietrich Ayala (:dietrich)]

Can cause crashes, per mrbkap.

Comment 3

[Andreas Gal :gal]

How are we doing here?

Comment 4

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Fix

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 656942 [details] [diff] [review]
Fix

Julian, I'd appreciate it if you could re-check with this patch (I'll probably check this in sooner rather than later).

Comment 6

[Andreas Gal :gal]

Comment on attachment 656942 [details] [diff] [review]
Fix

Stealing.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

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