[PATCH] ntdll/server: Make robust to spurious short writes

Keno Fischer keno at juliacomputing.com
Wed Dec 15 04:22:33 CST 2021


It is possible for the write/writev functions in send_request to
return short writes, even in non-error conditions. For example,
an unfortunately timed SIGINT after the first 4096 bytes of a
pipe transfer will interrupt the syscall and return. In some
cases (in particular when no bytes have been transferred at all),
the SA_RESTART spec on the signal handler will take care of the
restart, but once any bytes have been transferred, the result will
be a short write and no automatic restart. Some linux profiling and
debugging tooling can also increase the rate of signal interruptions
making this corner case more likely.

Make wine robust to this corner case by properly restarting a
short write with adjusted buffers.
---
 dlls/ntdll/unix/server.c | 38 +++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/dlls/ntdll/unix/server.c b/dlls/ntdll/unix/server.c
index a388247beb2..a848328f2a9 100644
--- a/dlls/ntdll/unix/server.c
+++ b/dlls/ntdll/unix/server.c
@@ -183,13 +183,22 @@ static DECLSPEC_NORETURN void server_protocol_perror( const char *err )
 static unsigned int send_request( const struct __server_request_info *req )
 {
     unsigned int i;
-    int ret;
+    int ret = 0;
 
+    int to_write = sizeof(req->u.req) + req->u.req.request_header.request_size;
     if (!req->u.req.request_header.request_size)
     {
-        if ((ret = write( ntdll_get_thread_data()->request_fd, &req->u.req,
-                          sizeof(req->u.req) )) == sizeof(req->u.req)) return STATUS_SUCCESS;
-
+        const char *write_ptr = (const char *)&req->u.req;
+        for (;;) {
+            ret = write( ntdll_get_thread_data()->request_fd, (void*)write_ptr,
+                         to_write );
+            if (ret == to_write) return STATUS_SUCCESS;
+            else if (ret < 0) break;
+            // Short write (e.g. due to an unfortunately timed SIGINT) - not
+            // an error. Adjust remaining write length and buffer and start again.
+            to_write -= ret;
+            write_ptr += ret;
+        }
     }
     else
     {
@@ -202,11 +211,26 @@ static unsigned int send_request( const struct __server_request_info *req )
             vec[i+1].iov_base = (void *)req->data[i].ptr;
             vec[i+1].iov_len = req->data[i].size;
         }
-        if ((ret = writev( ntdll_get_thread_data()->request_fd, vec, i+1 )) ==
-            req->u.req.request_header.request_size + sizeof(req->u.req)) return STATUS_SUCCESS;
+
+        for (;;) {
+            ret = writev( ntdll_get_thread_data()->request_fd, vec, i+1 );
+            if (ret == to_write) return STATUS_SUCCESS;
+            else if (ret < 0) break;
+            // Short write as above. Adjust buffer lengths and start again.
+            to_write -= ret;
+            for (unsigned int j = 0; j < i+1; j++) {
+                if (ret >= vec[j].iov_len) {
+                    ret -= vec[j].iov_len;
+                    vec[j].iov_len = 0;
+                } else {
+                    vec[j].iov_base = (char *)vec[j].iov_base + ret;
+                    vec[j].iov_len -= ret;
+                    break;
+                }
+            }
+        }
     }
 
-    if (ret >= 0) server_protocol_error( "partial write %d\n", ret );
     if (errno == EPIPE) abort_thread(0);
     if (errno == EFAULT) return STATUS_ACCESS_VIOLATION;
     server_protocol_perror( "write" );
-- 
2.25.1




More information about the wine-devel mailing list