STR on flame: 1) Flash to m-c 2) Enable data 3) Enable USB tethering See the following error message: E/GeckoConsole( 321): [JavaScript Error: "NS_ERROR_FAILURE: Failure arg 1 [nsINetworkService.enableUsbRndis]" {file: "jar:file:///system/b2g/omni.ja!/components/NetworkManager.js" line: 1024}]

The exception is originated from http://hg.mozilla.org/mozilla-central/file/c9f8cc9ce89c/js/xpconnect/src/XPCWrappedNative.cpp#l2163

Just change the uuid of nsIEnableUsbRndisCallback then it works like a charm... The error path is: XPCWrappedNative::CallMethod CallMethodHelper::Call() ConvertIndependentParams ConvertIndependentParam (i = 1) XPCConvert::JSData2Native JSObject2NativeInterface nsXPCWrappedJS::GetNewOrUsed nsXPCWrappedJSClass::GetNewOrUsed nsXPConnect::XPConnect()->GetInfoForIID(&aIID, getter_AddRefs(info)) where info is nullptr...

QA Wanted for branch checks.

The bug repros on Flame 2.1 and Open C 2.1 Actual result: When activating USB Tethering on the device, the connected PC does not recognize the new connection. Flame 2.1 BuildID: 20140814191115 Gaia: 9979fccc6321be72cd17370f3a20c65bc376af33 Gecko: 9827d3cf6f69 Platform Version: 34.0a1 Firmware Version: V123 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 Open C 2.1 BuildID: 20140814191115 Gaia: 9979fccc6321be72cd17370f3a20c65bc376af33 Gecko: 9827d3cf6f69 Platform Version: 34.0a1 Firmware Version: P821A10v1.0.0B06_LOG_DL User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 -------------------------------------------------------------------------------------------------------- The bug does not repro of Flame 2.0 or Buri 2.1 Actual result: When activating USB Tethering on the device, the connected PC recognizes the new wired connection. Flame 2.0 BuildID: 20140814204728 Gaia: 7fdb37b8a6688ef39ceaa41b56c4a9339802e78e Gecko: 7505293883fb Platform Version: 32.0 Firmware Version: V123 User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0 Buri 2.1 BuildID: 20140814191916 Gaia: 9979fccc6321be72cd17370f3a20c65bc376af33 Gecko: c9f8cc9ce89c Platform Version: 34.0a1 Firmware Version: v1.2device.cfg User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

[Blocking Requested - why for this release]: Bad functionality regression. Requesting a window.

Note: The pushlog for B2G inbound is large due to the fact that the window straddles the bug where the screen goes black right after flashing and none of the apps load properly. Regression window Last working BuildID: 20140801110356 Gaia: 9689218473b6fc4dd927ad6aa7b06c05f0843824 Gecko: ff1e09666c42 Platform Version: 34.0a1 Firmware Version: V123 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 First broken BuildID: 20140801122949 Gaia: 1f20f326f317feda0ec59aff011b54a7c62cb9bf Gecko: 9fa94a0f6c57 Platform Version: 34.0a1 Firmware Version: V123 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 Working Gaia / Broken Gecko = Repro Gaia: 9689218473b6fc4dd927ad6aa7b06c05f0843824 Gecko: 9fa94a0f6c57 Broken Gaia / Working Gecko = No repro Gaia: 1f20f326f317feda0ec59aff011b54a7c62cb9bf Gecko: ff1e09666c42 Gecko push log: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=ff1e09666c42&tochange=9fa94a0f6c57 B2G-inbound Last working BuildID: 20140731035207 Gaia: 29793743960686ee8d5c4617a36172241c8ae0d0 Gecko: a5fc614872dd Platform Version: 34.0a1 Firmware Version: V123 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 First broken BuildID: 20140801100457 Gaia: 22846bf06d34bd42d124952402ab121e6ae05c27 Gecko: f2acc6ff7aee Platform Version: 34.0a1 Firmware Version: V123 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 Working Gaia / Broken Gecko = Repro Gaia: 29793743960686ee8d5c4617a36172241c8ae0d0 Gecko: f2acc6ff7aee Broken Gaia / Working Gecko = No repro Gaia: 22846bf06d34bd42d124952402ab121e6ae05c27 Gecko: a5fc614872dd Gecko pushlog: http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=a5fc614872dd&tochange=f2acc6ff7aee

Chris can you also get the mozilla-inbound perhaps we can get a smaller window by comparing the pushlogs.

