[PATCH v2 0/1] MR10966: dsound: Fail IDirectSoundCaptureBufferImpl_Lock() for invalid buffer range.

[Paul Gofman (＠gofman)]

--
  v2: dsound: Fail IDirectSoundCaptureBufferImpl_Lock() for invalid buffer range.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10966

[PATCH v2 1/1] dsound: Fail IDirectSoundCaptureBufferImpl_Lock() for invalid buffer range.

[Paul Gofman]

From: Paul Gofman <[e-mail]>

---
 dlls/dsound/capture.c       | 22 ++++++++++++----------
 dlls/dsound/tests/capture.c | 14 ++++++++++++++
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/dlls/dsound/capture.c b/dlls/dsound/capture.c
index dfb5e7d921c..78663caac9f 100644
--- a/dlls/dsound/capture.c
+++ b/dlls/dsound/capture.c
@@ -452,18 +452,20 @@ static HRESULT WINAPI IDirectSoundCaptureBufferImpl_Lock(IDirectSoundCaptureBuff
 
     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 || This->device->buflen - dwReadCusor < dwReadBytes) {
+            *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;
+            if (lplpvAudioPtr2)
+                *lplpvAudioPtr2 = 0;
+            if (lpdwAudioBytes2)
+                *lpdwAudioBytes2 = 0;
         }
     } else {
         TRACE("invalid call\n");
diff --git a/dlls/dsound/tests/capture.c b/dlls/dsound/tests/capture.c
index 63998276a77..6e5adc9e45f 100644
--- a/dlls/dsound/tests/capture.c
+++ b/dlls/dsound/tests/capture.c
@@ -290,6 +290,20 @@ 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)-1,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,state->offset,10 * 1048576,
+                                      &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);
+
     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

[Paul Gofman (＠gofman)]

On Thu May 21 08:19:58 2026 +0000, Huw Davies wrote:
> Could you remove the remaining tabs from this block?  It's already
> somewhat of a mix and your changes make the indentation look very odd on
> when displayed using 8-space tabs.
> v2:

* remove remaining tabs in IDirectSoundCaptureBufferImpl_Lock().

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10966#note_140970

[Matteo Bruni (＠Mystral)]

Matteo Bruni (@Mystral) commented about dlls/dsound/capture.c:
> *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 || This->device->buflen - dwReadCusor < dwReadBytes) {
> +            *lpdwAudioBytes1 = 0;
> +            *lplpvAudioPtr1 = NULL;
> +            if (lplpvAudioPtr2)
> +                *lplpvAudioPtr2 = NULL;
> +            if (lpdwAudioBytes2)
> +                *lpdwAudioBytes2 = 0;
> +            hres = DSERR_INVALIDPARAM;
>          } else {

It doesn't look like we have tests confirming that `IDirectSoundCaptureBuffer::Lock()` never wraps around to the start of the buffer, returning valid data in the `lplpvAudioPtr2`, `lpdwAudioBytes2` out parameters (see e.g. https://learn.microsoft.com/en-us/previous-versions/windows/desktop/ee418179...) for an explanation of how it would work).

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10966#note_140985

[Paul Gofman (＠gofman)]

On Thu May 21 19:46:53 2026 +0000, Matteo Bruni wrote:
> It doesn't look like we have tests confirming that
> `IDirectSoundCaptureBuffer::Lock()` never wraps around to the start of
> the buffer, returning valid data in the `lplpvAudioPtr2`,
> `lpdwAudioBytes2` out parameters (see e.g.
> https://learn.microsoft.com/en-us/previous-versions/windows/desktop/ee418179...)
> for an explanation of how it would work).
> Indeed, thanks, my added test only tests the case when start is invalid and the requested size is larger than the total buffer size, might behave differently for valid start and wraparound. I will test such case and update the patch accoringly.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10966#note_140988