Windows: Prevent simultaneous pioctls

The Windows pioctl implementation makes an incorrect assumption.
It is not true that every CreateFile() operation results in a
SMB NTCreateX operation being delivered to SMB Server.  The SMB
client can combine open requests from multiple processes or threads
onto a single SMB file descriptor and locally manage the operations.
This is a problem for pioctls since the Transceive operation requires
that a WriteFile/ReadFile combination must belong to the same request.

Prior to this change simultaneous pioctl operations would be
combined and the individual reads and writes could overlap resulting
in responses going to the wrong requestor and end of file errors
being received by the others.

Due to lack of data validation in fs.c, ktc_nt.c, symlink.c,
etc random crashes are produced.

This change alters the sharing mode under which the pioctl file is
opened.  Instead of FILE_SHARE_READ | FILE_SHARE_WRITE, only
FILE_SHARE_READ is specified to CreateFile().  This ensures that
the CreateFile will fail with a sharing violation if the pioctl
file was previously opened for writing.

A sharing violation check is provided and the CreateFile is retried
indefinitely until the open succeeds or the error is not a sharing
violation.

LICENSE MIT

Reviewed-on: http://gerrit.openafs.org/404
Reviewed-by: Derrick Brashear <shadow@dementia.org>
Tested-by: Asanka Herath <asanka@secure-endpoints.com>
Reviewed-by: Asanka Herath <asanka@secure-endpoints.com>
Tested-by: Jeffrey Altman <jaltman@openafs.org>
Reviewed-by: Jeffrey Altman <jaltman@openafs.org>

diff --git a/src/sys/pioctl_nt.c b/src/sys/pioctl_nt.c
index db2acf3bd4116465aed7327b6e1cc30df73d6674..a4da876c3e40e65e8e1caec6330f66b73304cc51 100644 (file)
--- a/src/sys/pioctl_nt.c
+++ b/src/sys/pioctl_nt.c
@@ -594,6 +594,7 @@ GetIoctlHandle(char *fileNamep, HANDLE * handlep)
     DWORD dwSize = sizeof(szUser);
     int saveerrno;
     UINT driveType;
+    int sharingViolation;
 
     memset(HostName, '\0', sizeof(HostName));
     gethostname(HostName, sizeof(HostName));
@@ -695,11 +696,16 @@ GetIoctlHandle(char *fileNamep, HANDLE * handlep)
 
     fflush(stdout);
     /* now open the file */
-    fh = CreateFile(tbuffer, GENERIC_READ | GENERIC_WRITE,
-                   FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING,
-                   FILE_FLAG_WRITE_THROUGH, NULL);
-
-       fflush(stdout);
+    sharingViolation = 0;
+    do {
+        if (sharingViolation)
+            Sleep(1);
+        fh = CreateFile(tbuffer, FILE_READ_DATA | FILE_WRITE_DATA,
+                        FILE_SHARE_READ, NULL, OPEN_EXISTING,
+                        FILE_FLAG_WRITE_THROUGH, NULL);
+        sharingViolation = 1;
+    } while (fh == INVALID_HANDLE_VALUE && GetLastError() == ERROR_SHARING_VIOLATION);
+    fflush(stdout);
 
     if (fh == INVALID_HANDLE_VALUE) {
         int  gonext = 0;
@@ -771,9 +777,15 @@ GetIoctlHandle(char *fileNamep, HANDLE * handlep)
             if (gonext)
                 goto try_lsa_principal;
 
-            fh = CreateFile(tbuffer, GENERIC_READ | GENERIC_WRITE,
-                             FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING,
-                             FILE_FLAG_WRITE_THROUGH, NULL);
+            sharingViolation = 0;
+            do {
+                if (sharingViolation)
+                    Sleep(1);
+                fh = CreateFile(tbuffer, FILE_READ_DATA | FILE_WRITE_DATA,
+                                FILE_SHARE_READ, NULL, OPEN_EXISTING,
+                                FILE_FLAG_WRITE_THROUGH, NULL);
+                sharingViolation = 1;
+            } while (fh == INVALID_HANDLE_VALUE && GetLastError() == ERROR_SHARING_VIOLATION);
             fflush(stdout);
             if (fh == INVALID_HANDLE_VALUE) {
                 gle = GetLastError();
@@ -841,9 +853,15 @@ GetIoctlHandle(char *fileNamep, HANDLE * handlep)
             if (gonext)
                 goto try_sam_compat;
 
-            fh = CreateFile(tbuffer, GENERIC_READ | GENERIC_WRITE,
-                             FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING,
-                             FILE_FLAG_WRITE_THROUGH, NULL);
+            sharingViolation = 0;
+            do {
+                if (sharingViolation)
+                    Sleep(1);
+                fh = CreateFile(tbuffer, FILE_READ_DATA | FILE_WRITE_DATA,
+                                FILE_SHARE_READ, NULL, OPEN_EXISTING,
+                                FILE_FLAG_WRITE_THROUGH, NULL);
+                sharingViolation = 1;
+            } while (fh == INVALID_HANDLE_VALUE && GetLastError() == ERROR_SHARING_VIOLATION);
             fflush(stdout);
             if (fh == INVALID_HANDLE_VALUE) {
                 gle = GetLastError();
@@ -906,9 +924,15 @@ GetIoctlHandle(char *fileNamep, HANDLE * handlep)
                 return -1;
             }
 
-            fh = CreateFile(tbuffer, GENERIC_READ | GENERIC_WRITE,
-                             FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING,
-                             FILE_FLAG_WRITE_THROUGH, NULL);
+            sharingViolation = 0;
+            do {
+                if (sharingViolation)
+                    Sleep(1);
+                fh = CreateFile(tbuffer, FILE_READ_DATA | FILE_WRITE_DATA,
+                                FILE_SHARE_READ, NULL, OPEN_EXISTING,
+                                FILE_FLAG_WRITE_THROUGH, NULL);
+                sharingViolation = 1;
+            } while (fh == INVALID_HANDLE_VALUE && GetLastError() == ERROR_SHARING_VIOLATION);
             fflush(stdout);
             if (fh == INVALID_HANDLE_VALUE) {
                 gle = GetLastError();