Cygwin: Try to fix potential data corruption in pipe write

* fhandler.cc (fhandler_base_overlapped::raw_write): When performing
        nonblocking I/O, copy user space data into own buffer.  Add longish
        comment to explain why.
        * fhandler.h (fhandler_base_overlapped::atomic_write_buf): New member.
        (fhandler_base_overlapped::fhandler_base_overlapped): Initialize
        atomic_write_buf.
        (fhandler_base_overlapped::fhandler_base_overlapped): New destructor,
        free'ing atomic_write_buf.
        (fhandler_base_overlapped::copyto): Set atomic_write_buf to NULL in
        copied fhandler.

Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

winsup/cygwin/ChangeLog

@@ -1,3 +1,16 @@
+2015-08-15  Corinna Vinschen  <corinna@vinschen.de>
+
+	* fhandler.cc (fhandler_base_overlapped::raw_write): When performing
+	nonblocking I/O, copy user space data into own buffer.  Add longish
+	comment to explain why.
+	* fhandler.h (fhandler_base_overlapped::atomic_write_buf): New member.
+	(fhandler_base_overlapped::fhandler_base_overlapped): Initialize
+	atomic_write_buf.
+	(fhandler_base_overlapped::fhandler_base_overlapped): New destructor,
+	free'ing atomic_write_buf.
+	(fhandler_base_overlapped::copyto): Set atomic_write_buf to NULL in
+	copied fhandler.
+
 2015-08-14  Corinna Vinschen  <corinna@vinschen.de>
 
 	* security.cc (convert_samba_sd): Fix copy/paste error in previous

winsup/cygwin/fhandler.cc

@@ -2093,6 +2093,46 @@ fhandler_base_overlapped::raw_write (const void *ptr, size_t len)
       else
 	chunk = max_atomic_write;
 
+      /* MSDN "WriteFile" contains the following note: "Accessing the output
+         buffer while a write operation is using the buffer may lead to
+	 corruption of the data written from that buffer.  [...]  This can
+	 be particularly problematic when using an asynchronous file handle.
+	 (https://msdn.microsoft.com/en-us/library/windows/desktop/aa365747)
+
+	 MSDN "Synchronous and Asynchronous I/O" contains the following note:
+	 "Do not deallocate or modify [...] the data buffer until all
+	 asynchronous I/O operations to the file object have been completed."
+	 (https://msdn.microsoft.com/en-us/library/windows/desktop/aa365683)
+
+	 This problem is a non-issue for blocking I/O, but it can lead to
+	 problems when using nonblocking I/O.  Consider:
+	 - The application uses a static buffer in repeated calls to
+	   non-blocking write.
+	 - The previous write returned with success, but the overlapped I/O
+	   operation is ongoing.
+	 - The application copies the next set of data to the static buffer,
+	   thus overwriting data still accessed by the previous write call.
+	 --> potential data corruption.
+
+	 What we do here is to allocate a per-fhandler buffer big enough
+	 to perform the maximum atomic operation from, copy the user space
+	 data over to this buffer and then call NtWriteFile on this buffer.
+	 This decouples the write operation from the user buffer and the
+	 user buffer can be reused without data corruption issues.
+
+	 Since no further write can occur while we're still having ongoing
+	 I/O, this should be reasanably safe.
+
+	 Note: We only have proof that this problem actually occurs on Wine
+	 yet.  However, the MSDN language indicates that this may be a real
+	 problem on real Windows as well. */
+      if (is_nonblocking ())
+	{
+	  if (!atomic_write_buf)
+	    atomic_write_buf = cmalloc_abort (HEAP_BUF, max_atomic_write);
+	  ptr = memcpy (atomic_write_buf, ptr, chunk);
+	}
+
       nbytes = 0;
       DWORD nbytes_now = 0;
       /* Write to fd in smaller chunks, accumulating a total.

winsup/cygwin/fhandler.h

@@ -661,6 +661,7 @@ protected:
   OVERLAPPED io_status;
   OVERLAPPED *overlapped;
   size_t max_atomic_write;
+  void *atomic_write_buf;
 public:
   wait_return __reg3 wait_overlapped (bool, bool, DWORD *, bool, DWORD = 0);
   int __reg1 setup_overlapped ();
@@ -670,7 +671,7 @@ public:
   OVERLAPPED *&get_overlapped () {return overlapped;}
   OVERLAPPED *get_overlapped_buffer () {return &io_status;}
   void set_overlapped (OVERLAPPED *ov) {overlapped = ov;}
-  fhandler_base_overlapped (): io_pending (false), overlapped (NULL), max_atomic_write (0)
+  fhandler_base_overlapped (): io_pending (false), overlapped (NULL), max_atomic_write (0), atomic_write_buf (NULL)
   {
     memset (&io_status, 0, sizeof io_status);
   }
@@ -686,11 +687,17 @@ public:
   static void __reg1 flush_all_async_io ();;
 
   fhandler_base_overlapped (void *) {}
+  ~fhandler_base_overlapped ()
+  {
+    if (atomic_write_buf)
+      cfree (atomic_write_buf);
+  }
 
   virtual void copyto (fhandler_base *x)
   {
     x->pc.free_strings ();
     *reinterpret_cast<fhandler_base_overlapped *> (x) = *this;
+    reinterpret_cast<fhandler_base_overlapped *> (x)->atomic_write_buf = NULL;
     x->reset (this);
   }
 

winsup/cygwin/release/2.2.1

@@ -19,3 +19,8 @@ Bug Fixes
 - Don't try to perform RFC2307 owner/group mapping on Samba/NFS if account
   info is only fetched from local passwd/group files.
   Addresses: https://cygwin.com/ml/cygwin/2015-07/msg00270.html
+
+- Precautionally fix a potential data corruption problem in pipe I/O, only
+  actually observered in Wine yet.  However, MSDN language indicates this
+  might be a problem on real Windows as well.
+  Addresses: Freenode IRC #cygwin discussion, ML entry follows.