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

[Paul Gofman (＠gofman)]

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

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

[Paul Gofman]

From: Paul Gofman <[e-mail]>

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

diff --git a/dlls/dsound/capture.c b/dlls/dsound/capture.c
index dfb5e7d921c..1f0aa4aff09 100644
--- a/dlls/dsound/capture.c
+++ b/dlls/dsound/capture.c
@@ -452,12 +452,14 @@ 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 ( dwReadCusor > This->device->buflen || This->device->buflen - dwReadCusor < dwReadBytes) {
+            *lpdwAudioBytes1 = 0;
+            *lplpvAudioPtr1 = NULL;
 	    if (lplpvAudioPtr2)
-            	*lplpvAudioPtr2 = This->device->buffer;
+            *lplpvAudioPtr2 = NULL;
 	    if (lpdwAudioBytes2)
-		*lpdwAudioBytes2 = dwReadBytes - *lpdwAudioBytes1;
+		*lpdwAudioBytes2 = 0;
+	    hres = DSERR_INVALIDPARAM;
         } else {
             *lpdwAudioBytes1 = dwReadBytes;
 	    if (lplpvAudioPtr2)
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)]

That helps Azure Striker Gunvolt 2 which otherwise crashes in the last phase of final boss where a player is required to sing in Japanese and the game starts audio capture through dsound. For some reason it subtracts buffer length from current audio position and initially calls IDirectSoundCaptureBufferImpl_Lock with dwReadCusor of 4294946128. Existing clamp to buffer range doesn't work very well due to integer overflow, while my testing on Windows shows that it shouldn't clamp the output range to buffer size but instead return DSERR_INVALIDPARAM and zero output pointers and lengths.

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

[Huw Davies (＠huw)]

Huw Davies (@huw) commented about dlls/dsound/capture.c:
> if (This->device->client) {
>          *lplpvAudioPtr1 = This->device->buffer + dwReadCusor;
> -        if ( (dwReadCusor + dwReadBytes) > This->device->buflen) {
> -            *lpdwAudioBytes1 = This->device->buflen - dwReadCusor;
> +        if ( dwReadCusor > This->device->buflen || This->device->buflen - dwReadCusor < dwReadBytes) {
> +            *lpdwAudioBytes1 = 0;
> +            *lplpvAudioPtr1 = NULL;
>       if (lplpvAudioPtr2)
> -            	*lplpvAudioPtr2 = This->device->buffer;
> +            *lplpvAudioPtr2 = NULL;
>       if (lpdwAudioBytes2)
> -		*lpdwAudioBytes2 = dwReadBytes - *lpdwAudioBytes1;
> +		*lpdwAudioBytes2 = 0;
> +	    hres = DSERR_INVALIDPARAM;
>          } else {
>              *lpdwAudioBytes1 = dwReadBytes;

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.

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