[PATCH 0/5] MR9749: mmdevapi: Reject wave formats that crash audio drivers.
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? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9749
From: Giovanni Mascellani <gmascellani@codeweavers.com> 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
From: Giovanni Mascellani <gmascellani@codeweavers.com> 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
From: Giovanni Mascellani <gmascellani@codeweavers.com> 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
From: Giovanni Mascellani <gmascellani@codeweavers.com> 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
From: Giovanni Mascellani <gmascellani@codeweavers.com> 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
Is it possible to consolidate those checks in mmdevapi, and not duplicating them for each driver? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9749#note_125376
On Wed Dec 10 20:54:37 2025 +0000, Nikolay Sivov wrote:
Is it possible to consolidate those checks in mmdevapi, and not duplicating them for each driver? The driver-specific checks are different for each driver, and they depend on values computed in driver-specific code, so I see no way of consolidating them in the same place. There is probably room for making all drivers share more code (and that's likely to be a good thing to do), but that's a much bigger thing which I'm not planning to do, and that doesn't look appropriate for code freeze. Here I'm trying to keep the changes focused on fixing a specific crash.
One the freeze is over I plan to upstream more code dealing with validating the wave format data, which we currently a bit differently from Windows and that shows in some applications. But that's not specifically focused on preventing crashes nor sharing code between drivers. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9749#note_125411
As usual, it really doesn't looke like the test failures are related to my changes. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/9749#note_125412
participants (3)
-
Giovanni Mascellani -
Giovanni Mascellani (＠giomasce)
-
Nikolay Sivov (＠nsivov)