Chip Davis : ntoskrnl: Always copy the output buffer for non-buffered ioctls.

[Alexandre Julliard]

Module: wine
Branch: master
Commit: 7c0f6420059974ab127ae944b55d11dd71df3b85
URL:    https://source.winehq.org/git/wine.git/?a=commit;h=7c0f6420059974ab127ae944b...

Author: Chip Davis 
Date:   Tue Sep 14 01:02:45 2021 -0500

ntoskrnl: Always copy the output buffer for non-buffered ioctls.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=30155
Signed-off-by: Zebediah Figura 
Signed-off-by: Alexandre Julliard

---

dlls/ntoskrnl.exe/ntoskrnl.c       | 51 +++++++++++++++++++++++++++-----------
 dlls/ntoskrnl.exe/tests/ntoskrnl.c | 21 ++++++----------
 2 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/dlls/ntoskrnl.exe/ntoskrnl.c b/dlls/ntoskrnl.exe/ntoskrnl.c
index 1bd5d6fb351..f2fb0a6d66e 100644
--- a/dlls/ntoskrnl.exe/ntoskrnl.c
+++ b/dlls/ntoskrnl.exe/ntoskrnl.c
@@ -440,15 +440,43 @@ static void free_dispatch_irp( struct irp_data *irp_data )
     free( irp_data );
 }

+static ULONG get_irp_output_size( IRP *irp )
+{
+    IO_STACK_LOCATION *stack = IoGetNextIrpStackLocation( irp );
+
+    if (!irp->UserBuffer || (irp->Flags & IRP_WRITE_OPERATION))
+        return 0;
+
+    /* For IRPs not using buffered I/O, the driver is supposed to have direct
+     * access to the user's output buffer, either via an MDL (direct I/O) or
+     * with the raw user VA (neither). We can't fully support this, but we
+     * should at least copy the entire buffer back to the caller. */
+    switch (stack->MajorFunction)
+    {
+        case IRP_MJ_FILE_SYSTEM_CONTROL:
+        case IRP_MJ_DEVICE_CONTROL:
+        case IRP_MJ_INTERNAL_DEVICE_CONTROL:
+            if ((stack->Parameters.DeviceIoControl.IoControlCode & 3) != METHOD_BUFFERED)
+                return stack->Parameters.DeviceIoControl.OutputBufferLength;
+            break;
+
+        case IRP_MJ_READ:
+            /* FIXME: Handle non-buffered reads. */
+        default:
+            break;
+    }
+
+    if (NT_ERROR(irp->IoStatus.u.Status))
+        return 0;
+    return irp->IoStatus.Information;
+}
+
 /* transfer result of IRP back to wineserver */
 static NTSTATUS WINAPI dispatch_irp_completion( DEVICE_OBJECT *device, IRP *irp, void *context )
 {
     struct irp_data *irp_data = context;
-    void *out_buff = irp->UserBuffer;
     NTSTATUS status;
-
-    if (irp->Flags & IRP_WRITE_OPERATION)
-        out_buff = NULL;  /* do not transfer back input buffer */
+    ULONG out_size;

EnterCriticalSection( &irp_completion_cs );

@@ -460,15 +488,14 @@ static NTSTATUS WINAPI dispatch_irp_completion( DEVICE_OBJECT *device, IRP *irp,
         return STATUS_MORE_PROCESSING_REQUIRED;
     }

+    out_size = get_irp_output_size( irp );
+
     SERVER_START_REQ( set_irp_result )
     {
         req->handle   = wine_server_obj_handle( irp_data->handle );
         req->status   = irp->IoStatus.u.Status;
         req->size     = irp->IoStatus.Information;
-        if (!NT_ERROR(irp->IoStatus.u.Status))
-        {
-            if (out_buff) wine_server_add_data( req, out_buff, irp->IoStatus.Information );
-        }
+        if (out_size) wine_server_add_data( req, irp->UserBuffer, out_size );
         status = wine_server_call( req );
     }
     SERVER_END_REQ;
@@ -943,17 +970,13 @@ NTSTATUS CDECL wine_ntoskrnl_main_loop( HANDLE stop_event )
                 if (context.irp_data->complete)
                 {
                     /* IRP completed even before we got here; we can report completion now */
-                    void *out_buff = irp->UserBuffer;
-
-                    if (irp->Flags & IRP_WRITE_OPERATION)
-                        out_buff = NULL;  /* do not transfer back input buffer */
+                    unsigned int out_size = get_irp_output_size( irp );

req->prev        = wine_server_obj_handle( context.irp_data->handle );
                     req->pending     = irp->PendingReturned;
                     req->iosb_status = irp->IoStatus.u.Status;
                     req->result      = irp->IoStatus.Information;
-                    if (!NT_ERROR(irp->IoStatus.u.Status) && out_buff)
-                        wine_server_add_data( req, out_buff, irp->IoStatus.Information );
+                    if (out_size) wine_server_add_data( req, irp->UserBuffer, out_size );
                 }
                 else
                 {
diff --git a/dlls/ntoskrnl.exe/tests/ntoskrnl.c b/dlls/ntoskrnl.exe/tests/ntoskrnl.c
index edc4a607225..f9fd9aa23e8 100644
--- a/dlls/ntoskrnl.exe/tests/ntoskrnl.c
+++ b/dlls/ntoskrnl.exe/tests/ntoskrnl.c
@@ -679,8 +679,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params)
     else if (!NT_ERROR(params->iosb_status))
         todo_wine_if (params->iosb_status == STATUS_PENDING) ok(size == 3, "got size %u\n", size);
     /* size is garbage if !NT_ERROR(expect_status) && NT_ERROR(iosb_status) */
-    todo_wine_if (ioctl != IOCTL_WINETEST_RETURN_STATUS_BUFFERED)
-        ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);
+    ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);

