[PATCH 0/1] MR5588: xaudio2: Use FAudioVoice_DestroyVoiceSafeEXT() in destroy_voice().

[Paul Gofman (＠gofman)]

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5588

[PATCH 1/1] xaudio2: Use FAudioVoice_DestroyVoiceSafeEXT() in destroy_voice().

[Paul Gofman]

From: Paul Gofman <pgofman(a)codeweavers.com>

---
 dlls/xaudio2_7/tests/xaudio2.c | 29 ++++++++++++++++++++++++++++-
 dlls/xaudio2_7/xaudio_dll.c    |  6 +++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/dlls/xaudio2_7/tests/xaudio2.c b/dlls/xaudio2_7/tests/xaudio2.c
index 2b3102d21fe..62f766a827a 100644
--- a/dlls/xaudio2_7/tests/xaudio2.c
+++ b/dlls/xaudio2_7/tests/xaudio2.c
@@ -837,7 +837,9 @@ static void test_submix(IXAudio2 *xa)
     HRESULT hr;
     IXAudio2MasteringVoice *master;
     XAUDIO2_VOICE_DETAILS details;
-    IXAudio2SubmixVoice *sub;
+    IXAudio2SubmixVoice *sub, *sub2;
+    XAUDIO2_SEND_DESCRIPTOR send_desc = { 0 };
+    XAUDIO2_VOICE_SENDS sends = { 1, &send_desc };
 
     IXAudio2_StopEngine(xa);
 
@@ -855,6 +857,31 @@ static void test_submix(IXAudio2 *xa)
     ok(details.InputChannels == 2, "Got wrong channel count: 0x%x\n", details.InputChannels);
     ok(details.InputSampleRate == 44100, "Got wrong sample rate: 0x%x\n", details.InputSampleRate);
 
+    hr = IXAudio2_CreateSubmixVoice(xa, &sub2, 2, 44100, 0, 0, NULL, NULL);
+    ok(hr == S_OK, "CreateSubmixVoice failed: %08lx\n", hr);
+
+    send_desc.pOutputVoice = (IXAudio2Voice *)sub2;
+    hr = IXAudio2SubmixVoice_SetOutputVoices(sub, &sends);
+    ok(hr == S_OK, "CreateSubmixVoice failed: %08lx\n", hr);
+
+    IXAudio2SubmixVoice_DestroyVoice(sub2);
+    /* The voice is not destroyed. */
+    memset(&details, 0xcc, sizeof(details));
+    IXAudio2SubmixVoice_GetVoiceDetails(sub2, &details);
+    ok(details.InputChannels == 2, "Got wrong channel count: 0x%x\n", details.InputChannels);
+    ok(details.InputSampleRate == 44100, "Got wrong sample rate: 0x%x\n", details.InputSampleRate);
+
+    sends.SendCount = 0;
+    hr = IXAudio2SubmixVoice_SetOutputVoices(sub, &sends);
+    ok(hr == S_OK, "CreateSubmixVoice failed: %08lx\n", hr);
+
+    IXAudio2SubmixVoice_DestroyVoice(sub2);
+    if (0)
+    {
+        /* Crashes on Windows and thus suggests that now the voice is actually destroyed. */
+        IXAudio2SubmixVoice_GetVoiceDetails(sub2, &details);
+    }
+
     IXAudio2SubmixVoice_DestroyVoice(sub);
     IXAudio2MasteringVoice_DestroyVoice(master);
 }
diff --git a/dlls/xaudio2_7/xaudio_dll.c b/dlls/xaudio2_7/xaudio_dll.c
index cd1c62f3cd4..a51bb499793 100644
--- a/dlls/xaudio2_7/xaudio_dll.c
+++ b/dlls/xaudio2_7/xaudio_dll.c
@@ -499,7 +499,11 @@ static const FAudioEngineCallback FAudioEngineCallback_Vtbl = {
 
 static inline void destroy_voice(XA2VoiceImpl *This)
 {
-    FAudioVoice_DestroyVoice(This->faudio_voice);
+    if (FAILED(FAudioVoice_DestroyVoiceSafeEXT(This->faudio_voice)))
+    {
+        ERR("Destroying voice %p failed.\n", This);
+        return;
+    }
     free_effect_chain(This->effect_chain);
     This->effect_chain = NULL;
     This->in_use = FALSE;
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/5588

[Paul Gofman (＠gofman)]

On Windows attempt to destroy voice which is the output for another voice does nothing. FAudioVoice_DestroyVoice() currently used is destroying the voice without any checks and return code, so that results in use after free later. FAudioVoice_DestroyVoiceSafeEXT() was added to FAudio to solve this.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5588#note_69709

[PATCH 0/1] MR5588: xaudio2: Use FAudioVoice_DestroyVoiceSafeEXT() in destroy_voice(). - approved

[Huw Davies (＠huw)]

This merge request was approved by Huw Davies.
-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/5588