Re: [PATCH 3/6] forkables: Create forkable hardlinks, yet unused.

[Michael Haubenwallner <michael dot haubenwallner at ssi-schaefer dot com>]

(sorry about previous empty mail)

Hi Corinna,

This is a fixup for the race condition where multiple processes failed
to concurrently create identical hardlinks.

So I'm quite successful with the forkable hardlinks now...

/haubi/


On 03/30/2016 08:53 PM, Michael Haubenwallner wrote:
> In preparation to protect fork() against dll- and exe-updates, create
> hardlinks to the main executable and each loaded dll in subdirectories
> of /var/run/cygfork/, if that one exists on the NTFS file system.
> 
> The directory names consist of the user sid, the main executable's NTFS
> IndexNumber, and the most recent LastWriteTime of all involved binaries
> (dlls and main executable).  Next to the main.exe hardlink we create the
> empty file main.exe.local to enable dll redirection.
> 
> The name of the mutex to synchronize hardlink creation/cleanup also is
> assembled from these directory names, to allow for synchronized cleanup
> of even orphaned hardlink directories.
> 
> The hardlink to each dynamically loaded dll goes into another directory,
> named using the NTFS IndexNumber of the dll's original directory.
> 
> 	* dll_init.h (struct dll): Declare member variables fbi, fii,
> 	forkable_ntname.  Declare methods nominate_forkable,
> 	create_forkable.  Define inline method forkedntname.
> 	(struct dll_list): Declare enum forkables_needs.  Declare member
> 	variables forkables_dirx_size, forkables_dirx_ntname,
> 	forkables_mutex_name, forkables_mutex.  Declare private methods
> 	forkable_ntnamesize, prepare_forkables_nomination,
> 	update_forkables_needs, update_forkables, create_forkables,
> 	denominate_forkables, close_mutex, try_remove_forkables.
> 	Declare public method cleanup_forkables.
> 	* dll_init.cc (dll_list::alloc): Allocate memory to hold the
> 	name of the hardlink in struct dll member forkable_ntname.
> 	Initialize struct dll members fbi, fii.
> 	* forkable.cc: Implement static functions mkdirs, rmdirs,
> 	rmdirs_synchronized, read_fii, read_fbi, format_IndexNumber,
> 	rootname, sidname, exename, lwtimename.  Define static array
> 	forkable_nameparts.
> 	(struct dll): Implement nominate_forkable, create_forkable.
> 	(struct dll_list): Implement forkable_ntnamesize,
> 	prepare_forkables_nomination, update_forkables_needs,
> 	update_forkables, create_forkables, close_mutex,
> 	cleanup_forkables, try_remove_forkables, denominate_forkables.
> 	(dll_list::set_forkables_inheritance): Also for forkables_mutex.
> 	(dll_list::request_forkables): Use new methods to create the
> 	hardlinks as necessary.
> 	(dll_list::release_forkables): When hardlink creation turned out
> 	to be impossible, close all the related handles and free the
> 	distinct memory.
> 	* pinfo.cc (pinfo::exit): Call dlls.cleanup_forkables.
> 	* syscalls.cc (_unlink_nt): Rename public unlink_nt function to
> 	static _unlink_nt, with 'shareable' as additional argument.
> 	(unlink_nt): New, wrap _unlink_nt for original behaviour.
> 	(unlink_nt_shareable): New, wrap _unlink_nt to keep a binary
> 	file still loadable while removing one of its hardlinks.
> ---
>  winsup/cygwin/dll_init.cc |   28 +-
>  winsup/cygwin/dll_init.h  |   33 ++
>  winsup/cygwin/forkable.cc | 1036 +++++++++++++++++++++++++++++++++++++++++++++
>  winsup/cygwin/pinfo.cc    |    3 +
>  winsup/cygwin/syscalls.cc |   24 +-
>  5 files changed, 1115 insertions(+), 9 deletions(-)

>From 7a824877dbfe2aee0437378a52b8946000a38cbe Mon Sep 17 00:00:00 2001
From: Michael Haubenwallner <michael.haubenwallner@ssi-schaefer.com>
Date: Fri, 11 Nov 2016 14:29:21 +0100
Subject: [PATCH] forkables: fix creating dirs and .local file in parallel

---
 winsup/cygwin/forkable.cc | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/winsup/cygwin/forkable.cc b/winsup/cygwin/forkable.cc
index 0a8a528..b712834 100644
--- a/winsup/cygwin/forkable.cc
+++ b/winsup/cygwin/forkable.cc
@@ -65,14 +65,14 @@ mkdirs (PWCHAR ntdirname, int lastsepcount)
 	  IO_STATUS_BLOCK iosb;
 	  status = NtCreateFile (&dh, GENERIC_READ | SYNCHRONIZE,
 				 &oa, &iosb, NULL, FILE_ATTRIBUTE_NORMAL,
-				 FILE_SHARE_READ,
-				 FILE_OPEN_IF, /* allow concurrency */
+				 FILE_SHARE_READ | FILE_SHARE_WRITE,
+				 FILE_CREATE,
 				 FILE_DIRECTORY_FILE
 				 | FILE_SYNCHRONOUS_IO_NONALERT,
 				 NULL, 0);
 	  if (NT_SUCCESS(status))
 	    NtClose (dh);
-	  else
+	  else if (status != STATUS_OBJECT_NAME_COLLISION) /* already exists */
 	    success = false;
 	  debug_printf ("%y = NtCreateFile (%p, dir %W)", status, dh, ntdirname);
 	}
@@ -806,14 +806,14 @@ dll_list::create_forkables ()
       IO_STATUS_BLOCK iosb;
       status = NtCreateFile (&hlocal, GENERIC_WRITE | SYNCHRONIZE,
 			     &oa, &iosb, NULL, FILE_ATTRIBUTE_NORMAL,
-			     FILE_SHARE_READ,
-			     FILE_OPEN_IF, /* allow concurrency */
+			     FILE_SHARE_READ | FILE_SHARE_WRITE,
+			     FILE_CREATE,
 			     FILE_NON_DIRECTORY_FILE
 			     | FILE_SYNCHRONOUS_IO_NONALERT,
 			     NULL, 0);
       if (NT_SUCCESS (status))
 	CloseHandle (hlocal);
-      else
+      else if (status != STATUS_OBJECT_NAME_COLLISION) /* already exists */
 	success = false;
       debug_printf ("%y = NtCreateFile (%p, %W)", status, hlocal, ntname);
     }
@@ -874,7 +874,10 @@ rmdirs_synchronized (WCHAR ntbuf[NT_MAX_PATH], int depth, int maxdepth,
       if (mutex)
 	{
 	  if (lasterror != ERROR_ALREADY_EXISTS)
-	    rmdirs (ntbuf);
+	    {
+	      debug_printf ("cleaning up for mutex %W", mutexname);
+	      rmdirs (ntbuf);
+	    }
 	  BOOL bret = CloseHandle (mutex);
 	  debug_printf ("%d = CloseHandle (%p, %W): %E",
 			bret, mutex, mutexname);
-- 
2.8.3