Mozilla-inbound Last working BuildID: 20140801121007 Gaia: 9689218473b6fc4dd927ad6aa7b06c05f0843824 Gecko: ceb3128f93e9 Platform Version: 34.0a1 Firmware Version: V123 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 First broken BuildID: 20140801130150 Gaia: 1f20f326f317feda0ec59aff011b54a7c62cb9bf Gecko: a628362a2af8 Platform Version: 34.0a1 Firmware Version: V123 User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0 Working Gaia / Broken Gecko = Repro Gaia: 9689218473b6fc4dd927ad6aa7b06c05f0843824 Gecko: a628362a2af8 Broken Gaia / Working Gecko = No repro Gaia: 1f20f326f317feda0ec59aff011b54a7c62cb9bf Gecko: ceb3128f93e9 Gecko pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=ceb3128f93e9&tochange=a628362a2af8

Not working 39f167a45afc Thinker K.F. Li — Bug 977026 - Part 3: Prelo.... ... eabafb319aae Thinker K.F. Li — Bug 977026 - Part 2: B2G lo.... ... 6bcd4c911e68 Thinker Li — Bug 977026 - Part 1: Allow ... Working 07c91b928cd7 Viral Wang — Bug 932698 - Hold a wakelock when .... One of the patches for Bug 977026 is the regressor.

39f167a45afc is verified to be the regressor and I am looking into it.

[1] and [2] doesn't make mDisplayName a null-terminated string so that [3] would overrun the buffer and the hash table used to store the IID info [4] would be corrupted. The upstream seems to have solved this issue [5]. Will attach the hwcomposer.msm8610.so I rebuilt with fixes (simply strcpy instead strcpy) to verify my argument. $ adb push hwcomposer.msm8610.so system/lib/hw/hwcomposer.msm8610.so and usb tethering will works. [1] https://www.codeaurora.org/cgit/quic/la/platform/hardware/qcom/display/tree/libhwcomposer/hwc_dump_layers.cpp?h=jb_3.2_rb5.60#n61 [2] https://www.codeaurora.org/cgit/quic/la/platform/hardware/qcom/display/tree/libhwcomposer/hwc_dump_layers.cpp?h=jb_3.2_rb5.60#n61 [3] https://www.codeaurora.org/cgit/quic/la/platform/hardware/qcom/display/tree/libhwcomposer/hwc_dump_layers.cpp?h=jb_3.2_rb5.60#n65 [4] http://hg.mozilla.org/mozilla-central/file/ffdd1a398105/xpcom/reflect/xptinfo/XPTInterfaceInfoManager.h#l104 [5] https://www.codeaurora.org/cgit/quic/la/platform/hardware/qcom/display/diff/libhwcomposer/hwc_dump_layers.cpp?id=36bd527bc8a2abba8a003879c46f76bc5cfcdca6

