[PATCH v2 0/5] MR9749: mmdevapi: Reject wave formats that crash audio drivers.

[Giovanni Mascellani (＠giomasce)]

I am working on a overhaul of how mmdevapi should behave when given invalid or borderline wave formats. In some cases that leads to a crash, which I suppose is appropriate to fix even during the freeze. So here it is.

I'm pretty sure that the OSS backend suffers from the same problem. I could write a patch and check that it builds, but have no way to test it. Should I submit it anyway?

--
  v2: winealsa.drv: Avoid a division by zero.
      winepulse.drv: Avoid a division by zero.
      winecoreaudio.drv: Avoid a division by zero.
      mmdevapi: Reject wave formats with zero block alignment.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/9749

[PATCH v2 1/5] mmdevapi: Reject wave formats with zero samples per seconds.

[Giovanni Mascellani]

From: Giovanni Mascellani <[e-mail]>

That currently triggers an assertion the PulseAudio library.
---
 dlls/mmdevapi/client.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c
index 4538eca9e1c..beb2c12a3df 100644
--- a/dlls/mmdevapi/client.c
+++ b/dlls/mmdevapi/client.c
@@ -388,6 +388,9 @@ static HRESULT validate_wfx(const WAVEFORMATEX *fmt, AUDCLNT_SHAREMODE share_mod
     if (FAILED(ret))
         return ret;
 
+    if (fmt->nSamplesPerSec == 0)
+        ret = E_INVALIDARG;
+
     switch (fmt->wFormatTag) {
         case WAVE_FORMAT_EXTENSIBLE:
             if ((fmt->cbSize != sizeof(WAVEFORMATEXTENSIBLE) - sizeof(WAVEFORMATEX) &&
-- 
GitLab


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

[PATCH v2 2/5] mmdevapi: Reject wave formats with zero block alignment.

[Giovanni Mascellani]

From: Giovanni Mascellani <[e-mail]>

The ALSA driver uses it as a divisor.
---
 dlls/mmdevapi/client.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/dlls/mmdevapi/client.c b/dlls/mmdevapi/client.c
index beb2c12a3df..6eac6b8714a 100644
--- a/dlls/mmdevapi/client.c
+++ b/dlls/mmdevapi/client.c
@@ -388,7 +388,7 @@ static HRESULT validate_wfx(const WAVEFORMATEX *fmt, AUDCLNT_SHAREMODE share_mod
     if (FAILED(ret))
         return ret;
 
-    if (fmt->nSamplesPerSec == 0)
+    if (fmt->nSamplesPerSec == 0 || fmt->nBlockAlign == 0)
         ret = E_INVALIDARG;
 
     switch (fmt->wFormatTag) {
-- 
GitLab


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

[PATCH v2 3/5] winecoreaudio.drv: Avoid a division by zero.

[Giovanni Mascellani]

From: Giovanni Mascellani <[e-mail]>

This can happen if nSamplesPerSec is too small, and it leads to
a crash.
---
 dlls/winecoreaudio.drv/coreaudio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/dlls/winecoreaudio.drv/coreaudio.c b/dlls/winecoreaudio.drv/coreaudio.c
index da2ae2dd67b..3957cf40cf4 100644
--- a/dlls/winecoreaudio.drv/coreaudio.c
+++ b/dlls/winecoreaudio.drv/coreaudio.c
@@ -742,6 +742,13 @@ static NTSTATUS unix_create_stream(void *args)
 
     stream->period = params->period;
     stream->period_frames = muldiv(params->period, stream->fmt->nSamplesPerSec, 10000000);
+
+    if (stream->period_frames == 0)
+    {
+        params->result = E_INVALIDARG;
+        goto end;
+    }
+
     stream->dev_id = dev_id_from_device(params->device);
     stream->flow = params->flow;
     stream->flags = params->flags;
-- 
GitLab


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

[PATCH v2 4/5] winepulse.drv: Avoid a division by zero.

[Giovanni Mascellani]

From: Giovanni Mascellani <[e-mail]>

This can happen if nSamplesPerSec is too small, and it leads to
a crash.
---
 dlls/winepulse.drv/pulse.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/dlls/winepulse.drv/pulse.c b/dlls/winepulse.drv/pulse.c
index 7c8d872c604..8033f0f851b 100644
--- a/dlls/winepulse.drv/pulse.c
+++ b/dlls/winepulse.drv/pulse.c
@@ -1178,6 +1178,12 @@ static NTSTATUS pulse_create_stream(void *args)
                                                                stream->ss.rate,
                                                                10000000);
 
+    if (stream->period_bytes == 0)
+    {
+        hr = E_INVALIDARG;
+        goto exit;
+    }
+
     stream->bufsize_frames = ceil((params->duration / 10000000.) * params->fmt->nSamplesPerSec);
     bufsize_bytes = stream->bufsize_frames * pa_frame_size(&stream->ss);
     stream->mmdev_period_usec = params->period / 10;
-- 
GitLab


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

[PATCH v2 5/5] winealsa.drv: Avoid a division by zero.

[Giovanni Mascellani]

From: Giovanni Mascellani <[e-mail]>

This can happen if nSamplesPerSec is too small, and it leads to
a crash.
---
 dlls/winealsa.drv/alsa.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/dlls/winealsa.drv/alsa.c b/dlls/winealsa.drv/alsa.c
index aea4ddfa326..0fcf180d856 100644
--- a/dlls/winealsa.drv/alsa.c
+++ b/dlls/winealsa.drv/alsa.c
@@ -882,6 +882,12 @@ static NTSTATUS alsa_create_stream(void *args)
     stream->mmdev_period_frames = muldiv(params->fmt->nSamplesPerSec,
             stream->mmdev_period_rt, 10000000);
 
+    if (stream->mmdev_period_frames == 0)
+    {
+        params->result = E_INVALIDARG;
+        goto exit;
+    }
+
     /* Buffer 4 ALSA periods if large enough, else 4 mmdevapi periods */
     stream->alsa_bufsize_frames = stream->mmdev_period_frames * 4;
     if(err < 0 || alsa_period_us < params->period / 10)
-- 
GitLab

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