[PATCH v3 0/1] MR10424: services: Cancel overlapped IO after timeout.

[Yuxuan Shui (＠yshui)]

Though WaitForSingleObject timed out, the IO operation is still running in the background. After the
function process_send_command returns, the OVERLAPPED object referenced by this IO operation becomes
invalid. Later, when this IO finally completes, server_select will use invalid stack memory.

--
  v3: services: Cancel overlapped IO after timeout.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10424

[PATCH v3 1/1] services: Cancel overlapped IO after timeout.

[Yuxuan Shui]

From: Yuxuan Shui <[e-mail]>

Though WaitForSingleObject timed out, the IO operation is still running in the background. After the
function process_send_command returns, the OVERLAPPED object referenced by this IO operation becomes
invalid. Later, when this IO finally completes, server_select will use invalid stack memory.
---
 programs/services/rpc.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/programs/services/rpc.c b/programs/services/rpc.c
index 2f4d4a5c26c..a462696218d 100644
--- a/programs/services/rpc.c
+++ b/programs/services/rpc.c
@@ -1223,16 +1223,22 @@ static BOOL process_send_command(struct process_entry *process, const void *data
         if (ret == WAIT_TIMEOUT)
         {
             WINE_ERR("receiving command result timed out\n");
-            *result = ERROR_SERVICE_REQUEST_TIMEOUT;
-            return FALSE;
+            if (!CancelIoEx(process->control_pipe, &overlapped))
+                WINE_ERR("Failed to cancel IO, %#lx\n", GetLastError());
         }
-        r = GetOverlappedResult(process->control_pipe, &overlapped, &count, FALSE);
+        r = GetOverlappedResult(process->control_pipe, &overlapped, &count, TRUE);
     }
     if (!r || count != sizeof *result)
     {
-        WINE_ERR("service protocol error - failed to read pipe "
-            "r = %d  count = %ld!\n", r, count);
-        *result = (!r ? GetLastError() : ERROR_READ_FAULT);
+        DWORD error = !r ? GetLastError() : ERROR_READ_FAULT;
+        if (error == ERROR_OPERATION_ABORTED)
+            *result = ERROR_SERVICE_REQUEST_TIMEOUT;
+        else
+        {
+            WINE_ERR("service protocol error - failed to read pipe "
+                "r = %d  count = %ld!\n", r, count);
+            *result = error;
+        }
         return FALSE;
     }
 
-- 
GitLab

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

[Yuxuan Shui (＠yshui)]

fix: avoid calling GetLastError if GetOverlappedResult returned true.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10424#note_135714

[Yuxuan Shui (＠yshui)]

ping @iamahuman

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10424#note_144058

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about programs/services/rpc.c:
> if (ret == WAIT_TIMEOUT)
>          {
>              WINE_ERR("receiving command result timed out\n");
> -            *result = ERROR_SERVICE_REQUEST_TIMEOUT;
> -            return FALSE;
> +            if (!CancelIoEx(process->control_pipe, &overlapped))
> +                WINE_ERR("Failed to cancel IO, %#lx\n", GetLastError());
>          }
> -        r = GetOverlappedResult(process->control_pipe, &overlapped, &count, FALSE);
> +        r = GetOverlappedResult(process->control_pipe, &overlapped, &count, TRUE);
>      }
>      if (!r || count != sizeof *result)
> +    {
> +        DWORD error = !r ? GetLastError() : ERROR_READ_FAULT;
> +        if (error == ERROR_OPERATION_ABORTED)

To be clear, this (`error == ERROR_OPERATION_ABORTED`) is ONLY true when we have cancelled it ourselves, right?

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/10424#note_144326