[PATCH v4 0/1] MR10966: dsound: Fail IDirectSoundCaptureBufferImpl_Lock() for invalid buffer range.
-- v4: dsound: Fail IDirectSoundCaptureBufferImpl_Lock() for invalid buffer range. https://gitlab.winehq.org/wine/wine/-/merge_requests/10966
From: Paul Gofman <pgofman@codeweavers.com> --- dlls/dsound/capture.c | 26 +++++++++--------- dlls/dsound/tests/capture.c | 53 +++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 12 deletions(-) diff --git a/dlls/dsound/capture.c b/dlls/dsound/capture.c index dfb5e7d921c..2112f5dfe86 100644 --- a/dlls/dsound/capture.c +++ b/dlls/dsound/capture.c @@ -451,19 +451,21 @@ static HRESULT WINAPI IDirectSoundCaptureBufferImpl_Lock(IDirectSoundCaptureBuff EnterCriticalSection(&(This->device->lock)); if (This->device->client) { - *lplpvAudioPtr1 = This->device->buffer + dwReadCusor; - if ( (dwReadCusor + dwReadBytes) > This->device->buflen) { - *lpdwAudioBytes1 = This->device->buflen - dwReadCusor; - if (lplpvAudioPtr2) - *lplpvAudioPtr2 = This->device->buffer; - if (lpdwAudioBytes2) - *lpdwAudioBytes2 = dwReadBytes - *lpdwAudioBytes1; + if ( dwReadCusor >= This->device->buflen || !dwReadBytes || dwReadBytes > This->device->buflen) { + *lpdwAudioBytes1 = 0; + *lplpvAudioPtr1 = NULL; + if (lplpvAudioPtr2) + *lplpvAudioPtr2 = NULL; + if (lpdwAudioBytes2) + *lpdwAudioBytes2 = 0; + hres = DSERR_INVALIDPARAM; } else { - *lpdwAudioBytes1 = dwReadBytes; - if (lplpvAudioPtr2) - *lplpvAudioPtr2 = 0; - if (lpdwAudioBytes2) - *lpdwAudioBytes2 = 0; + *lplpvAudioPtr1 = This->device->buffer + dwReadCusor; + *lpdwAudioBytes1 = min(dwReadBytes, This->device->buflen - dwReadCusor); + if (lpdwAudioBytes2) + *lpdwAudioBytes2 = dwReadBytes - *lpdwAudioBytes1; + if (lplpvAudioPtr2) + *lplpvAudioPtr2 = dwReadBytes - *lpdwAudioBytes1 ? This->device->buffer : NULL; } } else { TRACE("invalid call\n"); diff --git a/dlls/dsound/tests/capture.c b/dlls/dsound/tests/capture.c index 63998276a77..fc05b8ae745 100644 --- a/dlls/dsound/tests/capture.c +++ b/dlls/dsound/tests/capture.c @@ -290,6 +290,59 @@ static BOOL capture_buffer_service(capture_state_t* state) if (rc!=DS_OK) return FALSE; + ptr1 = ptr2 = (void *)0xdeadbeef; + len1 = len2 = 0xdeadbeef; + rc=IDirectSoundCaptureBuffer_Lock(state->dscbo,(DWORD)-32,0,&ptr1,&len1,&ptr2,&len2,0); + ok(rc==DSERR_INVALIDPARAM,"IDirectSoundCaptureBuffer_Lock() failed: %08lx\n", rc); + ok(!ptr1 && !ptr2 && !len1 && !len2, "got %p, %lu, %p, %lu.\n", ptr1, len1, ptr2, len2); + + ptr1 = ptr2 = (void *)0xdeadbeef; + len1 = len2 = 0xdeadbeef; + rc=IDirectSoundCaptureBuffer_Lock(state->dscbo,(DWORD)-32,64,&ptr1,&len1,&ptr2,&len2,0); + ok(rc==DSERR_INVALIDPARAM,"IDirectSoundCaptureBuffer_Lock() returned: %08lx\n", rc); + ok(!ptr1 && !ptr2 && !len1 && !len2, "got %p, %lu, %p, %lu.\n", ptr1, len1, ptr2, len2); + + ptr1 = ptr2 = (void *)0xdeadbeef; + len1 = len2 = 0xdeadbeef; + rc=IDirectSoundCaptureBuffer_Lock(state->dscbo,0,0,&ptr1,&len1,&ptr2,&len2,0); + ok(rc==DSERR_INVALIDPARAM,"IDirectSoundCaptureBuffer_Lock() returned: %08lx\n", rc); + ok(!ptr1 && !ptr2 && !len1 && !len2, "got %p, %lu, %p, %lu.\n", ptr1, len1, ptr2, len2); + + ptr1 = ptr2 = (void *)0xdeadbeef; + len1 = len2 = 0xdeadbeef; + rc=IDirectSoundCaptureBuffer_Lock(state->dscbo,0,0,&ptr1,&len1,NULL,NULL,0); + ok(rc==DSERR_INVALIDPARAM,"IDirectSoundCaptureBuffer_Lock() returned: %08lx\n", rc); + ok(!ptr1 && !len1, "got %p, %lu.\n", ptr1, len1); + + ptr1 = ptr2 = (void *)0xdeadbeef; + len1 = len2 = 0xdeadbeef; + rc=IDirectSoundCaptureBuffer_Lock(state->dscbo,state->buffer_size,1,&ptr1,&len1,&ptr2,&len2,0); + ok(rc==DSERR_INVALIDPARAM,"IDirectSoundCaptureBuffer_Lock() returned: %08lx\n", rc); + ok(!ptr1 && !ptr2 && !len1 && !len2, "got %p, %lu, %p, %lu.\n", ptr1, len1, ptr2, len2); + + ptr1 = ptr2 = (void *)0xdeadbeef; + len1 = len2 = 0xdeadbeef; + rc=IDirectSoundCaptureBuffer_Lock(state->dscbo,0,state->buffer_size + 1,&ptr1,&len1,&ptr2,&len2,0); + ok(rc==DSERR_INVALIDPARAM,"IDirectSoundCaptureBuffer_Lock() returned: %08lx\n", rc); + ok(!ptr1 && !ptr2 && !len1 && !len2, "got %p, %lu, %p, %lu.\n", ptr1, len1, ptr2, len2); + + ptr1 = ptr2 = (void *)0xdeadbeef; + len1 = len2 = 0xdeadbeef; + rc=IDirectSoundCaptureBuffer_Lock(state->dscbo,state->offset,state->buffer_size, + &ptr1,&len1,&ptr2,&len2,0); + ok(!rc,"IDirectSoundCaptureBuffer_Lock() returned: %08lx\n", rc); + if (state->offset) + ok(ptr1 == (char *)ptr2 + state->offset, "got %p, %p, buffer_size %ld, offset %ld.\n", + ptr1, ptr2, state->buffer_size, state->offset); + else + ok(!ptr2, "got %p.\n", ptr2); + ok(len1 == state->buffer_size - state->offset, "got %ld, buffer_size %ld, offset %ld.\n", + len1, state->buffer_size, state->offset); + ok(len2 == state->offset, "got %ld, buffer_size %ld, offset %ld.\n", + len2, state->buffer_size, state->offset); + rc=IDirectSoundCaptureBuffer_Unlock(state->dscbo,ptr1,len1,ptr2,len2); + ok(rc==DS_OK,"IDirectSoundCaptureBuffer_Unlock() returned: %08lx\n", rc); + rc=IDirectSoundCaptureBuffer_Lock(state->dscbo,state->offset,state->size, &ptr1,&len1,&ptr2,&len2,0); ok(rc==DS_OK,"IDirectSoundCaptureBuffer_Lock() failed: %08lx\n", rc); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10966
On Mon Jun 1 15:03:17 2026 +0000, Paul Gofman wrote:
changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/10966/diffs?diff_id=271863&start_sha=2778b3bd49dc8510e673cff8676712fbe46a0ee9#da86c716fd800a9217d100954315e8131faeb6fd_333_323) Oh yeah, most likely. I described above the situation with NULL ptr2 / len2 and why I didn't add those (while tested with the described workaround).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/10966#note_141873
v3: * remove duplicate tests. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10966#note_141874
This merge request was approved by Matteo Bruni. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10966
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10966
participants (4)
-
Huw Davies (＠huw) -
Matteo Bruni (＠Mystral)
-
Paul Gofman
-
Paul Gofman (＠gofman)