strcpy(buffer, "abcdef");
     io.Status = 0xdeadf00d;
@@ -701,8 +700,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params)
             ok(io.Information == 3, "got size %Iu\n", io.Information);
         }
     }
-    todo_wine_if (ioctl != IOCTL_WINETEST_RETURN_STATUS_BUFFERED)
-        ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);
+    ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);

/* Test the overlapped case. */

@@ -741,8 +739,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params)
         ret = WaitForSingleObject(event, 0);
         ok(!ret, "got %d\n", ret);
     }
-    todo_wine_if (ioctl != IOCTL_WINETEST_RETURN_STATUS_BUFFERED)
-        ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);
+    ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);

ret = WaitForSingleObject(file, 0);
     ok(ret == WAIT_TIMEOUT, "got %d\n", ret);
@@ -793,8 +790,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params)
         ret = WaitForSingleObject(event, 0);
         ok(!ret, "got %d\n", ret);
     }
-    todo_wine_if (ioctl != IOCTL_WINETEST_RETURN_STATUS_BUFFERED)
-        ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);
+    ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);

/* As above, but use the file handle instead of an event. */
     ret = WaitForSingleObject(file, 0);
@@ -825,8 +821,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params)
         ret = WaitForSingleObject(file, 0);
         ok(!ret, "got %d\n", ret);
     }
-    todo_wine_if (ioctl != IOCTL_WINETEST_RETURN_STATUS_BUFFERED)
-        ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);
+    ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);

/* Test FILE_SKIP_COMPLETION_PORT_ON_SUCCESS. */

@@ -861,8 +856,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params)
             ret = WaitForSingleObject(event, 0);
             ok(!ret, "got %d\n", ret);
         }
-        todo_wine_if (ioctl != IOCTL_WINETEST_RETURN_STATUS_BUFFERED)
-            ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);
+        ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);

key = 0xdeadf00d;
         value = 0xdeadf00d;
@@ -926,8 +920,7 @@ static void do_return_status(ULONG ioctl, struct return_status_params *params)
             ok(io.Information == 3, "got size %Iu\n", io.Information);
         }
     }
-    todo_wine_if (ioctl != IOCTL_WINETEST_RETURN_STATUS_BUFFERED)
-        ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);
+    ok(!strcmp(buffer, expect_buffer), "got buffer %s\n", buffer);

ret = SleepEx(0, TRUE);
     if (!params->pending && NT_ERROR(params->iosb_status))