(In reply to Henry Chang [:henry] from comment #11) > [1] and [2] doesn't make mDisplayName a null-terminated string > so that [3] would overrun the buffer and the hash table > used to store the IID info [4] would be corrupted. > > The upstream seems to have solved this issue [5]. > > Will attach the hwcomposer.msm8610.so I rebuilt with > fixes (simply strcpy instead strcpy) to verify my argument. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ simply use strcpy instead of strncpy. (use sizeof(mDisplayName) is also an option)

By the way, since 39f167a45afc change the timing of parsing xpt, XPTInterfaceInfoManager::mWorkingSet:: mIIDTable becomes the victim.

After pushing my modified hwcomposer.so, usb tethering still doesn't work due to http://hg.mozilla.org/mozilla-central/rev/19e59c14b0aa

Partially backed out http://hg.mozilla.org/mozilla-central/rev/19e59c14b0aa to fix usb tethering. Dave, usb tethering was broken due to 19e59c14b0aa and is still not fixed with the latest m-c. Backing out AutoMounter only is good enough to fix usb tethering so there might be some issue regarding MTP/Mass storage/USB coexistence. Do you have any comment on this issue? Thanks! By the way, since usb tethering is also broken by an gonk issue (HwcDebug buffer overflow), you have to push the attached hwcomposer.msm8610.so to the device first. STR: 1) Flash flame with v123 base image the flash the latest gecko. 2) Push the attachment hwcomposer.msm8610.so to system/lib/hw 3) Settings => Internet sharing => usb tethering At this time usb tethering will not work 4) Apply patch, rebuild and reflash gecko. Usb tethering will work now.

I updated the AutoMounter state machine to accomodate rndis, so usb tethering should work properly now.

(In reply to Henry Chang [:henry] from comment #13) > (In reply to Henry Chang [:henry] from comment #11) > > [1] and [2] doesn't make mDisplayName a null-terminated string > > so that [3] would overrun the buffer and the hash table > > used to store the IID info [4] would be corrupted. > > > > The upstream seems to have solved this issue [5]. > > > > Will attach the hwcomposer.msm8610.so I rebuilt with > > fixes (simply strcpy instead strcpy) to verify my argument. > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > simply use strcpy instead of strncpy. (use sizeof(mDisplayName) is also an > option) We should NEVER use strcpy or strcat. Generally speaking these are what attackers look for when trying to find buffer overflow attacks. You're MUCH better off to use strlcpy/strlcat.

(In reply to Dave Hylands [:dhylands] from comment #18) > (In reply to Henry Chang [:henry] from comment #13) > > (In reply to Henry Chang [:henry] from comment #11) > > > [1] and [2] doesn't make mDisplayName a null-terminated string > > > so that [3] would overrun the buffer and the hash table > > > used to store the IID info [4] would be corrupted. > > > > > > The upstream seems to have solved this issue [5]. > > > > > > Will attach the hwcomposer.msm8610.so I rebuilt with > > > fixes (simply strcpy instead strcpy) to verify my argument. > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > simply use strcpy instead of strncpy. (use sizeof(mDisplayName) is also an > > option) > > We should NEVER use strcpy or strcat. Generally speaking these are what > attackers look for when trying to find buffer overflow attacks. > > You're MUCH better off to use strlcpy/strlcat. The upstream uses strlcpy instead. I am just making a temporarily fix to make it work for testing :p Thanks!

Comment on attachment 8483179 [details] [diff] [review] AutoMounter should accomodate rndis Review of attachment 8483179 [details] [diff] [review]: ----------------------------------------------------------------- This patch is verified to fix broken usb tethering! Good job! But I think I am not familiar with AutoMounter enough to review this patch. Forwarding to Eric who is the original reviewer. Thanks!

Kyle, did you mean to tag this bug as Graphics:Layers?

Reclassifying. This is definitely Firefox OS::General

(In reply to Milan Sreckovic [:milan] from comment #21) > Kyle, did you mean to tag this bug as Graphics:Layers? No, I didn't. I probably left the bug open too long before CCing myself.

Comment on attachment 8483179 [details] [diff] [review] AutoMounter should accomodate rndis Review of attachment 8483179 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me. The original UMS/MTP functionality doesn't seem to be affected and the patch has already been verified by Henry.

Based on the patch I'm going to assume that this wasn't actually Nuwa related.

(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #27) > Based on the patch I'm going to assume that this wasn't actually Nuwa > related. Nope - this was related to the way MTP got integrated into the AutoMounter, and as you surmised had absolutely nothing to do with Nuwa.

Triage reviewed, major functional regression -> blocking+.

Please request Aurora approval on this patch when you get a chance.

Comment on attachment 8483179 [details] [diff] [review] AutoMounter should accomodate rndis Since the regressor 19e59c14b0aa [1] is in the mozilla-aurora tree, the patch needs to be uplifted. http://hg.mozilla.org/releases/mozilla-aurora/rev/19e59c14b0aa Approval Request Comment [Feature/regressing bug #]: USB tethering [User impact if declined]: USB tethering won't work [Describe test coverage new/current, TBPL]: Manually testing [Risks and why]: No risk [String/UUID change made/needed]: No

Verified as fixed for the latest 2.1 Flame build: Environmental Variables: ---------------------------------------- Device: Flame 2.1 BuildID: 20141011000201 Gaia: f5d4ff60ffed8961f7d0380ada9d0facfdfd56b1 Gecko: d813d79d3eae Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf Version: 34.0a2 (2.1) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 When user enables USB tethering on their device, a connection is formed and internet tethering functions as expected - This issue is NOT fixed for the latest 2.2 Flame build: Environmental Variables: ---------------------------------------- Device: Flame 2.2 Master BuildID: 20141011040204 Gaia: 95f580a1522ffd0f09302372b78200dab9b6f322 Gecko: 3f6a51950eb5 Gonk: 52c909e821d107d414f851e267dedcd7aae2cebf Version: 35.0a1 (2.2 Master) Firmware: V180 User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0 When enabling the USB tethering, the toggle animates from "on" to "off" every time meaning it cannot be enabled. Updating tracking flags to reflect current behaviour.

(In reply to Roland Kunkel [:RolandK] from comment #33) > Verified as fixed for the latest 2.1 Flame build: > This issue is NOT fixed for the latest 2.2 Flame build: After re-flashing my device and verifying on 3 separate devices, this feature is fixed for the latest 2.2 Flame device. Verified against all affected branches, changing bug status to "Verified Fixed"