1722073 - AddressSanitizer: stack-use-after-scope [@ `anonymous namespace'::wasapi_find_matching_output_device] with READ of size 8

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, defect, P1)

Description

[Chris Peterson [:cpeterson]]

Steps to reproduce

1. Install a Firefox Nightly ASan build from https://firefox-source-docs.mozilla.org/tools/sanitizer/asan_nightly.html. I'm testing on Windows 10.
2. Set security.sandbox.content.win32k-disable pref = true.
3. Restart Nightly ASan.
4. Open about:support.

Result

The browser crashes 100% of the time.

I don't see a crash reporter UI. Do the Nightly ASan builds automatically submit their crash reports to https://anf1.fuzzing.mozilla.org/crashproxy/submit/ (the asanreporter.apiurl pref value)?

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Toolkit::Crash Reporting' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Andrew McCreight [:mccr8]]

I think it dumps an ASan report into a file but I'm not sure where it is.

Comment 3

[Chris Peterson [:cpeterson]]

This crash requires both ASan and Win32k Lockdown (security.sandbox.content.win32k-disable pref = true).

Comment 4

[Chris Peterson [:cpeterson]]

This is a stack-use-after-scope error in cubeb.

Comment 5

[Chris Peterson [:cpeterson]]

Attachment: ff_asan_log.22284.submitted

ERROR: AddressSanitizer: stack-use-after-scope on address 0x00fe1a67e478 at pc 0x7ff92668d1a1 bp 0x00fe1a67e280 sp 0x00fe1a67e2c8
READ of size 8 at 0x00fe1a67e478 thread T57
    #0 0x7ff92668d1a0 in `anonymous namespace'::wasapi_find_matching_output_device /builds/worker/checkouts/gecko/media/libcubeb/src/cubeb_wasapi.cpp:2284
    #1 0x7ff92668d1a0 in `anonymous namespace'::setup_wasapi_stream /builds/worker/checkouts/gecko/media/libcubeb/src/cubeb_wasapi.cpp:2342
    #2 0x7ff926681a3d in `anonymous namespace'::wasapi_stream_init /builds/worker/checkouts/gecko/media/libcubeb/src/cubeb_wasapi.cpp:2598
    #3 0x7ff92665a949 in cubeb_stream_init /builds/worker/checkouts/gecko/media/libcubeb/src/cubeb.c:357
    #4 0x7ff9297feae3 in cubeb_core::context::ContextRef::stream_init /builds/worker/checkouts/gecko/third_party/rust/cubeb-core/src/context.rs:111
    #5 0x7ff9297feae3 in audioipc_server::server::CubebServer::process_stream_init /builds/worker/checkouts/gecko/third_party/rust/audioipc-server/src/server.rs:790
    #6 0x7ff9297feae3 in audioipc_server::server::CubebServer::process_msg /builds/worker/checkouts/gecko/third_party/rust/audioipc-server/src/server.rs:488
    #7 0x7ff929817085 in audioipc_server::server::{{impl}}::process::{{closure}} /builds/worker/checkouts/gecko/third_party/rust/audioipc-server/src/server.rs:379
    #8 0x7ff929817085 in audioipc_server::server::with_local_context::{{closure}} /builds/worker/checkouts/gecko/third_party/rust/audioipc-server/src/server.rs:202
    #9 0x7ff929817085 in std::thread::local::LocalKey<core::cell::RefCell<core::option::Option<audioipc_server::server::CubebContextState>>>::try_with /rustc/53cb7b09b00cbea8754ffb78e7e3cb521cb8af4b\library\std\src\thread\local.rs:376

Comment 6

[Andrew McCreight [:mccr8]]

Looks like the relevant part is here:

  for (uint32_t i = 0; i < collection.count; i++) {
    cubeb_device_info dev = collection.device[i];
    if (dev.devid == input_device_id) {
      input_device = &dev; // <--- pointer to dev stored in input_device
      break;
    }
  }

Then later input_device is dereferenced.

Comment 7

[Andrew McCreight [:mccr8]]

There's a comment at the start of the function that says "Only try to match to an output device if the input device is a bluetooth device that is using the handsfree protocol". Do you have anything like that on your machine? That might explain why this wasn't detected in automation.

Comment 8

[Chris Peterson [:cpeterson]]

(In reply to Andrew McCreight [:mccr8] from comment #7)

There's a comment at the start of the function that says "Only try to match to an output device if the input device is a bluetooth device that is using the handsfree protocol". Do you have anything like that on your machine? That might explain why this wasn't detected in automation.

Yes. I'm using a Bluetooth headset paired to my machine. I wonder if this bug is what causes my Bluetooth headset to play some static when I open about:support and it enumerates my output devices.

Looks like dev should probably be a pointer to local variable collection.device[i] instead of a copy inside the for loop:

https://searchfox.org/mozilla-central/rev/290f33a3245f61f34ec5f80f548e6584823ba9ad/media/libcubeb/src/cubeb_wasapi.cpp#2246-2247,2275,2277,2285

Comment 9

[Matthew Gregan [:kinetik]]

Attachment: GitHub Pull Request

Comment 10

[Matthew Gregan [:kinetik]]

Attachment: Bug 1722073 - Uplift cubeb fix for WASAPI backend. r?#cubeb-reviewers

Comment 11

[Matthew Gregan [:kinetik]]

Comment on attachment 9233629 [details]
Bug 1722073 - Uplift cubeb fix for WASAPI backend. r?#cubeb-reviewers

Beta/Release Uplift Approval Request

• User impact if declined: Potential crash or unexpected behaviour when using Bluetooth headset on Windows.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very low risk, simple change to avoid using out-of-scope stack variable.
• String changes made/needed: n/a

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Very simple and safe fix for potential crash or unexpected behaviour when using Bluetooth headset on Windows.
• User impact if declined: Potential crash or unexpected behaviour when using Bluetooth headset on Windows.
• Fix Landed on Version: 92
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Very low risk, simple change to avoid using out-of-scope stack variable.
• String or UUID changes made by this patch: n/a

Comment 12

[Pascal Chevrel:pascalc (PTO until April 26)]

92 is fixed via the libecubeb update in bug 1722440

Comment 13

[Pascal Chevrel:pascalc (PTO until April 26)]

Comment on attachment 9233629 [details]
Bug 1722073 - Uplift cubeb fix for WASAPI backend. r?#cubeb-reviewers

Approved for our last 91 beta thanks.

Comment 14

[Pascal Chevrel:pascalc (PTO until April 26)]

https://hg.mozilla.org/releases/mozilla-beta/rev/5b4e2afc51e2

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9233629 [details]
Bug 1722073 - Uplift cubeb fix for WASAPI backend. r?#cubeb-reviewers

This will ride to ESR91 via the Beta uplift already done.

Comment 16

[Catalin Sasca, Desktop QA [:csasca]]

Attachment: asan bt headset.txt

Hey Chris, I was unable to reproduce this issue with a regular nightly build or asan build from (2021-07-23). I tried with two separate bt headphones with both builds and no crash ocurred (I'll attach the about:support data from the asan build) in Windows 10.

Can you please verify it on the latest builds ?

Comment 17

[Chris Peterson [:cpeterson]]

(In reply to Catalin Sasca, QA [:csasca] from comment #16)

Hey Chris, I was unable to reproduce this issue with a regular nightly build or asan build from (2021-07-23). I tried with two separate bt headphones with both builds and no crash ocurred (I'll attach the about:support data from the asan build) in Windows 10.

Can you please verify it on the latest builds ?

Unfortunately, I'm not able to test because I can no longer run ASan builds after I upgraded to Windows 11. I filed bug 1723593 for that issue.

Comment 18

[Chris Peterson [:cpeterson]]

Resolving as fixed. ASan Nightly builds work on Windows 11 now (bug 1723593), so I was able to verify (in Nightly 94) that this bug is fixed.

Comment 19

[BugBot [:suhaib / :marco/ :calixte]]

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.