public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH 3/6] forkables: Create forkable hardlinks, yet unused.
  2016-03-30 18:54 [PATCH 0/6] Protect fork() against dll- and exe-updates Michael Haubenwallner
  2016-03-30 18:54 ` [PATCH 4/6] forkables: Protect fork against dll-, exe-updates Michael Haubenwallner
  2016-03-30 18:54 ` [PATCH 1/6] forkables: Store dll file name as full NT path Michael Haubenwallner
@ 2016-03-30 18:54 ` Michael Haubenwallner
  2016-11-16 12:25   ` Michael Haubenwallner
  2016-11-16 12:34   ` Michael Haubenwallner
  2016-03-30 18:54 ` [PATCH 2/6] forkables: Track main executable and cygwin1.dll Michael Haubenwallner
                   ` (3 subsequent siblings)
  6 siblings, 2 replies; 16+ messages in thread
From: Michael Haubenwallner @ 2016-03-30 18:54 UTC (permalink / raw)
  To: cygwin-patches; +Cc: michael.haubenwallner

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(-)

diff --git a/winsup/cygwin/dll_init.cc b/winsup/cygwin/dll_init.cc
index fd807c9..e44ee84 100644
--- a/winsup/cygwin/dll_init.cc
+++ b/winsup/cygwin/dll_init.cc
@@ -372,8 +372,11 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type)
     }
   else
     {
+      size_t forkntsize = forkable_ntnamesize (type, ntname, modname);
+
       /* FIXME: Change this to new at some point. */
-      d = (dll *) cmalloc (HEAP_2_DLL, sizeof (*d) + (ntnamelen * sizeof (*ntname)));
+      d = (dll *) cmalloc (HEAP_2_DLL, sizeof (*d)
+			   + ((ntnamelen + forkntsize) * sizeof (*ntname)));
 
       /* Now we've allocated a block of information.  Fill it in with the
 	 supplied info about this DLL. */
@@ -389,11 +392,24 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type)
       d->image_size = ((pefile*)h)->optional_hdr ()->SizeOfImage;
       d->preferred_base = (void*) ((pefile*)h)->optional_hdr()->ImageBase;
       d->type = type;
-      NTSTATUS status;
-      d->fhandle = ntopenfile (d->ntname, &status);
-      if (!d->fhandle)
-	system_printf ("Unable (ntstatus %y) to open file %W",
-		       status, d->ntname);
+      d->fhandle = NULL;
+      d->fbi.FileAttributes = INVALID_FILE_ATTRIBUTES;
+      d->fii.IndexNumber.QuadPart = 0;
+      d->forkable_ntname = NULL;
+      if (forkntsize)
+	{
+	  NTSTATUS status;
+	  d->fhandle = ntopenfile (d->ntname, &status);
+	  if (!d->fhandle)
+	    system_printf ("Unable (ntstatus %y) to open file %W",
+			   status, d->ntname);
+	  else
+	    {
+	      /* may create a hardlink */
+	      d->forkable_ntname = d->ntname + ntnamelen + 1;
+	      *d->forkable_ntname = L'\0';
+	    }
+	}
       append (d);
       if (type == DLL_LOAD)
 	loaded_dlls++;
diff --git a/winsup/cygwin/dll_init.h b/winsup/cygwin/dll_init.h
index c50f889..554435d 100644
--- a/winsup/cygwin/dll_init.h
+++ b/winsup/cygwin/dll_init.h
@@ -65,6 +65,15 @@ struct dll
 
   /* forkable */
   HANDLE fhandle;
+  FILE_BASIC_INFORMATION fbi;
+  FILE_INTERNAL_INFORMATION fii;
+  PWCHAR forkable_ntname;
+  void nominate_forkable (PCWCHAR);
+  bool create_forkable ();
+  PWCHAR forkedntname ()
+  {
+    return forkable_ntname && *forkable_ntname ? forkable_ntname : ntname;
+  }
 
   WCHAR ntname[1]; /* must be the last data member */
   void detach ();
@@ -83,7 +92,30 @@ struct dll
 
 class dll_list
 {
+  /* forkables */
+  enum
+    {
+      forkables_unknown,
+      forkables_impossible,
+      forkables_disabled,
+      forkables_needless,
+      forkables_needed,
+      forkables_created,
+    }
+    forkables_needs;
+  DWORD forkables_dirx_size;
+  PWCHAR forkables_dirx_ntname;
+  PWCHAR forkables_mutex_name;
+  HANDLE forkables_mutex;
   void track_self ();
+  size_t forkable_ntnamesize (dll_type, PCWCHAR fullntname, PCWCHAR modname);
+  void prepare_forkables_nomination ();
+  void update_forkables_needs ();
+  bool update_forkables ();
+  bool create_forkables ();
+  void denominate_forkables ();
+  bool close_mutex ();
+  void try_remove_forkables (PWCHAR dirbuf, size_t dirlen, size_t dirbufsize);
   void set_forkables_inheritance (bool);
 
   dll *end;
@@ -97,6 +129,7 @@ public:
   dll *main_executable;
   void request_forkables ();
   void release_forkables ();
+  void cleanup_forkables ();
 
   static HANDLE ntopenfile (PCWCHAR ntname, NTSTATUS *pstatus = NULL,
 			    ULONG openopts = FILE_NON_DIRECTORY_FILE);
diff --git a/winsup/cygwin/forkable.cc b/winsup/cygwin/forkable.cc
index 5592985..0a8a528 100644
--- a/winsup/cygwin/forkable.cc
+++ b/winsup/cygwin/forkable.cc
@@ -27,6 +27,998 @@ details. */
 #include <assert.h>
 #include <tls_pbuf.h>
 
+/* Allow concurrent processes to use the same dll or exe
+ * via their hardlink while we delete our hardlink. */
+extern NTSTATUS unlink_nt_shareable (path_conv &pc);
+
+#define MUTEXSEP L"@"
+#define PATHSEP L"\\"
+
+/* Create the lastsepcount directories found in ntdirname, where
+   counting is done along path separators (including trailing ones).
+   Returns true when these directories exist afterwards, false otherways.
+   The ntdirname is used for the path-splitting. */
+static bool
+mkdirs (PWCHAR ntdirname, int lastsepcount)
+{
+  bool success = true;
+  int i = lastsepcount;
+  for (--i; i > 0; --i)
+    {
+      PWCHAR lastsep = wcsrchr (ntdirname, L'\\');
+      if (!lastsep)
+	break;
+      *lastsep = L'\0';
+    }
+
+  for (++i; i <= lastsepcount; ++i)
+    {
+      if (success && (i == 0 || wcslen (wcsrchr (ntdirname, L'\\')) > 1))
+	{
+	  UNICODE_STRING dn;
+	  RtlInitUnicodeString (&dn, ntdirname);
+	  OBJECT_ATTRIBUTES oa;
+	  InitializeObjectAttributes (&oa, &dn, 0, NULL,
+				      sec_none_nih.lpSecurityDescriptor);
+	  HANDLE dh = NULL;
+	  NTSTATUS status;
+	  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_DIRECTORY_FILE
+				 | FILE_SYNCHRONOUS_IO_NONALERT,
+				 NULL, 0);
+	  if (NT_SUCCESS(status))
+	    NtClose (dh);
+	  else
+	    success = false;
+	  debug_printf ("%y = NtCreateFile (%p, dir %W)", status, dh, ntdirname);
+	}
+      if (i < lastsepcount)
+	ntdirname[wcslen (ntdirname)] = L'\\'; /* restore original value */
+    }
+  return success;
+}
+
+/* Recursively remove the directory specified in ntmaxpathbuf,
+   using ntmaxpathbuf as the buffer to form subsequent filenames. */
+static void
+rmdirs (WCHAR ntmaxpathbuf[NT_MAX_PATH])
+{
+  PWCHAR basebuf = wcsrchr (ntmaxpathbuf, L'\\'); /* find last pathsep */
+  if (basebuf && *(basebuf+1))
+    basebuf += wcslen (basebuf); /* last pathsep is not trailing one */
+  if (!basebuf)
+    basebuf = ntmaxpathbuf + wcslen (ntmaxpathbuf);
+  *basebuf = L'\0'; /* kill trailing pathsep, if any */
+
+  NTSTATUS status;
+  HANDLE hdir = dll_list::ntopenfile (ntmaxpathbuf, &status,
+				      FILE_DIRECTORY_FILE |
+				      FILE_DELETE_ON_CLOSE);
+  if (!hdir)
+    return;
+
+  *basebuf++ = L'\\'; /* (re-)add trailing pathsep */
+
+  struct {
+    FILE_DIRECTORY_INFORMATION fdi;
+    WCHAR buf[NAME_MAX];
+  } fdibuf;
+  IO_STATUS_BLOCK iosb;
+
+  while (NT_SUCCESS (status = NtQueryDirectoryFile (hdir, NULL, NULL, NULL,
+						    &iosb,
+						    &fdibuf, sizeof (fdibuf),
+						    FileDirectoryInformation,
+						    FALSE, NULL, FALSE)))
+    {
+      PFILE_DIRECTORY_INFORMATION pfdi = &fdibuf.fdi;
+      while (true)
+	{
+	  int namelen = pfdi->FileNameLength / sizeof (WCHAR);
+	  wcsncpy (basebuf, pfdi->FileName, namelen);
+	  basebuf[namelen] = L'\0';
+
+	  if (pfdi->FileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+	    {
+	      if (wcscmp (basebuf, L".") && wcscmp (basebuf, L".."))
+		rmdirs (ntmaxpathbuf);
+	    }
+	  else
+	    {
+	      UNICODE_STRING fn;
+	      RtlInitUnicodeString (&fn, ntmaxpathbuf);
+
+	      path_conv pc (&fn);
+	      unlink_nt_shareable (pc); /* move to bin */
+	    }
+
+	  if (!pfdi->NextEntryOffset)
+	    break;
+	  pfdi = (PFILE_DIRECTORY_INFORMATION)((caddr_t)pfdi
+					       + pfdi->NextEntryOffset);
+	}
+    }
+  if (status != STATUS_NO_MORE_FILES)
+    debug_printf ("%y = NtQueryDirectoryFile (%p, io %y, info %d)",
+		  status, hdir, iosb.Status, iosb.Information);
+
+  CloseHandle (hdir);
+}
+
+static bool
+read_fii (HANDLE fh, PFILE_INTERNAL_INFORMATION pfii, bool once = false)
+{
+  if (once && pfii->IndexNumber.QuadPart != 0)
+    return true;
+
+  pfii->IndexNumber.QuadPart = 0LL;
+
+  NTSTATUS status;
+  IO_STATUS_BLOCK iosb;
+  status = NtQueryInformationFile (fh, &iosb,
+				   pfii, sizeof (*pfii),
+				   FileInternalInformation);
+  if (!NT_SUCCESS (status))
+    {
+      system_printf ("WARNING: %y = NtQueryInformationFile (%p,"
+		     " InternalInfo, io.Status %y)",
+		     status, fh, iosb.Status);
+      pfii->IndexNumber.QuadPart = -1LL;
+      return false;
+    }
+  return true;
+}
+
+static bool
+read_fbi (HANDLE fh, PFILE_BASIC_INFORMATION pfbi, bool once = false)
+{
+  if (once && pfbi->FileAttributes != INVALID_FILE_ATTRIBUTES)
+    return true;
+
+  pfbi->FileAttributes = INVALID_FILE_ATTRIBUTES;
+  pfbi->LastWriteTime.QuadPart = 0LL;
+
+  NTSTATUS status;
+  IO_STATUS_BLOCK iosb;
+  status = NtQueryInformationFile (fh, &iosb,
+				   pfbi, sizeof (*pfbi),
+				   FileBasicInformation);
+  if (!NT_SUCCESS (status))
+    {
+      system_printf ("WARNING: %y = NtQueryInformationFile (%p,"
+		     " BasicInfo, io.Status %y)",
+		     status, fh, iosb.Status);
+      pfbi->FileAttributes = 0;
+      pfbi->LastWriteTime.QuadPart = -1LL;
+      return false;
+    }
+  return true;
+}
+
+/* Into buf if not NULL, write the IndexNumber in pli.
+   Return the number of characters (that would be) written. */
+static int
+format_IndexNumber (PWCHAR buf, ssize_t bufsize, LARGE_INTEGER const *pli)
+{
+  if (!buf)
+    return 16;
+  if (bufsize >= 0 && bufsize <= 16)
+    return 0;
+  return __small_swprintf (buf, L"%016X", pli->QuadPart);
+}
+
+/* Into buf if not NULL, write the ntname of cygwin installation_root.
+   Return the number of characters (that would be) written. */
+static int
+rootname (PWCHAR buf, ssize_t bufsize)
+{
+  PWCHAR cygroot = cygheap->installation_root;
+  if (!buf)
+    return 6 /* "\??\UN" */ + wcslen (cygroot);
+  return dll_list::form_ntname (buf, bufsize, cygroot) - buf;
+}
+
+/* Into buf if not NULL, write the string representation of current user sid.
+   Return the number of characters (that would be) written. */
+static int
+sidname (PWCHAR buf, ssize_t bufsize)
+{
+  if (!buf)
+    return 128;
+  if (bufsize >= 0 && bufsize <= 128)
+    return 0;
+  UNICODE_STRING sid;
+  WCHAR sidbuf[128+1];
+  RtlInitEmptyUnicodeString (&sid, sidbuf, sizeof (sidbuf));
+  RtlConvertSidToUnicodeString (&sid, cygheap->user.sid (), FALSE);
+  return wcpcpy (buf, sid.Buffer) - buf;
+}
+
+/* Into buf if not NULL, write the IndexNumber of the main executable.
+   Return the number of characters (that would be) written. */
+static int
+exename (PWCHAR buf, ssize_t bufsize)
+{
+  if (!buf)
+    return format_IndexNumber (NULL, bufsize, NULL);
+  dll *d = dlls.main_executable;
+  if (d->fhandle)
+    (void) read_fii (d->fhandle, &d->fii, true);
+  return format_IndexNumber (buf, bufsize, &d->fii.IndexNumber);
+}
+
+/* Into buf if not NULL, write the newest dll's LastWriteTime.
+   Return the number of characters (that would be) written. */
+static int
+lwtimename (PWCHAR buf, ssize_t bufsize)
+{
+  if (!buf)
+    return sizeof (LARGE_INTEGER) * 2;
+  if (bufsize >= 0 && bufsize <= (int)sizeof (LARGE_INTEGER) * 2)
+    return 0;
+
+  LARGE_INTEGER newest = { 0 };
+  /* Need by-handle-file-information for _all_ loaded dlls,
+     as most recent ctime forms the hardlinks directory. */
+  dll *d = &dlls.start;
+  while ((d = d->next))
+    {
+      if (!d->fhandle)
+	continue;
+
+      (void)read_fbi (d->fhandle, &d->fbi, true);
+
+      /* LastWriteTime more properly tells the last file-content modification
+	 time, because a newly created hardlink may have a different
+	 CreationTime compared to the original file. */
+      if (d->fbi.LastWriteTime.QuadPart > newest.QuadPart)
+	newest = d->fbi.LastWriteTime;
+    }
+
+  return __small_swprintf (buf, L"%016X", newest);
+}
+
+struct namepart {
+  PCWCHAR text; /* used when no pathfunc, description otherwise */
+  int (*textfunc)(PWCHAR buf, ssize_t bufsize);
+  bool mutex_from_dir; /* on path-separators add mutex-separator */
+  bool create_dir;
+};
+/* mutex name is formed along dir names */
+static namepart NO_COPY_RO const
+forkable_nameparts[] = {
+ /* text             textfunc  mutex_from_dir  create */
+  { L"<cygroot>",    rootname,          false, false, },
+  { L"\\var\\run\\",     NULL,          false, false, },
+  { L"cygfork",          NULL,          true,  false, },
+  { L"<sid>",         sidname,          true,  true,  },
+  { L"<exe>",         exename,          false, false, },
+  { MUTEXSEP,            NULL,          false, false, },
+  { L"<ctime>",    lwtimename,          true,  true,  },
+
+  { NULL, NULL },
+};
+
+/* Nominate the hardlink to an individual DLL inside dirx_name,
+   that ends with the path separator (hence the "x" varname).
+   With NULL as dirx_name, never nominate the hardlink any more.
+   With "" as dirx_name, denominate the hardlink. */
+void
+dll::nominate_forkable (PCWCHAR dirx_name)
+{
+  if (!dirx_name)
+    {
+      debug_printf ("type %d disable %W", type, ntname);
+      forkable_ntname = NULL; /* never create a hardlink for this dll */
+    }
+
+  if (!forkable_ntname)
+    return;
+
+  PWCHAR next = wcpcpy (forkable_ntname, dirx_name);
+
+  if (!*forkable_ntname)
+    return; /* denominate */
+
+  if (type < DLL_LOAD)
+    wcpcpy (next, modname);
+  else
+    {
+      /* Avoid lots of extra directories for loaded dll's:
+       * mangle full path into one single directory name,
+       * just keep original filename intact. The original
+       * filename is necessary to serve as linked
+       * dependencies of dynamically loaded dlls. */
+      PWCHAR lastpathsep = wcsrchr (ntname, L'\\');
+      if (!lastpathsep)
+        {
+	  forkable_ntname = NULL;
+	  return;
+	}
+      *lastpathsep = L'\0';
+      HANDLE fh = dll_list::ntopenfile (ntname, NULL, FILE_DIRECTORY_FILE);
+      *lastpathsep = L'\\';
+
+      FILE_INTERNAL_INFORMATION fii = { 0 };
+      if (fh)
+	{
+	  read_fii (fh, &fii);
+	  NtClose (fh);
+	}
+      next += format_IndexNumber (next, -1, &fii.IndexNumber);
+      wcpcpy (next, lastpathsep);
+    }
+}
+
+/* Create the nominated hardlink for one indivitual dll,
+   inside another subdirectory when dynamically loaded. */
+bool
+dll::create_forkable ()
+{
+  if (!forkable_ntname || !*forkable_ntname)
+    return true; /* disabled */
+
+  if (!fhandle)
+    return false; /* impossible */
+
+  PWCHAR ntname = forkable_ntname;
+
+  PWCHAR last = NULL;
+  bool success = true;
+  if (type >= DLL_LOAD)
+    {
+      last = wcsrchr (ntname, L'\\');
+      if (!last)
+	return false;
+      *last = L'\0';
+      success = mkdirs (ntname, 1);
+      *last = L'\\';
+      if (!success)
+	return false;
+    }
+
+  int ntlen = wcslen (ntname);
+  int bufsize = sizeof (FILE_LINK_INFORMATION) + ntlen * sizeof (*ntname);
+  PFILE_LINK_INFORMATION pfli = (PFILE_LINK_INFORMATION) alloca (bufsize);
+
+  wcscpy (pfli->FileName, ntname);
+
+  pfli->FileNameLength = ntlen * sizeof (*ntname);
+  pfli->ReplaceIfExists = FALSE; /* allow concurrency */
+  pfli->RootDirectory = NULL;
+
+  IO_STATUS_BLOCK iosb;
+  NTSTATUS status = NtSetInformationFile (fhandle, &iosb, pfli, bufsize,
+					  FileLinkInformation);
+  debug_printf ("%y = NtSetInformationFile (%p, FileLink %W, iosb.Status %y)",
+		status, fhandle, pfli->FileName, iosb.Status);
+  if (NT_SUCCESS (status) || status == STATUS_OBJECT_NAME_COLLISION)
+    /* We've not found a performant way yet to protect fork against updates
+       to main executables and/or dlls that do not reside on the same NTFS
+       filesystem as the <cygroot>/var/run/cygfork/ directory.
+       But as long as the main executable can be hardlinked, dll redirection
+       works for any other hardlink-able dll, while non-hardlink-able dlls
+       are used from their original location. */
+    return true;
+
+  return false;
+}
+
+/* return the number of characters necessary to store one forkable name */
+size_t
+dll_list::forkable_ntnamesize (dll_type type, PCWCHAR fullntname, PCWCHAR modname)
+{
+  if (forkables_needs == forkables_impossible)
+    return 0;
+
+  if (!forkables_dirx_size)
+    {
+      DWORD forkables_mutex_size = 0;
+      bool needsep = false;
+      for (namepart const *part = forkable_nameparts; part->text; ++part)
+	{
+	  if (needsep)
+	    {
+	      forkables_dirx_size += wcslen (PATHSEP);
+	      forkables_mutex_size += wcslen (MUTEXSEP);
+	    }
+	  needsep = part->mutex_from_dir;
+	  int len = 0;
+	  if (part->textfunc)
+	    len = part->textfunc (NULL, 0);
+	  else
+	    len = wcslen (part->text);
+	  forkables_dirx_size += len;
+	  forkables_mutex_size += len;
+	}
+      /* trailing path sep */
+      forkables_dirx_size += wcslen (PATHSEP);
+      /* trailing zeros */
+      ++forkables_dirx_size;
+      ++forkables_mutex_size;
+
+      /* allocate here, to avoid cygheap size changes during fork */
+      forkables_dirx_ntname = (PWCHAR) cmalloc (HEAP_2_DLL,
+	  (forkables_dirx_size + forkables_mutex_size) *
+	    sizeof (*forkables_dirx_ntname));
+      *forkables_dirx_ntname = L'\0';
+
+      forkables_mutex_name = forkables_dirx_ntname + forkables_dirx_size;
+      *forkables_mutex_name = L'\0';
+    }
+
+  size_t ret = forkables_dirx_size;
+  if (type >= DLL_LOAD)
+    ret += format_IndexNumber (NULL, -1, NULL) + 1; /* one more directory */
+  return ret + wcslen (modname);
+}
+
+/* Prepare top-level names necessary to nominate individual DLL hardlinks,
+   eventually releasing/removing previous forkable hardlinks. */
+void
+dll_list::prepare_forkables_nomination ()
+{
+  if (!forkables_dirx_ntname)
+    return;
+
+  PWCHAR pbuf = nt_max_path_buf ();
+
+  bool needsep = false;
+  bool domutex = false;
+  namepart const *part;
+  for (part = forkable_nameparts; part->text; ++part)
+    {
+      if (part->mutex_from_dir)
+	domutex = true; /* mutex naming starts with first mutex_from_dir */
+      if (!domutex)
+	continue;
+      if (needsep)
+	pbuf += __small_swprintf (pbuf, L"%W", MUTEXSEP);
+      needsep = part->mutex_from_dir;
+      if (part->textfunc)
+	pbuf += part->textfunc (pbuf, -1);
+      else
+	pbuf += __small_swprintf (pbuf, L"%W", part->text);
+    }
+
+  if (!wcscmp (forkables_mutex_name, nt_max_path_buf ()))
+    return; /* nothing changed */
+
+  if (*forkables_mutex_name &&
+      wcscmp (forkables_mutex_name, nt_max_path_buf ()))
+    {
+      /* The mutex name has changed since last fork and we either have
+	 dlopen'ed a more recent or dlclose'd the most recent dll,
+	 so we will not use the current forkable hardlinks any more.
+	 Removing from the file system is done later, upon exit. */
+      close_mutex ();
+      denominate_forkables ();
+    }
+  wcscpy (forkables_mutex_name, nt_max_path_buf ());
+
+  pbuf = forkables_dirx_ntname;
+  needsep = false;
+  for (namepart const *part = forkable_nameparts; part->text; ++part)
+    {
+      if (needsep)
+	pbuf += __small_swprintf (pbuf, L"%W", PATHSEP);
+      needsep = part->mutex_from_dir;
+      if (part->textfunc)
+	pbuf += part->textfunc (pbuf, -1);
+      else
+	pbuf += __small_swprintf (pbuf, L"%W", part->text);
+    }
+  pbuf += __small_swprintf (pbuf, L"%W", PATHSEP);
+
+  debug_printf ("forkables dir %W", forkables_dirx_ntname);
+  debug_printf ("forkables mutex %W", forkables_mutex_name);
+}
+
+/* Test if creating hardlinks is necessary. If creating hardlinks is possible
+   in general, each individual dll is tested if its previously created
+   hardlink (if any, or the original file) still is the same.
+   Testing is protected against hardlink removal by concurrent processes. */
+void
+dll_list::update_forkables_needs ()
+{
+  dll *d;
+
+  if (forkables_needs == forkables_unknown)
+    {
+      /* check if filesystem of forkables dir is NTFS */
+      PWCHAR pbuf = nt_max_path_buf ();
+      for (namepart const *part = forkable_nameparts; part->text; ++part)
+	{
+	  if (part->mutex_from_dir)
+	    break; /* leading non-mutex-naming dirs, must exist anyway */
+	  if (part->textfunc)
+	    pbuf += part->textfunc (pbuf, -1);
+	  else
+	    pbuf += __small_swprintf (pbuf, L"%W", part->text);
+	}
+
+      UNICODE_STRING fn;
+      RtlInitUnicodeString (&fn, nt_max_path_buf ());
+
+      fs_info fsi;
+      if (fsi.update (&fn, NULL) &&
+/* FIXME: !fsi.is_readonly () && */
+	  fsi.is_ntfs ())
+	forkables_needs = forkables_disabled; /* check directory itself */
+      else
+	{
+	  debug_printf ("impossible, not on NTFS %W", fn.Buffer);
+	  forkables_needs = forkables_impossible;
+	}
+    }
+
+  if (forkables_needs == forkables_impossible)
+    return; /* we have not created any hardlink, nothing to clean up */
+
+  if (forkables_needs == forkables_disabled ||
+      forkables_needs == forkables_needless ||
+      forkables_needs == forkables_created)
+    {
+      /* (re-)check existence of forkables dir */
+      PWCHAR pbuf = nt_max_path_buf ();
+      for (namepart const *part = forkable_nameparts; part->text; ++part)
+	{
+	  if (part->textfunc)
+	    pbuf += part->textfunc (pbuf, -1);
+	  else
+	    pbuf += __small_swprintf (pbuf, L"%W", part->text);
+	  if (part->mutex_from_dir)
+	    break; /* up to first mutex-naming dir */
+	}
+      pbuf = nt_max_path_buf ();
+
+      HANDLE dh = ntopenfile (pbuf, NULL, FILE_DIRECTORY_FILE);
+      if (dh)
+	{
+	  NtClose (dh);
+	  if (forkables_needs == forkables_disabled)
+	    forkables_needs = forkables_needless;
+	}
+      else if (forkables_needs != forkables_disabled)
+	{
+	  debug_printf ("disabled, disappearing %W", pbuf);
+	  close_mutex ();
+	  denominate_forkables ();
+	  forkables_needs = forkables_disabled;
+	}
+      else
+	debug_printf ("disabled, missing %W", pbuf);
+    }
+
+  if (forkables_needs == forkables_disabled)
+    return;
+
+  if (forkables_needs == forkables_created)
+    {
+      /* already have created hardlinks in this process, ... */
+      forkables_needs = forkables_needless;
+      d = &start;
+      while ((d = d->next) != NULL)
+	if (d->forkable_ntname && !*d->forkable_ntname)
+	  {
+	    /* ... but another dll was loaded since last fork */
+	    debug_printf ("needed, since last fork loaded %W", d->ntname);
+	    forkables_needs = forkables_needed;
+	    break;
+	  }
+    }
+
+  if (forkables_needs > forkables_needless)
+    return; /* no need to check anything else */
+
+  if (forkables_needs != forkables_needless)
+    {
+      /* paranoia */
+      system_printf ("WARNING: invalid forkables_needs value %d",
+		     forkables_needs);
+      return;
+    }
+
+  if (!forkables_mutex)
+    {
+      /* debugging: check for ".unchecked" file in toplevel forkables dir,
+	 that is: always (try to) create the hardlinks.  Need to find out
+	 if just trying to create the hardlinks is faster than reading
+	 all the hardlink's internal and basic informations. */
+      PWCHAR pbuf = nt_max_path_buf ();
+      for (namepart const *part = forkable_nameparts; part->text; ++part)
+	{
+	  if (part->textfunc)
+	    pbuf += part->textfunc (pbuf, -1);
+	  else
+	    pbuf += __small_swprintf (pbuf, L"%W", part->text);
+	  if (part->mutex_from_dir)
+	    break; /* up to first mutex-naming dir */
+	}
+      pbuf += __small_swprintf (pbuf, L"%W.unchecked", PATHSEP);
+
+      pbuf = nt_max_path_buf ();
+      HANDLE dh = ntopenfile (pbuf);
+      if (dh)
+	{
+	  NtClose (dh);
+	  forkables_needs = forkables_needed;
+	  debug_printf ("needed, found %W", pbuf);
+	  return;
+	}
+    }
+
+  /* We have to check the main-executable and all dlls loaded so far via
+     their forked (if available, or their original) filename, if they are
+     still available for loading again. */
+  HANDLE fh = NULL;
+  d = &start;
+  while ((d = d->next) != NULL)
+    {
+      if (!d->fhandle)
+	continue;
+
+      if (fh)
+	{
+	  NtClose (fh);
+	  fh = NULL;
+	}
+
+      PWCHAR ntname = d->forkedntname ();
+      fh = ntopenfile (ntname);
+      if (!fh)
+	{
+	  debug_printf ("needed, something wrong with %W", ntname);
+	  forkables_needs = forkables_needed;
+	  break;
+	}
+
+      FILE_INTERNAL_INFORMATION fii_now;
+      if (!read_fii (d->fhandle, &d->fii, true) ||
+	  !read_fii (fh, &fii_now) ||
+	  d->fii.IndexNumber.QuadPart != fii_now.IndexNumber.QuadPart)
+        {
+	  debug_printf ("needed, found modified %W", ntname);
+	  forkables_needs = forkables_needed;
+	  break;
+	}
+
+      FILE_BASIC_INFORMATION fbi_now;
+      if (!read_fbi (d->fhandle, &d->fbi, true) ||
+	  !read_fbi (fh, &fbi_now) ||
+	  d->fbi.LastWriteTime.QuadPart != fbi_now.LastWriteTime.QuadPart)
+	{
+	  system_printf ("WARNING: changed by same file id %W"
+			 " (now lastwritetime %016X, old lastwritetime %016X)",
+			 ntname,
+			 fbi_now.LastWriteTime.QuadPart,
+			 d->fbi.LastWriteTime.QuadPart);
+	  forkables_needs = forkables_needed;
+	  break;
+	}
+    }
+  if (fh)
+    NtClose (fh);
+
+  if (forkables_needs == forkables_needless && !forkables_mutex)
+    {
+      /* debugging: check for ".needed" file in toplevel forkables dir */
+      PWCHAR pbuf = nt_max_path_buf ();
+      for (namepart const *part = forkable_nameparts; part->text; ++part)
+	{
+	  if (part->textfunc)
+	    pbuf += part->textfunc (pbuf, -1);
+	  else
+	    pbuf += __small_swprintf (pbuf, L"%W", part->text);
+	  if (part->mutex_from_dir)
+	    break; /* up to first mutex-naming dir */
+	}
+      pbuf += __small_swprintf (pbuf, L"%W.needed", PATHSEP);
+
+      pbuf = nt_max_path_buf ();
+      HANDLE dh = ntopenfile (pbuf);
+      if (dh)
+	{
+	  NtClose (dh);
+	  forkables_needs = forkables_needed;
+	  debug_printf ("needed, found %W", pbuf);
+	}
+    }
+}
+
+/* Create the nominated forkable hardlinks and directories as necessary,
+   mutex-protected to avoid concurrent processes removing them. */
+bool
+dll_list::update_forkables ()
+{
+  /* existence of mutex indicates that we use these hardlinks */
+  if (!forkables_mutex)
+    {
+      /* neither my parent nor myself did have need for hardlinks yet */
+      forkables_mutex = CreateMutexW (&sec_none, FALSE,
+				      forkables_mutex_name);
+      debug_printf ("%p = CreateMutexW (%W): %E",
+		    forkables_mutex, forkables_mutex_name);
+      if (!forkables_mutex)
+	return false;
+
+      /* Make sure another process does not rmdirs_synchronized () */
+      debug_printf ("WFSO (%p, %W, inf)...",
+		    forkables_mutex, forkables_mutex_name);
+      DWORD ret = WaitForSingleObject (forkables_mutex, INFINITE);
+      debug_printf ("%u = WFSO (%p, %W)",
+		    ret, forkables_mutex, forkables_mutex_name);
+      switch (ret)
+	{
+	case WAIT_OBJECT_0:
+	case WAIT_ABANDONED:
+	  break;
+	default:
+	  system_printf ("cannot wait for mutex %W: %E",
+			 forkables_mutex_name);
+	  return false;
+	}
+
+      BOOL bret = ReleaseMutex (forkables_mutex);
+      debug_printf ("%d = ReleaseMutex (%p, %W)",
+		    bret, forkables_mutex, forkables_mutex_name);
+    }
+
+  return create_forkables ();
+}
+
+/* Create the nominated forkable hardlinks and directories as necessary,
+   as well as the .local file for dll-redirection. */
+bool
+dll_list::create_forkables ()
+{
+  bool success = true;
+
+  int lastsepcount = 1; /* we have trailing pathsep */
+  for (namepart const *part = forkable_nameparts; part->text; ++part)
+    if (part->create_dir)
+      ++lastsepcount;
+
+  PWCHAR ntname = nt_max_path_buf ();
+  wcsncpy (ntname, forkables_dirx_ntname, NT_MAX_PATH);
+
+  if (!mkdirs (ntname, lastsepcount))
+    success = false;
+
+  if (success)
+    {
+      /* create the DotLocal file as empty file */
+      wcsncat (ntname, main_executable->modname, NT_MAX_PATH);
+      wcsncat (ntname, L".local", NT_MAX_PATH);
+
+      UNICODE_STRING fn;
+      RtlInitUnicodeString (&fn, ntname);
+
+      OBJECT_ATTRIBUTES oa;
+      InitializeObjectAttributes (&oa, &fn, 0, NULL,
+				  sec_none_nih.lpSecurityDescriptor);
+      HANDLE hlocal = NULL;
+      NTSTATUS status;
+      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_NON_DIRECTORY_FILE
+			     | FILE_SYNCHRONOUS_IO_NONALERT,
+			     NULL, 0);
+      if (NT_SUCCESS (status))
+	CloseHandle (hlocal);
+      else
+	success = false;
+      debug_printf ("%y = NtCreateFile (%p, %W)", status, hlocal, ntname);
+    }
+
+  if (success)
+    {
+      dll *d = &start;
+      while ((d = d->next))
+	if (!d->create_forkable ())
+	  d->nominate_forkable (NULL); /* never again */
+      debug_printf ("hardlinks created");
+    }
+
+  return success;
+}
+
+static void
+rmdirs_synchronized (WCHAR ntbuf[NT_MAX_PATH], int depth, int maxdepth,
+		     PFILE_DIRECTORY_INFORMATION pfdi, ULONG fdisize)
+{
+  if (depth == maxdepth)
+    {
+      debug_printf ("sync on %W", ntbuf);
+      /* calculate mutex name from path parts, using
+	 full path name length to allocate mutex name buffer */
+      WCHAR mutexname[wcslen (ntbuf)];
+      mutexname[0] = L'\0';
+      PWCHAR mutexnext = mutexname;
+
+      /* mutex name is formed by dir names */
+      int pathcount = 0;
+      for (namepart const *part = forkable_nameparts; part->text; ++part)
+	if (part->mutex_from_dir)
+	  ++pathcount;
+
+      PWCHAR pathseps[pathcount];
+
+      /* along the path separators split needed path parts */
+      int i = pathcount;
+      while (--i >= 0)
+	if ((pathseps[i] = wcsrchr (ntbuf, L'\\')))
+	  *pathseps[i] = L'\0';
+	else
+	  return; /* something's wrong */
+
+      /* build the mutex name from dir names */
+      for (i = 0; i < pathcount; ++i)
+	{
+	  if (i > 0)
+	    mutexnext = wcpcpy (mutexnext, MUTEXSEP);
+	  mutexnext = wcpcpy (mutexnext, &pathseps[i][1]);
+	  *pathseps[i] = L'\\'; /* restore full path */
+	}
+
+      HANDLE mutex = CreateMutexW (&sec_none_nih, TRUE, mutexname);
+      DWORD lasterror = GetLastError ();
+      debug_printf ("%p = CreateMutexW (%W): %E", mutex, mutexname);
+      if (mutex)
+	{
+	  if (lasterror != ERROR_ALREADY_EXISTS)
+	    rmdirs (ntbuf);
+	  BOOL bret = CloseHandle (mutex);
+	  debug_printf ("%d = CloseHandle (%p, %W): %E",
+			bret, mutex, mutexname);
+	}
+      return;
+    }
+
+  IO_STATUS_BLOCK iosb;
+  NTSTATUS status;
+
+  HANDLE hdir = dll_list::ntopenfile (ntbuf, &status,
+				 FILE_DIRECTORY_FILE |
+				 (depth ? FILE_DELETE_ON_CLOSE : 0));
+  if (!hdir)
+    return;
+
+  PWCHAR plast = ntbuf + wcslen (ntbuf);
+  while (NT_SUCCESS (status = NtQueryDirectoryFile (hdir,
+						    NULL, NULL, NULL, &iosb,
+						    pfdi, fdisize,
+						    FileDirectoryInformation,
+						    TRUE, NULL, FALSE)))
+    if (pfdi->FileAttributes & FILE_ATTRIBUTE_DIRECTORY)
+      {
+	int namelen = pfdi->FileNameLength / sizeof (WCHAR);
+	if (!wcsncmp (pfdi->FileName, L".", namelen) ||
+	    !wcsncmp (pfdi->FileName, L"..", namelen))
+	  continue;
+	*plast = L'\\';
+	wcsncpy (plast+1, pfdi->FileName, namelen);
+	plast[1+namelen] = L'\0';
+	rmdirs_synchronized (ntbuf, depth+1, maxdepth, pfdi, fdisize);
+	*plast = L'\0';
+      }
+  if (status != STATUS_NO_MORE_FILES)
+    debug_printf ("%y = NtQueryDirectoryFile (%p, io %y, info %d)",
+		  status, hdir, iosb.Status, iosb.Information);
+  CloseHandle (hdir);
+}
+
+/* Try to lock the mutex handle with almost no timeout, then close the
+   mutex handle.  Locking before closing is to get the mutex closing
+   promoted synchronously, otherways we might end up with no one
+   succeeding in create-with-lock, which is the precondition
+   to actually remove the hardlinks from the filesystem. */
+bool
+dll_list::close_mutex ()
+{
+  if (!forkables_mutex || !*forkables_mutex_name)
+    return false;
+
+  HANDLE hmutex = forkables_mutex;
+  forkables_mutex = NULL;
+
+  bool locked = false;
+  DWORD ret = WaitForSingleObject (hmutex, 1);
+  debug_printf ("%u = WFSO (%p, %W, 1)",
+		ret, hmutex, forkables_mutex_name);
+  switch (ret)
+    {
+    case WAIT_OBJECT_0:
+    case WAIT_ABANDONED:
+      locked = true;
+      break;
+    case WAIT_TIMEOUT:
+      break;
+    default:
+      system_printf ("error locking mutex %W: %E", forkables_mutex_name);
+      break;
+    }
+  BOOL bret = CloseHandle (hmutex);
+  debug_printf ("%d = CloseHandle (%p, %W): %E",
+		bret, hmutex, forkables_mutex_name);
+  return locked;
+}
+
+/* Release the forkable hardlinks, and remove them if the
+   mutex can be create-locked after locked-closing. */
+void
+dll_list::cleanup_forkables ()
+{
+  bool locked = close_mutex ();
+
+  if (!forkables_dirx_ntname)
+    return;
+
+  /* Start the removal below with current forkables dir,
+     which is cleaned in denominate_forkables (). */
+  PWCHAR buf = nt_max_path_buf ();
+  PWCHAR pathsep = wcpncpy (buf, forkables_dirx_ntname, NT_MAX_PATH);
+  buf[NT_MAX_PATH-1] = L'\0';
+
+  denominate_forkables ();
+
+  if (!locked)
+    return;
+
+  /* drop last path separator */
+  while (--pathsep >= buf && *pathsep != L'\\');
+  *pathsep = L'\0';
+
+  try_remove_forkables (buf, pathsep - buf, NT_MAX_PATH);
+}
+
+void
+dll_list::try_remove_forkables (PWCHAR dirbuf, size_t dirlen, size_t dirbufsize)
+{
+  /* Instead of just the current forkables, try to remove any forkables
+     found, to ensure some cleanup even in situations like power-loss. */
+  PWCHAR end = dirbuf + wcslen (dirbuf);
+  int backcount = 0;
+  for (namepart const *part = forkable_nameparts; part->text; ++part)
+    if (part->create_dir)
+      {
+	/* drop one path separator per create_dir */
+	while (--end >= dirbuf && *end != L'\\');
+	if (end < dirbuf)
+	  return;
+	*end = L'\0';
+	++backcount;
+      }
+
+  /* reading one at a time to reduce stack pressure */
+  struct {
+    FILE_DIRECTORY_INFORMATION fdi;
+    WCHAR buf[NAME_MAX];
+  } fdibuf;
+  rmdirs_synchronized (dirbuf, 0, backcount, &fdibuf.fdi, sizeof (fdibuf));
+}
+
+void
+dll_list::denominate_forkables ()
+{
+  if (forkables_dirx_ntname)
+    {
+      *forkables_dirx_ntname = L'\0';
+      *forkables_mutex_name = L'\0';
+    }
+
+  dll *d = &start;
+  while ((d = d->next))
+    d->nominate_forkable (forkables_dirx_ntname);
+}
+
 /* Set or clear HANDLE_FLAG_INHERIT for all handles necessary
    to maintain forkables-hardlinks. */
 void
@@ -35,6 +1027,9 @@ dll_list::set_forkables_inheritance (bool inherit)
   DWORD mask = HANDLE_FLAG_INHERIT;
   DWORD flags = inherit ? HANDLE_FLAG_INHERIT : 0;
 
+  if (forkables_mutex)
+    SetHandleInformation (forkables_mutex, mask, flags);
+
   dll *d = &start;
   while ((d = d->next))
     if (d->fhandle)
@@ -45,11 +1040,52 @@ dll_list::set_forkables_inheritance (bool inherit)
 void
 dll_list::request_forkables ()
 {
+  /* Even on forkables_impossible, keep the number of open handles
+     stable across the fork, and close them when releasing only. */
+  prepare_forkables_nomination ();
+
+  update_forkables_needs ();
+
   set_forkables_inheritance (true);
+
+  if (forkables_needs <= forkables_needless)
+    return;
+
+  dll *d = &start;
+  while ((d = d->next))
+    d->nominate_forkable (forkables_dirx_ntname);
+
+  bool updated = update_forkables ();
+
+  if (!updated)
+    forkables_needs = forkables_needless;
+  else
+    forkables_needs = forkables_created;
 }
 
+
 void
 dll_list::release_forkables ()
 {
   set_forkables_inheritance (false);
+
+  if (forkables_needs == forkables_impossible)
+    {
+      cleanup_forkables ();
+
+      dll *d = &start;
+      while ((d = d->next))
+	if (d->fhandle)
+	  {
+	    NtClose (d->fhandle);
+	    d->fhandle = NULL;
+	    d->forkable_ntname = NULL;
+	  }
+
+      if (forkables_dirx_ntname) {
+	cfree (forkables_dirx_ntname);
+	forkables_dirx_ntname = NULL;
+	forkables_mutex_name = NULL;
+      }
+    }
 }
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index d4b2afb..e414a26 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -28,6 +28,7 @@ details. */
 #include "cygtls.h"
 #include "tls_pbuf.h"
 #include "child_info.h"
+#include "dll_init.h"
 
 class pinfo_basic: public _pinfo
 {
@@ -225,6 +226,8 @@ pinfo::exit (DWORD n)
   int exitcode = self->exitcode & 0xffff;
   if (!self->cygstarted)
     exitcode = ((exitcode & 0xff) << 8) | ((exitcode >> 8) & 0xff);
+  sigproc_printf ("Calling dlls.cleanup_forkables n %y, exitcode %y", n, exitcode);
+  dlls.cleanup_forkables ();
   sigproc_printf ("Calling ExitProcess n %y, exitcode %y", n, exitcode);
   if (!TerminateProcess (GetCurrentProcess (), exitcode))
     system_printf ("TerminateProcess failed, %E");
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 15fb8ce..7f5354e 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -689,8 +689,8 @@ check_dir_not_empty (HANDLE dir, path_conv &pc)
   return STATUS_SUCCESS;
 }
 
-NTSTATUS
-unlink_nt (path_conv &pc)
+static NTSTATUS
+_unlink_nt (path_conv &pc, bool shareable)
 {
   NTSTATUS status;
   HANDLE fh, fh_ro = NULL;
@@ -771,6 +771,9 @@ retry_open:
      bin so that it actually disappears from its directory even though its
      in use.  Otherwise, if opening doesn't fail, the file is not in use and
      we can go straight to setting the delete disposition flag.
+     However, while we have the file open with FILE_SHARE_DELETE, using
+     this file via another hardlink for anything other than DELETE by
+     concurrent processes fails. The 'shareable' argument is to prevent this.
 
      NOTE: The missing sharing modes FILE_SHARE_READ and FILE_SHARE_WRITE do
 	   NOT result in a STATUS_SHARING_VIOLATION, if another handle is
@@ -780,7 +783,10 @@ retry_open:
 	   will succeed.  So, apparently there is no reliable way to find out
 	   if a file is already open elsewhere for other purposes than
 	   reading and writing data.  */
-  status = NtOpenFile (&fh, access, &attr, &io, FILE_SHARE_DELETE, flags);
+  if (shareable)
+    status = STATUS_SHARING_VIOLATION;
+  else
+    status = NtOpenFile (&fh, access, &attr, &io, FILE_SHARE_DELETE, flags);
   /* STATUS_SHARING_VIOLATION is what we expect. STATUS_LOCK_NOT_GRANTED can
      be generated under not quite clear circumstances when trying to open a
      file on NFS with FILE_SHARE_DELETE only.  This has been observed with
@@ -1026,6 +1032,18 @@ out:
   return status;
 }
 
+NTSTATUS
+unlink_nt (path_conv &pc)
+{
+  return _unlink_nt (pc, false);
+}
+
+NTSTATUS
+unlink_nt_shareable (path_conv &pc)
+{
+  return _unlink_nt (pc, true);
+}
+
 extern "C" int
 unlink (const char *ourname)
 {
-- 
2.7.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 0/6] Protect fork() against dll- and exe-updates.
@ 2016-03-30 18:54 Michael Haubenwallner
  2016-03-30 18:54 ` [PATCH 4/6] forkables: Protect fork against dll-, exe-updates Michael Haubenwallner
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Michael Haubenwallner @ 2016-03-30 18:54 UTC (permalink / raw)
  To: cygwin-patches; +Cc: michael.haubenwallner

Hi,

this is the updated and split series of patches to use hardlinks
for creating the child process by fork(), in reply to
https://cygwin.com/ml/cygwin-developers/2016-01/msg00002.html
https://cygwin.com/ml/cygwin-developers/2016-03/msg00005.html
http://thread.gmane.org/gmane.os.cygwin.devel/1378

Thanks for review!
/haubi/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 2/6] forkables: Track main executable and cygwin1.dll.
  2016-03-30 18:54 [PATCH 0/6] Protect fork() against dll- and exe-updates Michael Haubenwallner
                   ` (2 preceding siblings ...)
  2016-03-30 18:54 ` [PATCH 3/6] forkables: Create forkable hardlinks, yet unused Michael Haubenwallner
@ 2016-03-30 18:54 ` Michael Haubenwallner
  2016-03-30 18:55 ` [PATCH 0/6] Protect fork() against dll- and exe-updates Daniel Colascione
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Michael Haubenwallner @ 2016-03-30 18:54 UTC (permalink / raw)
  To: cygwin-patches; +Cc: michael.haubenwallner

In preparation to protect fork() against dll- and exe-updates, even for
the main executable and cygwin1.dll store the file name as full NT path,
and keep handles to the files open for subsequent hardlink creation.
Create the child process using the main executable's file name converted
from the full NT path stored before.

	* dll_init.cc (dll_list::alloc): Search for DLL_SELF type entry
	with module name like for DLL_LINK, use full NT path to search
	for DLL_LOAD type only.  For DLL_SELF type do not indicate
	having a destructor to be called.
	(dll_list::find): Ignore DLL_SELF type entries.
	(dll_list::init): Ditto.  Call track_self method.
	(dll_list::track_self): New.
	(dll_list::load_after_fork): Call track_self method.
	* dll_init.h (enum dll_type): Add DLL_SELF, for the main
	executable and cygwin1.dll.
	(struct dll_list): Declare method track_self.  Declare member
	variable main_executable.
	* fork.cc (frok::parent): Use ntname from dlls.main_executable
	to create child process, converted to short path using
	dll_list::buffered_shortname.
---
 winsup/cygwin/dll_init.cc | 26 ++++++++++++++++++++------
 winsup/cygwin/dll_init.h  |  3 +++
 winsup/cygwin/fork.cc     | 15 ++++++++++++---
 3 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/winsup/cygwin/dll_init.cc b/winsup/cygwin/dll_init.cc
index 0494297..fd807c9 100644
--- a/winsup/cygwin/dll_init.cc
+++ b/winsup/cygwin/dll_init.cc
@@ -343,8 +343,9 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type)
   guard (true);
   /* Already loaded?  For linked DLLs, only compare the basenames.  Linked
      DLLs are loaded using just the basename and the default DLL search path.
-     The Windows loader picks up the first one it finds.  */
-  dll *d = (type == DLL_LINK) ? dlls.find_by_modname (modname) : dlls[ntname];
+     The Windows loader picks up the first one it finds.
+     This also applies to cygwin1.dll and the main-executable (DLL_SELF).  */
+  dll *d = (type != DLL_LOAD) ? dlls.find_by_modname (modname) : dlls[ntname];
   if (d)
     {
       if (!in_forkee)
@@ -380,7 +381,8 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type)
       wcscpy (d->ntname, ntname);
       d->modname = d->ntname + (modname - ntname);
       d->handle = h;
-      d->has_dtors = true;
+      /* DLL_SELF dtors (main-executable, cygwin1.dll) are run elsewhere */
+      d->has_dtors = type != DLL_SELF;
       d->p = p;
       d->ndeps = 0;
       d->deps = NULL;
@@ -529,7 +531,7 @@ dll_list::find (void *retaddr)
 
   dll *d = &start;
   while ((d = d->next))
-    if (d->handle == h)
+    if (d->type != DLL_SELF && d->handle == h)
       break;
   return d;
 }
@@ -579,11 +581,22 @@ dll_list::detach (void *retaddr)
 void
 dll_list::init ()
 {
+  track_self ();
+
   /* Walk the dll chain, initializing each dll */
   dll *d = &start;
   dll_global_dtors_recorded = d->next != NULL;
   while ((d = d->next))
-    d->init ();
+    if (d->type != DLL_SELF) /* linked and early loaded dlls */
+      d->init ();
+}
+
+void
+dll_list::track_self ()
+{
+  /* for cygwin1.dll and main-executable: maintain hardlinks only */
+  alloc (cygwin_hmodule, user_data, DLL_SELF);
+  main_executable = alloc (GetModuleHandle (NULL), user_data, DLL_SELF);
 }
 
 #define A64K (64 * 1024)
@@ -650,7 +663,7 @@ dll_list::reserve_space ()
 
 /* Reload DLLs after a fork.  Iterates over the list of dynamically loaded
    DLLs and attempts to load them in the same place as they were loaded in the
-   parent. */
+   parent.  Updates main-executable and cygwin1.dll tracking. */
 void
 dll_list::load_after_fork (HANDLE parent)
 {
@@ -662,6 +675,7 @@ dll_list::load_after_fork (HANDLE parent)
   in_load_after_fork = true;
   if (reload_on_fork)
     load_after_fork_impl (parent, dlls.istart (DLL_LOAD), 0);
+  track_self ();
   in_load_after_fork = false;
 }
 
diff --git a/winsup/cygwin/dll_init.h b/winsup/cygwin/dll_init.h
index d4cbd3d..c50f889 100644
--- a/winsup/cygwin/dll_init.h
+++ b/winsup/cygwin/dll_init.h
@@ -43,6 +43,7 @@ struct per_module
 typedef enum
 {
   DLL_NONE,
+  DLL_SELF, /* main-program.exe, cygwin1.dll */
   DLL_LINK,
   DLL_LOAD,
   DLL_ANY
@@ -82,6 +83,7 @@ struct dll
 
 class dll_list
 {
+  void track_self ();
   void set_forkables_inheritance (bool);
 
   dll *end;
@@ -92,6 +94,7 @@ class dll_list
   static WCHAR NO_COPY nt_max_path_buffer[NT_MAX_PATH];
 public:
   /* forkables */
+  dll *main_executable;
   void request_forkables ();
   void release_forkables ();
 
diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index 4361f58..7bc3dec 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -188,7 +188,7 @@ frok::child (volatile char * volatile here)
 
   MALLOC_CHECK;
 
-  /* load dynamic dlls, if any */
+  /* load dynamic dlls, if any, re-track main-executable and cygwin1.dll */
   dlls.load_after_fork (hParent);
 
   cygheap->fdtab.fixup_after_fork (hParent);
@@ -354,11 +354,20 @@ frok::parent (volatile char * volatile stack_here)
     {
       dlls.request_forkables ();
 
+      PCWCHAR forking_progname = NULL;
+      if (dlls.main_executable)
+        forking_progname = dll_list::buffered_shortname
+			   (dlls.main_executable->ntname);
+      if (!forking_progname || !*forking_progname)
+	forking_progname = myself->progname;
+
       debug_printf ("CreateProcessW (%W, %W, 0, 0, 1, %y, 0, 0, %p, %p)",
-		    myself->progname, myself->progname, c_flags, &si, &pi);
+		    forking_progname, myself->progname, c_flags, &si, &pi);
 
       hchild = NULL;
-      rc = CreateProcessW (myself->progname,	/* image to run */
+      /* cygwin1.dll - linked to shortname dll - may reuse the forking_progname
+	 buffer, even in case of failure: don't reuse forking_progname later */
+      rc = CreateProcessW (forking_progname,	/* image to run */
 			   GetCommandLineW (),	/* Take same space for command
 						   line as in parent to make
 						   sure child stack is allocated
-- 
2.7.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/6] forkables: Store dll file name as full NT path.
  2016-03-30 18:54 [PATCH 0/6] Protect fork() against dll- and exe-updates Michael Haubenwallner
  2016-03-30 18:54 ` [PATCH 4/6] forkables: Protect fork against dll-, exe-updates Michael Haubenwallner
@ 2016-03-30 18:54 ` Michael Haubenwallner
  2016-03-30 18:54 ` [PATCH 3/6] forkables: Create forkable hardlinks, yet unused Michael Haubenwallner
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Michael Haubenwallner @ 2016-03-30 18:54 UTC (permalink / raw)
  To: cygwin-patches; +Cc: michael.haubenwallner

In preparation to protect fork() against dll- and exe-updates, store
any dll file name as full NT path.  Additionally, keep a file handle
open for each loaded dll file for subsequent hardlink creation.  The
earlier we open a handle to the dll file, the lower is the chance the
dll file has been renamed since first loading.  To lower that chance
even more, the dll file handle is inherited by child processes rather
than opened again.

	* Makefile.in (DLL_OFILES): Add forkable.o.
	* dll_init.h (struct dll): Declare member fhandle.
	Rename member variable name to ntname.
	(struct dll_list): Declare private method
	set_forkables_inheritance.  Declare public methods
	request_forkables, release_forkables.  Declare public static
	methods ntopenfile, form_ntname, form_shortname, nt_max_path_buf,
	buffered_ntname, buffered_shortname.
	(dll_list::operator []): Use PCWCHAR rather than const PWCHAR.
	(dll_list::find_by_modname): Ditto.
	* dll_init.cc (in_load_after_fork): Define earlier in file.
	(struct dll_list): Rename member variable name to ntname.
	Define nt_max_path_buffer variable.
	Implement static methods form_ntname, form_shortname, ntopenfile.
	(dll_list::operator []): Use PCWCHAR rather than const PWCHAR.
	(dll_list::find_by_modname): Ditto.
	(reserve_at): Ditto.
	(release_at): Ditto.
	(dll_list::alloc): Use nt_max_path_buf method instead of local
	buffer.  Store module file name as full NT path, convert using
	the form_ntname static method.  Open file handle as fhandle for
	the loaded dll.
	(dll_list::detach): Close the fhandle.
	(dll_list::load_after_fork): Call release_forkables method.
	Call load_after_fork_impl only when reload_on_fork is set.
	* fork.cc (frok::child): Call dlls.load_after_fork even without
	need to dynamically load dlls, to release_forkables at least.
	(frok::parent): Call dlls.request_forkables before
	CreateProcessW, dlls.release_forkables afterwards.
	* forkable.cc: New file.
	(struct dll_list): Implement methods set_forkables_inheritance,
	request_forkables, release_forkables.
---
 winsup/cygwin/Makefile.in |   1 +
 winsup/cygwin/dll_init.cc | 250 +++++++++++++++++++++++++++++++++++++---------
 winsup/cygwin/dll_init.h  |  37 ++++++-
 winsup/cygwin/fork.cc     |  33 +++---
 winsup/cygwin/forkable.cc |  55 ++++++++++
 5 files changed, 310 insertions(+), 66 deletions(-)
 create mode 100644 winsup/cygwin/forkable.cc

diff --git a/winsup/cygwin/Makefile.in b/winsup/cygwin/Makefile.in
index 43919bd..15ddce8 100644
--- a/winsup/cygwin/Makefile.in
+++ b/winsup/cygwin/Makefile.in
@@ -307,6 +307,7 @@ DLL_OFILES:= \
 	flock.o \
 	fnmatch.o \
 	fork.o \
+	forkable.o \
 	fts.o \
 	ftw.o \
 	getopt.o \
diff --git a/winsup/cygwin/dll_init.cc b/winsup/cygwin/dll_init.cc
index 8deceb9..0494297 100644
--- a/winsup/cygwin/dll_init.cc
+++ b/winsup/cygwin/dll_init.cc
@@ -33,10 +33,162 @@ extern void __stdcall check_sanity_and_sync (per_process *);
 
 dll_list dlls;
 
+WCHAR NO_COPY dll_list::nt_max_path_buffer[NT_MAX_PATH];
+
 muto dll_list::protect;
 
 static bool dll_global_dtors_recorded;
 
+/* We need the in_load_after_fork flag so dll_dllcrt0_1 can decide at fork
+   time if this is a linked DLL or a dynamically loaded DLL.  In either case,
+   both, cygwin_finished_initializing and in_forkee are true, so they are not
+   sufficient to discern the situation. */
+static bool NO_COPY in_load_after_fork;
+
+/* Into ntbuf with ntbufsize, prints name prefixed with "\\??\\"
+   or "\\??\\UNC" as necessary to form the native NT path name.
+   Returns the end of the resulting string in ntbuf.
+   Supports using (a substring of) ntbuf as name argument. */
+PWCHAR dll_list::form_ntname (PWCHAR ntbuf, size_t ntbufsize, PCWCHAR name)
+{
+  while (true)
+    {
+      /* avoid using path_conv here: cygheap might not be
+	 initialized when started from non-cygwin process,
+	 or still might be frozen in_forkee */
+      if (name[0] == L'\0' || ntbufsize < 8)
+	break;
+      if (name[1] == L':') /* short Win32 drive letter path name */
+	{
+	  int winlen = min (ntbufsize - 5, wcslen (name));
+	  if (ntbuf + 4 != name)
+	    memmove (ntbuf + 4, name, sizeof (*ntbuf) * winlen);
+	  wcsncpy (ntbuf, L"\\??\\", 4);
+	  ntbuf += 4 + winlen;
+	  break;
+	}
+      if (!wcsncmp (name, L"\\\\?\\", 4)) /* long Win32 path name */
+	{
+	  int winlen = min (ntbufsize - 1, wcslen (name));
+	  if (ntbuf != name)
+	    memmove (ntbuf, name, sizeof (*ntbuf) * winlen);
+	  ntbuf[1] = L'?';
+	  ntbuf += winlen;
+	  break;
+	}
+      if (!wcsncmp (name, L"\\\\", 2)) /* short Win32 UNC path name */
+	{
+	  name += 1; /* skip first backslash */
+	  int winlen = min (ntbufsize - 8, wcslen (name));
+	  if (ntbuf + 7 != name)
+	    memmove (ntbuf + 7, name, sizeof (*ntbuf) * winlen);
+	  wcsncpy (ntbuf, L"\\??\\UNC", 7);
+	  ntbuf += 7 + winlen;
+	  break;
+	}
+      if (!wcsncmp (name, L"\\??\\", 4)) /* already a long NT path name */
+	{
+	  int winlen = min (ntbufsize - 1, wcslen (name));
+	  if (ntbuf != name)
+	    memmove (ntbuf, name, sizeof (*ntbuf) * winlen);
+	  ntbuf += winlen;
+	  break;
+	}
+      system_printf ("WARNING: invalid path name '%W'", name);
+      break;
+    }
+  if (ntbufsize)
+    *ntbuf = L'\0';
+  return ntbuf;
+}
+
+/* Into shortbuf with shortbufsize, prints name with "\\??\\"
+   or "\\??\\UNC" prefix removed/modified as necessary to form
+   the short Win32 path name.
+   Returns the end of the resulting string in shortbuf.
+   Supports using (a substring of) shortbuf as name argument. */
+PWCHAR
+dll_list::form_shortname (PWCHAR shortbuf, size_t shortbufsize, PCWCHAR name)
+{
+  while (true)
+    {
+      /* avoid using path_conv here: cygheap might not be
+	 initialized when started from non-cygwin process,
+	 or still might be frozen in_forkee */
+      if (name[0] == L'\0' || shortbufsize < 2)
+	break;
+      if (name[0] == L'\\' &&
+	  (name[1] == L'\\' || name[1] == L'?') &&
+	  name[2] == L'?' &&
+	  name[3] == L'\\') /* long Win32 or NT path name */
+	 name += 4;
+      if (name[1] == L':') /* short Win32 drive letter path name */
+	{
+	  int ntlen = min (shortbufsize - 1, wcslen (name));
+	  if (shortbuf != name)
+	    memmove (shortbuf, name, sizeof (*shortbuf) * ntlen);
+	  shortbuf += ntlen;
+	  break;
+	}
+      if (!wcsncmp (name, L"UNC\\", 4)) /* UNC path name */
+	{
+	  name += 3; /* skip "UNC" */
+	  int winlen = min (shortbufsize - 2, wcslen (name));
+	  if (shortbuf + 1 != name)
+	    memmove (shortbuf + 1, name, sizeof (*shortbuf) * winlen);
+	  shortbuf[0] = L'\\';
+	  shortbuf += 1 + winlen;
+	  break;
+	}
+      if (!wcsncmp (name, L"\\\\", 2)) /* already a short Win32 UNC path name */
+	{
+	  int winlen = min (shortbufsize - 1, wcslen (name));
+	  if (shortbuf != name)
+	    memmove (shortbuf, name, sizeof (*shortbuf) * winlen);
+	  shortbuf += winlen;
+	  break;
+	}
+      system_printf ("WARNING: invalid path name '%W'", name);
+      break;
+    }
+  if (shortbufsize)
+    *shortbuf = L'\0';
+  return shortbuf;
+}
+
+/* easy use of NtOpenFile */
+HANDLE
+dll_list::ntopenfile (PCWCHAR ntname, NTSTATUS *pstatus, ULONG openopts)
+{
+  NTSTATUS status;
+  if (!pstatus)
+    pstatus = &status;
+
+  UNICODE_STRING fn;
+  RtlInitUnicodeString (&fn, ntname);
+
+  OBJECT_ATTRIBUTES oa;
+  InitializeObjectAttributes (&oa, &fn, 0, NULL, NULL);
+
+  ACCESS_MASK access = FILE_READ_ATTRIBUTES;
+  if (openopts & FILE_DELETE_ON_CLOSE)
+    access |= DELETE;
+  if (openopts & FILE_DIRECTORY_FILE)
+    access |= FILE_LIST_DIRECTORY;
+
+  access |= SYNCHRONIZE;
+  openopts |= FILE_SYNCHRONOUS_IO_NONALERT;
+
+  HANDLE fh = NULL;
+  ULONG share = FILE_SHARE_VALID_FLAGS;
+  IO_STATUS_BLOCK iosb;
+  *pstatus = NtOpenFile (&fh, access, &oa, &iosb, share, openopts);
+  debug_printf ("%y = NtOpenFile (%p, a %xh, sh %xh, o %xh, io %y, '%W')",
+      *pstatus, fh, access, share, openopts, iosb.Status, fn.Buffer);
+
+  return NT_SUCCESS(*pstatus) ? fh : NULL;
+}
+
 /* Run destructors for all DLLs on exit. */
 void
 dll_global_dtors ()
@@ -151,11 +303,11 @@ dll::init ()
    of dll_list::alloc, as well as the comment preceeding the definition of
    the in_load_after_fork bool later in the file. */
 dll *
-dll_list::operator[] (const PWCHAR name)
+dll_list::operator[] (PCWCHAR ntname)
 {
   dll *d = &start;
   while ((d = d->next) != NULL)
-    if (!wcscasecmp (name, d->name))
+    if (!wcscasecmp (ntname, d->ntname))
       return d;
 
   return NULL;
@@ -163,7 +315,7 @@ dll_list::operator[] (const PWCHAR name)
 
 /* Look for a dll based on the basename. */
 dll *
-dll_list::find_by_modname (const PWCHAR modname)
+dll_list::find_by_modname (PCWCHAR modname)
 {
   dll *d = &start;
   while ((d = d->next) != NULL)
@@ -180,37 +332,29 @@ dll *
 dll_list::alloc (HINSTANCE h, per_process *p, dll_type type)
 {
   /* Called under loader lock conditions so this function can't be called
-     multiple times in parallel.  A static buffer is safe. */
-  static WCHAR buf[NT_MAX_PATH];
-  GetModuleFileNameW (h, buf, NT_MAX_PATH);
-  PWCHAR name = buf;
-  if (!wcsncmp (name, L"\\\\?\\", 4))
-    {
-      name += 4;
-      if (!wcsncmp (name, L"UNC\\", 4))
-	{
-	  name += 2;
-	  *name = L'\\';
-	}
-    }
-  DWORD namelen = wcslen (name);
-  PWCHAR modname = wcsrchr (name, L'\\') + 1;
+     multiple times in parallel.  The static buffer is safe. */
+  PWCHAR ntname = nt_max_path_buf ();
+  GetModuleFileNameW (h, ntname, NT_MAX_PATH);
+  PWCHAR modname = form_ntname (ntname, NT_MAX_PATH, ntname);
+  DWORD ntnamelen = modname - ntname;
+  while (modname > ntname && *(modname - 1) != L'\\')
+    --modname;
 
   guard (true);
   /* Already loaded?  For linked DLLs, only compare the basenames.  Linked
      DLLs are loaded using just the basename and the default DLL search path.
      The Windows loader picks up the first one it finds.  */
-  dll *d = (type == DLL_LINK) ? dlls.find_by_modname (modname) : dlls[name];
+  dll *d = (type == DLL_LINK) ? dlls.find_by_modname (modname) : dlls[ntname];
   if (d)
     {
       if (!in_forkee)
 	d->count++;	/* Yes.  Bump the usage count. */
       else if (d->handle != h)
 	fabort ("%W: Loaded to different address: parent(%p) != child(%p)",
-		name, d->handle, h);
+		ntname, d->handle, h);
       /* If this DLL has been linked against, and the full path differs, try
 	 to sanity check if this is the same DLL, just in another path. */
-      else if (type == DLL_LINK && wcscasecmp (name, d->name)
+      else if (type == DLL_LINK && wcscasecmp (ntname, d->ntname)
 	       && (d->p.data_start != p->data_start
 		   || d->p.data_start != p->data_start
 		   || d->p.bss_start != p->bss_start
@@ -222,19 +366,19 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type)
 		" child loaded: %W\n"
 		"The DLLs differ, so it's not safe to run the forked child.\n"
 		"Make sure to remove the offending DLL before trying again.",
-		d->name, name);
+		d->ntname, ntname);
       d->p = p;
     }
   else
     {
       /* FIXME: Change this to new at some point. */
-      d = (dll *) cmalloc (HEAP_2_DLL, sizeof (*d) + (namelen * sizeof (*name)));
+      d = (dll *) cmalloc (HEAP_2_DLL, sizeof (*d) + (ntnamelen * sizeof (*ntname)));
 
       /* Now we've allocated a block of information.  Fill it in with the
 	 supplied info about this DLL. */
       d->count = 1;
-      wcscpy (d->name, name);
-      d->modname = d->name + (modname - name);
+      wcscpy (d->ntname, ntname);
+      d->modname = d->ntname + (modname - ntname);
       d->handle = h;
       d->has_dtors = true;
       d->p = p;
@@ -243,6 +387,11 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type)
       d->image_size = ((pefile*)h)->optional_hdr ()->SizeOfImage;
       d->preferred_base = (void*) ((pefile*)h)->optional_hdr()->ImageBase;
       d->type = type;
+      NTSTATUS status;
+      d->fhandle = ntopenfile (d->ntname, &status);
+      if (!d->fhandle)
+	system_printf ("Unable (ntstatus %y) to open file %W",
+		       status, d->ntname);
       append (d);
       if (type == DLL_LOAD)
 	loaded_dlls++;
@@ -408,6 +557,11 @@ dll_list::detach (void *retaddr)
 	  if (!exit_state)
 	    __cxa_finalize (d->handle);
 	  d->run_dtors ();
+	  if (d->fhandle)
+	    {
+	      NtClose (d->fhandle);
+	      d->fhandle = NULL;
+	    }
 	  d->prev->next = d->next;
 	  if (d->next)
 	    d->next->prev = d->prev;
@@ -440,7 +594,7 @@ dll_list::init ()
    to clobber the dll's target address range because it often overlaps.
  */
 static PVOID
-reserve_at (const PWCHAR name, PVOID here, PVOID dll_base, DWORD dll_size)
+reserve_at (PCWCHAR name, PVOID here, PVOID dll_base, DWORD dll_size)
 {
   DWORD size;
   MEMORY_BASIC_INFORMATION mb;
@@ -469,7 +623,7 @@ reserve_at (const PWCHAR name, PVOID here, PVOID dll_base, DWORD dll_size)
 
 /* Release the memory previously allocated by "reserve_at" above. */
 static void
-release_at (const PWCHAR name, PVOID here)
+release_at (PCWCHAR name, PVOID here)
 {
   if (!VirtualFree (here, 0, MEM_RELEASE))
     fabort ("couldn't release memory %p for '%W' alignment, %E\n",
@@ -494,23 +648,20 @@ dll_list::reserve_space ()
 	      d->modname, d->handle);
 }
 
-/* We need the in_load_after_fork flag so dll_dllcrt0_1 can decide at fork
-   time if this is a linked DLL or a dynamically loaded DLL.  In either case,
-   both, cygwin_finished_initializing and in_forkee are true, so they are not
-   sufficient to discern the situation. */
-static bool NO_COPY in_load_after_fork;
-
 /* Reload DLLs after a fork.  Iterates over the list of dynamically loaded
    DLLs and attempts to load them in the same place as they were loaded in the
    parent. */
 void
 dll_list::load_after_fork (HANDLE parent)
 {
+  release_forkables ();
+
   // moved to frok::child for performance reasons:
   // dll_list::reserve_space();
 
   in_load_after_fork = true;
-  load_after_fork_impl (parent, dlls.istart (DLL_LOAD), 0);
+  if (reload_on_fork)
+    load_after_fork_impl (parent, dlls.istart (DLL_LOAD), 0);
   in_load_after_fork = false;
 }
 
@@ -544,33 +695,34 @@ void dll_list::load_after_fork_impl (HANDLE parent, dll* d, int retries)
 	   dll's protective reservation from step 1
 	 */
 	if (!retries && !VirtualFree (d->handle, 0, MEM_RELEASE))
-	  fabort ("unable to release protective reservation for %W (%p), %E",
-		  d->modname, d->handle);
+	  fabort ("unable to release protective reservation (%p) for %W, %E",
+		  d->handle, d->ntname);
 
-	HMODULE h = LoadLibraryExW (d->name, NULL, DONT_RESOLVE_DLL_REFERENCES);
+	HMODULE h = LoadLibraryExW (buffered_shortname (d->ntname),
+				    NULL, DONT_RESOLVE_DLL_REFERENCES);
 	if (!h)
-	  fabort ("unable to create interim mapping for %W, %E", d->name);
+	  fabort ("unable to create interim mapping for %W, %E", d->ntname);
 	if (h != d->handle)
 	  {
 	    sigproc_printf ("%W loaded in wrong place: %p != %p",
-			    d->modname, h, d->handle);
+			    d->ntname, h, d->handle);
 	    FreeLibrary (h);
-	    PVOID reservation = reserve_at (d->modname, h,
+	    PVOID reservation = reserve_at (d->ntname, h,
 					    d->handle, d->image_size);
 	    if (!reservation)
 	      fabort ("unable to block off %p to prevent %W from loading there",
-		      h, d->modname);
+		      h, d->ntname);
 
 	    if (retries < DLL_RETRY_MAX)
 	      load_after_fork_impl (parent, d, retries+1);
 	    else
 	       fabort ("unable to remap %W to same address as parent (%p) - try running rebaseall",
-		       d->modname, d->handle);
+		       d->ntname, d->handle);
 
 	    /* once the above returns all the dlls are mapped; release
 	       the reservation and continue unwinding */
 	    sigproc_printf ("releasing blocked space at %p", reservation);
-	    release_at (d->modname, reservation);
+	    release_at (d->ntname, reservation);
 	    return;
 	  }
       }
@@ -586,7 +738,7 @@ void dll_list::load_after_fork_impl (HANDLE parent, dll* d, int retries)
 	{
 	  if (!VirtualFree (d->handle, 0, MEM_RELEASE))
 	    fabort ("unable to release protective reservation for %W (%p), %E",
-		    d->modname, d->handle);
+		    d->ntname, d->handle);
 	}
       else
 	{
@@ -594,14 +746,16 @@ void dll_list::load_after_fork_impl (HANDLE parent, dll* d, int retries)
 	     to ours or we wouldn't have gotten this far */
 	  if (!FreeLibrary (d->handle))
 	    fabort ("unable to unload interim mapping of %W, %E",
-		    d->modname);
+		    d->ntname);
 	}
-      HMODULE h = LoadLibraryW (d->name);
+      /* cygwin1.dll - as linked dependency - may reuse the shortname
+	 buffer, even in case of failure: don't reuse shortname later */
+      HMODULE h = LoadLibraryW (buffered_shortname (d->ntname));
       if (!h)
-	fabort ("unable to map %W, %E", d->name);
+	fabort ("unable to map %W, %E", d->ntname);
       if (h != d->handle)
 	fabort ("unable to map %W to same address as parent: %p != %p",
-		d->modname, d->handle, h);
+		d->ntname, d->handle, h);
     }
 }
 
diff --git a/winsup/cygwin/dll_init.h b/winsup/cygwin/dll_init.h
index 8127d0b..d4cbd3d 100644
--- a/winsup/cygwin/dll_init.h
+++ b/winsup/cygwin/dll_init.h
@@ -61,7 +61,11 @@ struct dll
   DWORD image_size;
   void* preferred_base;
   PWCHAR modname;
-  WCHAR name[1];
+
+  /* forkable */
+  HANDLE fhandle;
+
+  WCHAR ntname[1]; /* must be the last data member */
   void detach ();
   int init ();
   void run_dtors ()
@@ -78,15 +82,42 @@ struct dll
 
 class dll_list
 {
+  void set_forkables_inheritance (bool);
+
   dll *end;
   dll *hold;
   dll_type hold_type;
   static muto protect;
+  /* Use this buffer under loader lock conditions only. */
+  static WCHAR NO_COPY nt_max_path_buffer[NT_MAX_PATH];
 public:
+  /* forkables */
+  void request_forkables ();
+  void release_forkables ();
+
+  static HANDLE ntopenfile (PCWCHAR ntname, NTSTATUS *pstatus = NULL,
+			    ULONG openopts = FILE_NON_DIRECTORY_FILE);
+  static PWCHAR form_ntname (PWCHAR ntbuf, size_t bufsize, PCWCHAR name);
+  static PWCHAR form_shortname (PWCHAR shortbuf, size_t bufsize, PCWCHAR name);
+  static PWCHAR nt_max_path_buf ()
+  {
+    return nt_max_path_buffer;
+  }
+  static PCWCHAR buffered_ntname (PCWCHAR name)
+  {
+    form_ntname (nt_max_path_buffer, NT_MAX_PATH, name);
+    return nt_max_path_buffer;
+  }
+  static PCWCHAR buffered_shortname (PCWCHAR name)
+  {
+    form_shortname (nt_max_path_buffer, NT_MAX_PATH, name);
+    return nt_max_path_buffer;
+  }
+
   dll start;
   int loaded_dlls;
   int reload_on_fork;
-  dll *operator [] (const PWCHAR name);
+  dll *operator [] (PCWCHAR ntname);
   dll *alloc (HINSTANCE, per_process *, dll_type);
   dll *find (void *);
   void detach (void *);
@@ -94,7 +125,7 @@ public:
   void load_after_fork (HANDLE);
   void reserve_space ();
   void load_after_fork_impl (HANDLE, dll* which, int retries);
-  dll *find_by_modname (const PWCHAR name);
+  dll *find_by_modname (PCWCHAR modname);
   void populate_deps (dll* d);
   void topsort ();
   void topsort_visit (dll* d, bool goto_tail);
diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index e7b9ea4..4361f58 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -188,21 +188,18 @@ frok::child (volatile char * volatile here)
 
   MALLOC_CHECK;
 
-  /* If we haven't dynamically loaded any dlls, just signal
-     the parent.  Otherwise, load all the dlls, tell the parent
-      that we're done, and wait for the parent to fill in the.
-      loaded dlls' data/bss. */
+  /* load dynamic dlls, if any */
+  dlls.load_after_fork (hParent);
+
+  cygheap->fdtab.fixup_after_fork (hParent);
+
+  /* If we haven't dynamically loaded any dlls, just signal the parent.
+     Otherwise, tell the parent that we've loaded all the dlls
+     and wait for the parent to fill in the loaded dlls' data/bss. */
   if (!load_dlls)
-    {
-      cygheap->fdtab.fixup_after_fork (hParent);
-      sync_with_parent ("performed fork fixup", false);
-    }
+    sync_with_parent ("performed fork fixup", false);
   else
-    {
-      dlls.load_after_fork (hParent);
-      cygheap->fdtab.fixup_after_fork (hParent);
-      sync_with_parent ("loaded dlls", true);
-    }
+    sync_with_parent ("loaded dlls", true);
 
   init_console_handler (myself->ctty > 0);
   ForceCloseHandle1 (fork_info->forker_finished, forker_finished);
@@ -345,8 +342,6 @@ frok::parent (volatile char * volatile stack_here)
   si.lpReserved2 = (LPBYTE) &ch;
   si.cbReserved2 = sizeof (ch);
 
-  syscall_printf ("CreateProcessW (%W, %W, 0, 0, 1, %y, 0, 0, %p, %p)",
-		  myself->progname, myself->progname, c_flags, &si, &pi);
   bool locked = __malloc_lock ();
 
   /* Remove impersonation */
@@ -357,6 +352,11 @@ frok::parent (volatile char * volatile stack_here)
 
   while (1)
     {
+      dlls.request_forkables ();
+
+      debug_printf ("CreateProcessW (%W, %W, 0, 0, 1, %y, 0, 0, %p, %p)",
+		    myself->progname, myself->progname, c_flags, &si, &pi);
+
       hchild = NULL;
       rc = CreateProcessW (myself->progname,	/* image to run */
 			   GetCommandLineW (),	/* Take same space for command
@@ -379,6 +379,7 @@ frok::parent (volatile char * volatile stack_here)
 	{
 	  this_errno = geterrno_from_win_error ();
 	  error ("CreateProcessW failed for '%W'", myself->progname);
+	  dlls.release_forkables ();
 	  memset (&pi, 0, sizeof (pi));
 	  goto cleanup;
 	}
@@ -392,6 +393,8 @@ frok::parent (volatile char * volatile stack_here)
       CloseHandle (pi.hThread);
       hchild = pi.hProcess;
 
+      dlls.release_forkables ();
+
       /* Protect the handle but name it similarly to the way it will
 	 be called in subproc handling. */
       ProtectHandle1 (hchild, childhProc);
diff --git a/winsup/cygwin/forkable.cc b/winsup/cygwin/forkable.cc
new file mode 100644
index 0000000..5592985
--- /dev/null
+++ b/winsup/cygwin/forkable.cc
@@ -0,0 +1,55 @@
+/* forkable.cc
+
+   Copyright 2015 Red Hat, Inc.
+
+This software is a copyrighted work licensed under the terms of the
+Cygwin license.  Please consult the file "CYGWIN_LICENSE" for
+details. */
+
+#include "winsup.h"
+#include "cygerrno.h"
+#include "perprocess.h"
+#include "sync.h"
+#include "dll_init.h"
+#include "environ.h"
+#include "security.h"
+#include "path.h"
+#include "fhandler.h"
+#include "dtable.h"
+#include "cygheap.h"
+#include "pinfo.h"
+#include "shared_info.h"
+#include "child_info.h"
+#include "cygtls.h"
+#include "exception.h"
+#include <wchar.h>
+#include <sys/reent.h>
+#include <assert.h>
+#include <tls_pbuf.h>
+
+/* Set or clear HANDLE_FLAG_INHERIT for all handles necessary
+   to maintain forkables-hardlinks. */
+void
+dll_list::set_forkables_inheritance (bool inherit)
+{
+  DWORD mask = HANDLE_FLAG_INHERIT;
+  DWORD flags = inherit ? HANDLE_FLAG_INHERIT : 0;
+
+  dll *d = &start;
+  while ((d = d->next))
+    if (d->fhandle)
+      SetHandleInformation (d->fhandle, mask, flags);
+}
+
+/* create the forkable hardlinks, if necessary */
+void
+dll_list::request_forkables ()
+{
+  set_forkables_inheritance (true);
+}
+
+void
+dll_list::release_forkables ()
+{
+  set_forkables_inheritance (false);
+}
-- 
2.7.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 4/6] forkables: Protect fork against dll-, exe-updates.
  2016-03-30 18:54 [PATCH 0/6] Protect fork() against dll- and exe-updates Michael Haubenwallner
@ 2016-03-30 18:54 ` Michael Haubenwallner
  2016-03-30 19:04   ` Yaakov Selkowitz
  2016-03-30 18:54 ` [PATCH 1/6] forkables: Store dll file name as full NT path Michael Haubenwallner
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Michael Haubenwallner @ 2016-03-30 18:54 UTC (permalink / raw)
  To: cygwin-patches; +Cc: michael.haubenwallner

To support in-cygwin package managers, the fork() implementation must
not rely on .exe and .dll files to stay in their original location, as
the package manager's job is to replace these files.  Instead, we use
the hardlinks to the original binaries in /var/run/cygfork/ to create
the child process during fork, and let the main.exe.local file enable
the "DotLocal Dll Redirection" feature for dlls.

The (probably few) users that need an update-safe fork manually have to
create the /var/run/cygfork/ directory for now, using:
mkdir --mode=a=rwxt /var/run/cygfork

	* dll_init.h (struct dll_list): Declare find_by_forkedntname.
	* dll_init.cc (struct dll_list): Implement find_by_forkedntname.
	(dll_list::alloc): Use find_by_forkedntname when in load after fork.
	(dll_list::load_after_fork_impl): Use dll::forkedntname to load dlls.
	* fork.cc (frok::parent): Use forkedntname of dlls.main_executable.
---
 winsup/cygwin/dll_init.cc | 47 +++++++++++++++++++++++++++++++++--------------
 winsup/cygwin/dll_init.h  |  1 +
 winsup/cygwin/fork.cc     |  2 +-
 3 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/winsup/cygwin/dll_init.cc b/winsup/cygwin/dll_init.cc
index e44ee84..71a7456 100644
--- a/winsup/cygwin/dll_init.cc
+++ b/winsup/cygwin/dll_init.cc
@@ -325,6 +325,19 @@ dll_list::find_by_modname (PCWCHAR modname)
   return NULL;
 }
 
+/* Look for a dll based on the ntname used
+   to dynamically reload in forked child. */
+dll *
+dll_list::find_by_forkedntname (PCWCHAR ntname)
+{
+  dll *d = &start;
+  while ((d = d->next) != NULL)
+    if (!wcscasecmp (ntname, d->forkedntname ()))
+      return d;
+
+  return NULL;
+}
+
 #define RETRIES 1000
 
 /* Allocate space for a dll struct. */
@@ -344,8 +357,11 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type)
   /* Already loaded?  For linked DLLs, only compare the basenames.  Linked
      DLLs are loaded using just the basename and the default DLL search path.
      The Windows loader picks up the first one it finds.
-     This also applies to cygwin1.dll and the main-executable (DLL_SELF).  */
-  dll *d = (type != DLL_LOAD) ? dlls.find_by_modname (modname) : dlls[ntname];
+     This also applies to cygwin1.dll and the main-executable (DLL_SELF).
+     When in_load_after_fork, dynamically loaded dll's are reloaded
+     using their parent's forkable_ntname, if available.  */
+  dll *d = (type != DLL_LOAD) ? dlls.find_by_modname (modname) :
+	   in_load_after_fork ? dlls.find_by_forkedntname (ntname) : dlls[ntname];
   if (d)
     {
       if (!in_forkee)
@@ -728,14 +744,16 @@ void dll_list::load_after_fork_impl (HANDLE parent, dll* d, int retries)
 	  fabort ("unable to release protective reservation (%p) for %W, %E",
 		  d->handle, d->ntname);
 
-	HMODULE h = LoadLibraryExW (buffered_shortname (d->ntname),
+	HMODULE h = LoadLibraryExW (buffered_shortname (d->forkedntname ()),
 				    NULL, DONT_RESOLVE_DLL_REFERENCES);
 	if (!h)
-	  fabort ("unable to create interim mapping for %W, %E", d->ntname);
+	  fabort ("unable to create interim mapping for %W (using %W), %E",
+		  d->ntname, buffered_shortname (d->forkedntname ()));
 	if (h != d->handle)
 	  {
-	    sigproc_printf ("%W loaded in wrong place: %p != %p",
-			    d->ntname, h, d->handle);
+	    sigproc_printf ("%W (using %W) loaded in wrong place: %p != %p",
+			    d->ntname, buffered_shortname (d->forkedntname ()),
+			    h, d->handle);
 	    FreeLibrary (h);
 	    PVOID reservation = reserve_at (d->ntname, h,
 					    d->handle, d->image_size);
@@ -746,8 +764,8 @@ void dll_list::load_after_fork_impl (HANDLE parent, dll* d, int retries)
 	    if (retries < DLL_RETRY_MAX)
 	      load_after_fork_impl (parent, d, retries+1);
 	    else
-	       fabort ("unable to remap %W to same address as parent (%p) - try running rebaseall",
-		       d->ntname, d->handle);
+	       fabort ("unable to remap %W (using %W) to same address as parent (%p) - try running rebaseall",
+		       d->ntname, buffered_shortname (d->forkedntname ()), d->handle);
 
 	    /* once the above returns all the dlls are mapped; release
 	       the reservation and continue unwinding */
@@ -775,17 +793,18 @@ void dll_list::load_after_fork_impl (HANDLE parent, dll* d, int retries)
 	  /* Free the library using our parent's handle: it's identical
 	     to ours or we wouldn't have gotten this far */
 	  if (!FreeLibrary (d->handle))
-	    fabort ("unable to unload interim mapping of %W, %E",
-		    d->ntname);
+	    fabort ("unable to unload interim mapping of %W (using %W), %E",
+		    d->ntname, buffered_shortname (d->forkedntname ()));
 	}
       /* cygwin1.dll - as linked dependency - may reuse the shortname
 	 buffer, even in case of failure: don't reuse shortname later */
-      HMODULE h = LoadLibraryW (buffered_shortname (d->ntname));
+      HMODULE h = LoadLibraryW (buffered_shortname (d->forkedntname ()));
       if (!h)
-	fabort ("unable to map %W, %E", d->ntname);
+	fabort ("unable to map %W (using %W), %E",
+		d->ntname, buffered_shortname (d->forkedntname ()));
       if (h != d->handle)
-	fabort ("unable to map %W to same address as parent: %p != %p",
-		d->ntname, d->handle, h);
+	fabort ("unable to map %W (using %W) to same address as parent: %p != %p",
+		d->ntname, buffered_shortname (d->forkedntname ()), d->handle, h);
     }
 }
 
diff --git a/winsup/cygwin/dll_init.h b/winsup/cygwin/dll_init.h
index 554435d..d75cbb4 100644
--- a/winsup/cygwin/dll_init.h
+++ b/winsup/cygwin/dll_init.h
@@ -108,6 +108,7 @@ class dll_list
   PWCHAR forkables_mutex_name;
   HANDLE forkables_mutex;
   void track_self ();
+  dll *find_by_forkedntname (PCWCHAR ntname);
   size_t forkable_ntnamesize (dll_type, PCWCHAR fullntname, PCWCHAR modname);
   void prepare_forkables_nomination ();
   void update_forkables_needs ();
diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index 7bc3dec..52ceeb3 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -357,7 +357,7 @@ frok::parent (volatile char * volatile stack_here)
       PCWCHAR forking_progname = NULL;
       if (dlls.main_executable)
         forking_progname = dll_list::buffered_shortname
-			   (dlls.main_executable->ntname);
+			   (dlls.main_executable->forkedntname ());
       if (!forking_progname || !*forking_progname)
 	forking_progname = myself->progname;
 
-- 
2.7.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] Protect fork() against dll- and exe-updates.
  2016-03-30 18:54 [PATCH 0/6] Protect fork() against dll- and exe-updates Michael Haubenwallner
                   ` (3 preceding siblings ...)
  2016-03-30 18:54 ` [PATCH 2/6] forkables: Track main executable and cygwin1.dll Michael Haubenwallner
@ 2016-03-30 18:55 ` Daniel Colascione
  2016-03-30 19:04   ` Michael Haubenwallner
  2016-04-01 12:19   ` Corinna Vinschen
  2016-03-30 19:24 ` [PATCH 5/6] forkables: Keep hardlinks disabled via shared mem Michael Haubenwallner
  2016-03-30 19:24 ` [PATCH 6/6] forkables: Document hardlink creation at forktime Michael Haubenwallner
  6 siblings, 2 replies; 16+ messages in thread
From: Daniel Colascione @ 2016-03-30 18:55 UTC (permalink / raw)
  To: cygwin-patches


[-- Attachment #1.1: Type: text/plain, Size: 593 bytes --]



On 03/30/2016 11:53 AM, Michael Haubenwallner wrote:
> Hi,
> 
> this is the updated and split series of patches to use hardlinks
> for creating the child process by fork(), in reply to
> https://cygwin.com/ml/cygwin-developers/2016-01/msg00002.html
> https://cygwin.com/ml/cygwin-developers/2016-03/msg00005.html
> http://thread.gmane.org/gmane.os.cygwin.devel/1378
> 
> Thanks for review!
> /haubi/
> 
> 

Creating a new process now requires a write operation on the filesystem
hosting the binary? Seriously? I don't think that's worth it no matter
the other benefits.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/6] forkables: Protect fork against dll-, exe-updates.
  2016-03-30 18:54 ` [PATCH 4/6] forkables: Protect fork against dll-, exe-updates Michael Haubenwallner
@ 2016-03-30 19:04   ` Yaakov Selkowitz
  2016-03-30 19:12     ` Michael Haubenwallner
  0 siblings, 1 reply; 16+ messages in thread
From: Yaakov Selkowitz @ 2016-03-30 19:04 UTC (permalink / raw)
  To: cygwin-patches

On 2016-03-30 13:53, Michael Haubenwallner wrote:
> To support in-cygwin package managers, the fork() implementation must
> not rely on .exe and .dll files to stay in their original location, as
> the package manager's job is to replace these files.  Instead, we use
> the hardlinks to the original binaries in /var/run/cygfork/ to create
> the child process during fork, and let the main.exe.local file enable
> the "DotLocal Dll Redirection" feature for dlls.
>
> The (probably few) users that need an update-safe fork manually have to
> create the /var/run/cygfork/ directory for now, using:
> mkdir --mode=a=rwxt /var/run/cygfork

Have the security implications of this been considered?

-- 
Yaakov

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] Protect fork() against dll- and exe-updates.
  2016-03-30 18:55 ` [PATCH 0/6] Protect fork() against dll- and exe-updates Daniel Colascione
@ 2016-03-30 19:04   ` Michael Haubenwallner
  2016-04-01 12:19   ` Corinna Vinschen
  1 sibling, 0 replies; 16+ messages in thread
From: Michael Haubenwallner @ 2016-03-30 19:04 UTC (permalink / raw)
  To: cygwin-patches

On 03/30/2016 08:55 PM, Daniel Colascione wrote:
> 
> 
> On 03/30/2016 11:53 AM, Michael Haubenwallner wrote:
>> Hi,
>>
>> this is the updated and split series of patches to use hardlinks
>> for creating the child process by fork(), in reply to
>> https://cygwin.com/ml/cygwin-developers/2016-01/msg00002.html
>> https://cygwin.com/ml/cygwin-developers/2016-03/msg00005.html
>> http://thread.gmane.org/gmane.os.cygwin.devel/1378
>>
>> Thanks for review!
>> /haubi/
>>
>>
> 
> Creating a new process now requires a write operation on the filesystem
> hosting the binary? Seriously? I don't think that's worth it no matter
> the other benefits.
> 

Only if the original binaries necessary to create the new child process
by fork() are not available any more - and the /var/run/cygfork/
directory does exist and is on NTFS.  I do prefer a working fork() even
when updating dlls and executables while the parent process is running.

/haubi/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/6] forkables: Protect fork against dll-, exe-updates.
  2016-03-30 19:04   ` Yaakov Selkowitz
@ 2016-03-30 19:12     ` Michael Haubenwallner
  2016-04-01 12:20       ` Corinna Vinschen
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Haubenwallner @ 2016-03-30 19:12 UTC (permalink / raw)
  To: cygwin-patches

On 03/30/2016 09:04 PM, Yaakov Selkowitz wrote:
> On 2016-03-30 13:53, Michael Haubenwallner wrote:
>> To support in-cygwin package managers, the fork() implementation must
>> not rely on .exe and .dll files to stay in their original location, as
>> the package manager's job is to replace these files.  Instead, we use
>> the hardlinks to the original binaries in /var/run/cygfork/ to create
>> the child process during fork, and let the main.exe.local file enable
>> the "DotLocal Dll Redirection" feature for dlls.
>>
>> The (probably few) users that need an update-safe fork manually have to
>> create the /var/run/cygfork/ directory for now, using:
>> mkdir --mode=a=rwxt /var/run/cygfork
> 
> Have the security implications of this been considered?

Which security implications do you think of?

Removed but in-use binaries are available in the recycle bin anyway,
and can manually be hardlinked to wherever one likes...

/haubi/

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 5/6] forkables: Keep hardlinks disabled via shared mem.
  2016-03-30 18:54 [PATCH 0/6] Protect fork() against dll- and exe-updates Michael Haubenwallner
                   ` (4 preceding siblings ...)
  2016-03-30 18:55 ` [PATCH 0/6] Protect fork() against dll- and exe-updates Daniel Colascione
@ 2016-03-30 19:24 ` Michael Haubenwallner
  2016-03-30 19:24 ` [PATCH 6/6] forkables: Document hardlink creation at forktime Michael Haubenwallner
  6 siblings, 0 replies; 16+ messages in thread
From: Michael Haubenwallner @ 2016-03-30 19:24 UTC (permalink / raw)
  To: cygwin-patches; +Cc: michael.haubenwallner

To avoid the need for each process to check the filesystem to detect
that hardlink creation is impossible or disabled, cache this fact in
shared memory.  Removing cygfork directory while in use does disable
hardlinks creation.  To (re-)enable hardlinks creation, the cygfork
directory has to exist before the first cygwin process does fork.

	* forkable.cc (dll_list::forkable_ntnamesize): Short cut
	forkables needs to impossible when disabled via shared memory.
	(dll_list::update_forkables_needs): When detecting hardlink
	creation as impossible (not on NTFS) while still (we are the
	first one checking) enabled via shared memory, disable the
	shared memory value.
	* (dll_list::request_forkables): Disable the shared memory value
	when hardlinks creation is disabled, that is when the cygfork
	directory is (or became) absent.
---
 winsup/cygwin/forkable.cc              | 13 +++++++++++++
 winsup/cygwin/include/cygwin/version.h |  5 +++--
 winsup/cygwin/shared.cc                |  1 +
 winsup/cygwin/shared_info.h            |  3 ++-
 4 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/winsup/cygwin/forkable.cc b/winsup/cygwin/forkable.cc
index 0a8a528..83b5731 100644
--- a/winsup/cygwin/forkable.cc
+++ b/winsup/cygwin/forkable.cc
@@ -412,6 +412,11 @@ dll::create_forkable ()
 size_t
 dll_list::forkable_ntnamesize (dll_type type, PCWCHAR fullntname, PCWCHAR modname)
 {
+  /* per process, this is the first forkables-method ever called */
+  if (forkables_needs == forkables_unknown &&
+      !cygwin_shared->prefer_forkable_hardlinks)
+      forkables_needs = forkables_impossible; /* short cut */
+
   if (forkables_needs == forkables_impossible)
     return 0;
 
@@ -553,6 +558,7 @@ dll_list::update_forkables_needs ()
 	{
 	  debug_printf ("impossible, not on NTFS %W", fn.Buffer);
 	  forkables_needs = forkables_impossible;
+	  cygwin_shared->prefer_forkable_hardlinks = 0;
 	}
     }
 
@@ -1048,6 +1054,13 @@ dll_list::request_forkables ()
 
   set_forkables_inheritance (true);
 
+  if (forkables_needs == forkables_disabled)
+    {
+      /* we do not support (re-)enabling on the fly */
+      forkables_needs = forkables_impossible;
+      cygwin_shared->prefer_forkable_hardlinks = 0;
+    }
+
   if (forkables_needs <= forkables_needless)
     return;
 
diff --git a/winsup/cygwin/include/cygwin/version.h b/winsup/cygwin/include/cygwin/version.h
index 8b1a8fc..7ac5899 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -503,9 +503,10 @@ details. */
 	shared mutexes, semaphores, etc.   The arbitrary starting
 	version was 0 (cygwin release 98r2).
 	Bump to 4 since this hasn't been rigorously updated in a
-	while.  */
+	while.
+	6. Add prefer_forkable_hardlinks (using /var/run/cygfork/). */
 
-#define CYGWIN_VERSION_SHARED_DATA 5
+#define CYGWIN_VERSION_SHARED_DATA 6
 
      /* An identifier used in the names used to create shared objects.
 	The full names include the CYGWIN_VERSION_SHARED_DATA version
diff --git a/winsup/cygwin/shared.cc b/winsup/cygwin/shared.cc
index 202d8cc..2bc9c7a 100644
--- a/winsup/cygwin/shared.cc
+++ b/winsup/cygwin/shared.cc
@@ -331,6 +331,7 @@ shared_info::initialize ()
       init_obcaseinsensitive ();	/* Initialize obcaseinsensitive */
       tty.init ();			/* Initialize tty table  */
       mt.initialize ();			/* Initialize shared tape information */
+      prefer_forkable_hardlinks = 1;    /* Yes */
       /* Defer debug output printing the installation root and installation key
 	 up to this point.  Debug output except for system_printf requires
 	 the global shared memory to exist. */
diff --git a/winsup/cygwin/shared_info.h b/winsup/cygwin/shared_info.h
index 90b386f..dd4cb3d 100644
--- a/winsup/cygwin/shared_info.h
+++ b/winsup/cygwin/shared_info.h
@@ -35,7 +35,7 @@ public:
 /* Data accessible to all tasks */
 
 
-#define CURR_SHARED_MAGIC 0x8fe4d9eeU
+#define CURR_SHARED_MAGIC 0xbc5d6a9cU
 
 #define USER_VERSION   1
 
@@ -51,6 +51,7 @@ class shared_info
   LONG last_used_bindresvport;
   DWORD obcaseinsensitive;
   mtinfo mt;
+  char prefer_forkable_hardlinks; /* single byte access always is atomic */
 
   void initialize ();
   void init_obcaseinsensitive ();
-- 
2.7.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 6/6] forkables: Document hardlink creation at forktime.
  2016-03-30 18:54 [PATCH 0/6] Protect fork() against dll- and exe-updates Michael Haubenwallner
                   ` (5 preceding siblings ...)
  2016-03-30 19:24 ` [PATCH 5/6] forkables: Keep hardlinks disabled via shared mem Michael Haubenwallner
@ 2016-03-30 19:24 ` Michael Haubenwallner
  6 siblings, 0 replies; 16+ messages in thread
From: Michael Haubenwallner @ 2016-03-30 19:24 UTC (permalink / raw)
  To: cygwin-patches; +Cc: michael.haubenwallner

	* faq-api.xml: Mention hardlink creation by fork.
	* highlights.xml: Describe hardlink creation.
---
 winsup/doc/faq-api.xml    |  5 +++++
 winsup/doc/highlights.xml | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)

diff --git a/winsup/doc/faq-api.xml b/winsup/doc/faq-api.xml
index 993274a..4c1f138 100644
--- a/winsup/doc/faq-api.xml
+++ b/winsup/doc/faq-api.xml
@@ -155,6 +155,11 @@ child, releases the mutex the child is waiting on and returns from the
 fork call.  Child wakes from blocking on mutex, recreates any mmapped
 areas passed to it via shared area and then returns from fork itself.
 </para>
+<para>When the executable or any dll in use by the parent was renamed
+or moved into the hidden recycle bin, fork tries to create hardlinks
+for the old executable and any dll into per-user subdirectories in the
+/var/run/cygfork/ directory, when that one exists and resides on NTFS.
+</para>
 </answer></qandaentry>
 
 <qandaentry id="faq.api.globbing">
diff --git a/winsup/doc/highlights.xml b/winsup/doc/highlights.xml
index 65407ab..c000f24 100644
--- a/winsup/doc/highlights.xml
+++ b/winsup/doc/highlights.xml
@@ -195,6 +195,47 @@ difficult to implement correctly.  Currently, the Cygwin fork is a
 non-copy-on-write implementation similar to what was present in early
 flavors of UNIX.</para>
 
+<para>As the child process is created as new process, both the main
+executable and all the dlls loaded either statically or dynamically have
+to be identical as to when the parent process has started or loaded a dll.
+While Windows does not allow to remove binaries in use from the file
+system, they still can be renamed or moved into the recycle bin, as
+outlined for unlink(2) in <xref linkend="ov-new1.7-file"></xref>.
+To allow an existing process to fork, the original binary files need to be
+available via their original file names, but they may reside in
+different directories when using the <ulink
+url="https://social.msdn.microsoft.com/search/en-US?query=dotlocal%20dll%20redirection"
+>DotLocal (.local) Dll Redirection</ulink> feature.
+Since NTFS does support hardlinks, we create a private directory
+containing hardlinks to the original files as well as the .local file.
+The private directory for the hardlinks is /var/run/cygfork/, which you
+have to create manually for now if you need to protect fork against exe-
+and dll- updates on your Cygwin instance.  As hardlinks cannot be used
+across multiple NTFS file systems, please make sure your exe- and dll-
+replacing operations operate on the same single NTFS file system as your
+Cygwin instance and the /var/run/cygfork/ directory.</para>
+
+<para>We create one directory per user, application and application age,
+and remove it when no more processes use that directory.  To indicate
+whether a directory still is in use, we define a mutex name similar to
+the directory name.  As mutexes are destroyed when no process holds a
+handle open any more, we can clean up even after power loss or similar:
+Both the parent and child process, at exit they lock the mutex with
+almost no timeout, and close it.
+If the lock succeeded before closing, directory cleanup is started:
+For each directory found, the corresponding mutex is created with lock.
+If that succeeds, the directory is removed, as it is unused now, and the
+corresponding mutex handle is closed.</para>
+
+<para>Before fork, when about to create hardlinks for the first time, the
+mutex is opened and locked with infinite timeout, to wait for the cleanup
+that may run at the same time.  Once locked, the mutex is unlocked
+immediately, but the mutex handle stays open until exit, and the hardlinks
+are created.  It is fine for multiple processes to concurrently create
+the same hardlinks, as the result really should be identical.  Once the
+mutex is open, we can create more hardlinks within this one directory
+without the need to lock the mutex again.</para>
+
 <para>The first thing that happens when a parent process
 forks a child process is that the parent initializes a space in the
 Cygwin process table for the child.  It then creates a suspended
-- 
2.7.3

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/6] Protect fork() against dll- and exe-updates.
  2016-03-30 18:55 ` [PATCH 0/6] Protect fork() against dll- and exe-updates Daniel Colascione
  2016-03-30 19:04   ` Michael Haubenwallner
@ 2016-04-01 12:19   ` Corinna Vinschen
  1 sibling, 0 replies; 16+ messages in thread
From: Corinna Vinschen @ 2016-04-01 12:19 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 1127 bytes --]

On Mar 30 11:55, Daniel Colascione wrote:
> 
> 
> On 03/30/2016 11:53 AM, Michael Haubenwallner wrote:
> > Hi,
> > 
> > this is the updated and split series of patches to use hardlinks
> > for creating the child process by fork(), in reply to
> > https://cygwin.com/ml/cygwin-developers/2016-01/msg00002.html
> > https://cygwin.com/ml/cygwin-developers/2016-03/msg00005.html
> > http://thread.gmane.org/gmane.os.cygwin.devel/1378
> > 
> > Thanks for review!
> > /haubi/
> > 
> > 
> 
> Creating a new process now requires a write operation on the filesystem
> hosting the binary? Seriously? I don't think that's worth it no matter
> the other benefits.

I'm with you on that, but as long as the entire functionality is
*optional* and does *not* affect environments not using it, it should
be ok.

What I'm concerned about is the size of the patches required to make
this stuff work.  I have to review this carefully but I need some time.


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 4/6] forkables: Protect fork against dll-, exe-updates.
  2016-03-30 19:12     ` Michael Haubenwallner
@ 2016-04-01 12:20       ` Corinna Vinschen
  0 siblings, 0 replies; 16+ messages in thread
From: Corinna Vinschen @ 2016-04-01 12:20 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 1265 bytes --]

On Mar 30 21:12, Michael Haubenwallner wrote:
> On 03/30/2016 09:04 PM, Yaakov Selkowitz wrote:
> > On 2016-03-30 13:53, Michael Haubenwallner wrote:
> >> To support in-cygwin package managers, the fork() implementation must
> >> not rely on .exe and .dll files to stay in their original location, as
> >> the package manager's job is to replace these files.  Instead, we use
> >> the hardlinks to the original binaries in /var/run/cygfork/ to create
> >> the child process during fork, and let the main.exe.local file enable
> >> the "DotLocal Dll Redirection" feature for dlls.
> >>
> >> The (probably few) users that need an update-safe fork manually have to
> >> create the /var/run/cygfork/ directory for now, using:
> >> mkdir --mode=a=rwxt /var/run/cygfork
> > 
> > Have the security implications of this been considered?
> 
> Which security implications do you think of?
> 
> Removed but in-use binaries are available in the recycle bin anyway,
> and can manually be hardlinked to wherever one likes...

Permissions on the parent dirs and the files are always an issue...


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/6] forkables: Create forkable hardlinks, yet unused.
  2016-03-30 18:54 ` [PATCH 3/6] forkables: Create forkable hardlinks, yet unused Michael Haubenwallner
@ 2016-11-16 12:25   ` Michael Haubenwallner
  2016-11-16 12:34   ` Michael Haubenwallner
  1 sibling, 0 replies; 16+ messages in thread
From: Michael Haubenwallner @ 2016-11-16 12:25 UTC (permalink / raw)
  To: cygwin-patches

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(-)
> 
> diff --git a/winsup/cygwin/dll_init.cc b/winsup/cygwin/dll_init.cc
> index fd807c9..e44ee84 100644
> --- a/winsup/cygwin/dll_init.cc
> +++ b/winsup/cygwin/dll_init.cc
> @@ -372,8 +372,11 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type)
>      }
>    else
>      {
> +      size_t forkntsize = forkable_ntnamesize (type, ntname, modname);
> +
>        /* FIXME: Change this to new at some point. */
> -      d = (dll *) cmalloc (HEAP_2_DLL, sizeof (*d) + (ntnamelen * sizeof (*ntname)));
> +      d = (dll *) cmalloc (HEAP_2_DLL, sizeof (*d)
> +			   + ((ntnamelen + forkntsize) * sizeof (*ntname)));
>  
>        /* Now we've allocated a block of information.  Fill it in with the
>  	 supplied info about this DLL. */
> @@ -389,11 +392,24 @@ dll_list::alloc (HINSTANCE h, per_process *p, dll_type type)
>        d->image_size = ((pefile*)h)->optional_hdr ()->SizeOfImage;
>        d->preferred_base = (void*) ((pefile*)h)->optional_hdr()->ImageBase;
>        d->type = type;
> -      NTSTATUS status;
> -      d->fhandle = ntopenfile (d->ntname, &status);
> -      if (!d->fhandle)
> -	system_printf ("Unable (ntstatus %y) to open file %W",
> -		       status, d->ntname);
> +      d->fhandle = NULL;
> +      d->fbi.FileAttributes = INVALID_FILE_ATTRIBUTES;
> +      d->fii.IndexNumber.QuadPart = 0;
> +      d->forkable_ntname = NULL;
> +      if (forkntsize)
> +	{
> +	  NTSTATUS status;
> +	  d->fhandle = ntopenfile (d->ntname, &status);
> +	  if (!d->fhandle)
> +	    system_printf ("Unable (ntstatus %y) to open file %W",
> +			   status, d->ntname);
> +	  else
> +	    {
> +	      /* may create a hardlink */
> +	      d->forkable_ntname = d->ntname + ntnamelen + 1;
> +	      *d->forkable_ntname = L'\0';
> +	    }
> +	}
>        append (d);
>        if (type == DLL_LOAD)
>  	loaded_dlls++;
> diff --git a/winsup/cygwin/dll_init.h b/winsup/cygwin/dll_init.h
> index c50f889..554435d 100644
> --- a/winsup/cygwin/dll_init.h
> +++ b/winsup/cygwin/dll_init.h
> @@ -65,6 +65,15 @@ struct dll
>  
>    /* forkable */
>    HANDLE fhandle;
> +  FILE_BASIC_INFORMATION fbi;
> +  FILE_INTERNAL_INFORMATION fii;
> +  PWCHAR forkable_ntname;
> +  void nominate_forkable (PCWCHAR);
> +  bool create_forkable ();
> +  PWCHAR forkedntname ()
> +  {
> +    return forkable_ntname && *forkable_ntname ? forkable_ntname : ntname;
> +  }
>  
>    WCHAR ntname[1]; /* must be the last data member */
>    void detach ();
> @@ -83,7 +92,30 @@ struct dll
>  
>  class dll_list
>  {
> +  /* forkables */
> +  enum
> +    {
> +      forkables_unknown,
> +      forkables_impossible,
> +      forkables_disabled,
> +      forkables_needless,
> +      forkables_needed,
> +      forkables_created,
> +    }
> +    forkables_needs;
> +  DWORD forkables_dirx_size;
> +  PWCHAR forkables_dirx_ntname;
> +  PWCHAR forkables_mutex_name;
> +  HANDLE forkables_mutex;
>    void track_self ();
> +  size_t forkable_ntnamesize (dll_type, PCWCHAR fullntname, PCWCHAR modname);
> +  void prepare_forkables_nomination ();
> +  void update_forkables_needs ();
> +  bool update_forkables ();
> +  bool create_forkables ();
> +  void denominate_forkables ();
> +  bool close_mutex ();
> +  void try_remove_forkables (PWCHAR dirbuf, size_t dirlen, size_t dirbufsize);
>    void set_forkables_inheritance (bool);
>  
>    dll *end;
> @@ -97,6 +129,7 @@ public:
>    dll *main_executable;
>    void request_forkables ();
>    void release_forkables ();
> +  void cleanup_forkables ();
>  
>    static HANDLE ntopenfile (PCWCHAR ntname, NTSTATUS *pstatus = NULL,
>  			    ULONG openopts = FILE_NON_DIRECTORY_FILE);
> diff --git a/winsup/cygwin/forkable.cc b/winsup/cygwin/forkable.cc
> index 5592985..0a8a528 100644
> --- a/winsup/cygwin/forkable.cc
> +++ b/winsup/cygwin/forkable.cc
> @@ -27,6 +27,998 @@ details. */
>  #include <assert.h>
>  #include <tls_pbuf.h>
>  
> +/* Allow concurrent processes to use the same dll or exe
> + * via their hardlink while we delete our hardlink. */
> +extern NTSTATUS unlink_nt_shareable (path_conv &pc);
> +
> +#define MUTEXSEP L"@"
> +#define PATHSEP L"\\"
> +
> +/* Create the lastsepcount directories found in ntdirname, where
> +   counting is done along path separators (including trailing ones).
> +   Returns true when these directories exist afterwards, false otherways.
> +   The ntdirname is used for the path-splitting. */
> +static bool
> +mkdirs (PWCHAR ntdirname, int lastsepcount)
> +{
> +  bool success = true;
> +  int i = lastsepcount;
> +  for (--i; i > 0; --i)
> +    {
> +      PWCHAR lastsep = wcsrchr (ntdirname, L'\\');
> +      if (!lastsep)
> +	break;
> +      *lastsep = L'\0';
> +    }
> +
> +  for (++i; i <= lastsepcount; ++i)
> +    {
> +      if (success && (i == 0 || wcslen (wcsrchr (ntdirname, L'\\')) > 1))
> +	{
> +	  UNICODE_STRING dn;
> +	  RtlInitUnicodeString (&dn, ntdirname);
> +	  OBJECT_ATTRIBUTES oa;
> +	  InitializeObjectAttributes (&oa, &dn, 0, NULL,
> +				      sec_none_nih.lpSecurityDescriptor);
> +	  HANDLE dh = NULL;
> +	  NTSTATUS status;
> +	  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_DIRECTORY_FILE
> +				 | FILE_SYNCHRONOUS_IO_NONALERT,
> +				 NULL, 0);
> +	  if (NT_SUCCESS(status))
> +	    NtClose (dh);
> +	  else
> +	    success = false;
> +	  debug_printf ("%y = NtCreateFile (%p, dir %W)", status, dh, ntdirname);
> +	}
> +      if (i < lastsepcount)
> +	ntdirname[wcslen (ntdirname)] = L'\\'; /* restore original value */
> +    }
> +  return success;
> +}
> +
> +/* Recursively remove the directory specified in ntmaxpathbuf,
> +   using ntmaxpathbuf as the buffer to form subsequent filenames. */
> +static void
> +rmdirs (WCHAR ntmaxpathbuf[NT_MAX_PATH])
> +{
> +  PWCHAR basebuf = wcsrchr (ntmaxpathbuf, L'\\'); /* find last pathsep */
> +  if (basebuf && *(basebuf+1))
> +    basebuf += wcslen (basebuf); /* last pathsep is not trailing one */
> +  if (!basebuf)
> +    basebuf = ntmaxpathbuf + wcslen (ntmaxpathbuf);
> +  *basebuf = L'\0'; /* kill trailing pathsep, if any */
> +
> +  NTSTATUS status;
> +  HANDLE hdir = dll_list::ntopenfile (ntmaxpathbuf, &status,
> +				      FILE_DIRECTORY_FILE |
> +				      FILE_DELETE_ON_CLOSE);
> +  if (!hdir)
> +    return;
> +
> +  *basebuf++ = L'\\'; /* (re-)add trailing pathsep */
> +
> +  struct {
> +    FILE_DIRECTORY_INFORMATION fdi;
> +    WCHAR buf[NAME_MAX];
> +  } fdibuf;
> +  IO_STATUS_BLOCK iosb;
> +
> +  while (NT_SUCCESS (status = NtQueryDirectoryFile (hdir, NULL, NULL, NULL,
> +						    &iosb,
> +						    &fdibuf, sizeof (fdibuf),
> +						    FileDirectoryInformation,
> +						    FALSE, NULL, FALSE)))
> +    {
> +      PFILE_DIRECTORY_INFORMATION pfdi = &fdibuf.fdi;
> +      while (true)
> +	{
> +	  int namelen = pfdi->FileNameLength / sizeof (WCHAR);
> +	  wcsncpy (basebuf, pfdi->FileName, namelen);
> +	  basebuf[namelen] = L'\0';
> +
> +	  if (pfdi->FileAttributes & FILE_ATTRIBUTE_DIRECTORY)
> +	    {
> +	      if (wcscmp (basebuf, L".") && wcscmp (basebuf, L".."))
> +		rmdirs (ntmaxpathbuf);
> +	    }
> +	  else
> +	    {
> +	      UNICODE_STRING fn;
> +	      RtlInitUnicodeString (&fn, ntmaxpathbuf);
> +
> +	      path_conv pc (&fn);
> +	      unlink_nt_shareable (pc); /* move to bin */
> +	    }
> +
> +	  if (!pfdi->NextEntryOffset)
> +	    break;
> +	  pfdi = (PFILE_DIRECTORY_INFORMATION)((caddr_t)pfdi
> +					       + pfdi->NextEntryOffset);
> +	}
> +    }
> +  if (status != STATUS_NO_MORE_FILES)
> +    debug_printf ("%y = NtQueryDirectoryFile (%p, io %y, info %d)",
> +		  status, hdir, iosb.Status, iosb.Information);
> +
> +  CloseHandle (hdir);
> +}
> +
> +static bool
> +read_fii (HANDLE fh, PFILE_INTERNAL_INFORMATION pfii, bool once = false)
> +{
> +  if (once && pfii->IndexNumber.QuadPart != 0)
> +    return true;
> +
> +  pfii->IndexNumber.QuadPart = 0LL;
> +
> +  NTSTATUS status;
> +  IO_STATUS_BLOCK iosb;
> +  status = NtQueryInformationFile (fh, &iosb,
> +				   pfii, sizeof (*pfii),
> +				   FileInternalInformation);
> +  if (!NT_SUCCESS (status))
> +    {
> +      system_printf ("WARNING: %y = NtQueryInformationFile (%p,"
> +		     " InternalInfo, io.Status %y)",
> +		     status, fh, iosb.Status);
> +      pfii->IndexNumber.QuadPart = -1LL;
> +      return false;
> +    }
> +  return true;
> +}
> +
> +static bool
> +read_fbi (HANDLE fh, PFILE_BASIC_INFORMATION pfbi, bool once = false)
> +{
> +  if (once && pfbi->FileAttributes != INVALID_FILE_ATTRIBUTES)
> +    return true;
> +
> +  pfbi->FileAttributes = INVALID_FILE_ATTRIBUTES;
> +  pfbi->LastWriteTime.QuadPart = 0LL;
> +
> +  NTSTATUS status;
> +  IO_STATUS_BLOCK iosb;
> +  status = NtQueryInformationFile (fh, &iosb,
> +				   pfbi, sizeof (*pfbi),
> +				   FileBasicInformation);
> +  if (!NT_SUCCESS (status))
> +    {
> +      system_printf ("WARNING: %y = NtQueryInformationFile (%p,"
> +		     " BasicInfo, io.Status %y)",
> +		     status, fh, iosb.Status);
> +      pfbi->FileAttributes = 0;
> +      pfbi->LastWriteTime.QuadPart = -1LL;
> +      return false;
> +    }
> +  return true;
> +}
> +
> +/* Into buf if not NULL, write the IndexNumber in pli.
> +   Return the number of characters (that would be) written. */
> +static int
> +format_IndexNumber (PWCHAR buf, ssize_t bufsize, LARGE_INTEGER const *pli)
> +{
> +  if (!buf)
> +    return 16;
> +  if (bufsize >= 0 && bufsize <= 16)
> +    return 0;
> +  return __small_swprintf (buf, L"%016X", pli->QuadPart);
> +}
> +
> +/* Into buf if not NULL, write the ntname of cygwin installation_root.
> +   Return the number of characters (that would be) written. */
> +static int
> +rootname (PWCHAR buf, ssize_t bufsize)
> +{
> +  PWCHAR cygroot = cygheap->installation_root;
> +  if (!buf)
> +    return 6 /* "\??\UN" */ + wcslen (cygroot);
> +  return dll_list::form_ntname (buf, bufsize, cygroot) - buf;
> +}
> +
> +/* Into buf if not NULL, write the string representation of current user sid.
> +   Return the number of characters (that would be) written. */
> +static int
> +sidname (PWCHAR buf, ssize_t bufsize)
> +{
> +  if (!buf)
> +    return 128;
> +  if (bufsize >= 0 && bufsize <= 128)
> +    return 0;
> +  UNICODE_STRING sid;
> +  WCHAR sidbuf[128+1];
> +  RtlInitEmptyUnicodeString (&sid, sidbuf, sizeof (sidbuf));
> +  RtlConvertSidToUnicodeString (&sid, cygheap->user.sid (), FALSE);
> +  return wcpcpy (buf, sid.Buffer) - buf;
> +}
> +
> +/* Into buf if not NULL, write the IndexNumber of the main executable.
> +   Return the number of characters (that would be) written. */
> +static int
> +exename (PWCHAR buf, ssize_t bufsize)
> +{
> +  if (!buf)
> +    return format_IndexNumber (NULL, bufsize, NULL);
> +  dll *d = dlls.main_executable;
> +  if (d->fhandle)
> +    (void) read_fii (d->fhandle, &d->fii, true);
> +  return format_IndexNumber (buf, bufsize, &d->fii.IndexNumber);
> +}
> +
> +/* Into buf if not NULL, write the newest dll's LastWriteTime.
> +   Return the number of characters (that would be) written. */
> +static int
> +lwtimename (PWCHAR buf, ssize_t bufsize)
> +{
> +  if (!buf)
> +    return sizeof (LARGE_INTEGER) * 2;
> +  if (bufsize >= 0 && bufsize <= (int)sizeof (LARGE_INTEGER) * 2)
> +    return 0;
> +
> +  LARGE_INTEGER newest = { 0 };
> +  /* Need by-handle-file-information for _all_ loaded dlls,
> +     as most recent ctime forms the hardlinks directory. */
> +  dll *d = &dlls.start;
> +  while ((d = d->next))
> +    {
> +      if (!d->fhandle)
> +	continue;
> +
> +      (void)read_fbi (d->fhandle, &d->fbi, true);
> +
> +      /* LastWriteTime more properly tells the last file-content modification
> +	 time, because a newly created hardlink may have a different
> +	 CreationTime compared to the original file. */
> +      if (d->fbi.LastWriteTime.QuadPart > newest.QuadPart)
> +	newest = d->fbi.LastWriteTime;
> +    }
> +
> +  return __small_swprintf (buf, L"%016X", newest);
> +}
> +
> +struct namepart {
> +  PCWCHAR text; /* used when no pathfunc, description otherwise */
> +  int (*textfunc)(PWCHAR buf, ssize_t bufsize);
> +  bool mutex_from_dir; /* on path-separators add mutex-separator */
> +  bool create_dir;
> +};
> +/* mutex name is formed along dir names */
> +static namepart NO_COPY_RO const
> +forkable_nameparts[] = {
> + /* text             textfunc  mutex_from_dir  create */
> +  { L"<cygroot>",    rootname,          false, false, },
> +  { L"\\var\\run\\",     NULL,          false, false, },
> +  { L"cygfork",          NULL,          true,  false, },
> +  { L"<sid>",         sidname,          true,  true,  },
> +  { L"<exe>",         exename,          false, false, },
> +  { MUTEXSEP,            NULL,          false, false, },
> +  { L"<ctime>",    lwtimename,          true,  true,  },
> +
> +  { NULL, NULL },
> +};
> +
> +/* Nominate the hardlink to an individual DLL inside dirx_name,
> +   that ends with the path separator (hence the "x" varname).
> +   With NULL as dirx_name, never nominate the hardlink any more.
> +   With "" as dirx_name, denominate the hardlink. */
> +void
> +dll::nominate_forkable (PCWCHAR dirx_name)
> +{
> +  if (!dirx_name)
> +    {
> +      debug_printf ("type %d disable %W", type, ntname);
> +      forkable_ntname = NULL; /* never create a hardlink for this dll */
> +    }
> +
> +  if (!forkable_ntname)
> +    return;
> +
> +  PWCHAR next = wcpcpy (forkable_ntname, dirx_name);
> +
> +  if (!*forkable_ntname)
> +    return; /* denominate */
> +
> +  if (type < DLL_LOAD)
> +    wcpcpy (next, modname);
> +  else
> +    {
> +      /* Avoid lots of extra directories for loaded dll's:
> +       * mangle full path into one single directory name,
> +       * just keep original filename intact. The original
> +       * filename is necessary to serve as linked
> +       * dependencies of dynamically loaded dlls. */
> +      PWCHAR lastpathsep = wcsrchr (ntname, L'\\');
> +      if (!lastpathsep)
> +        {
> +	  forkable_ntname = NULL;
> +	  return;
> +	}
> +      *lastpathsep = L'\0';
> +      HANDLE fh = dll_list::ntopenfile (ntname, NULL, FILE_DIRECTORY_FILE);
> +      *lastpathsep = L'\\';
> +
> +      FILE_INTERNAL_INFORMATION fii = { 0 };
> +      if (fh)
> +	{
> +	  read_fii (fh, &fii);
> +	  NtClose (fh);
> +	}
> +      next += format_IndexNumber (next, -1, &fii.IndexNumber);
> +      wcpcpy (next, lastpathsep);
> +    }
> +}
> +
> +/* Create the nominated hardlink for one indivitual dll,
> +   inside another subdirectory when dynamically loaded. */
> +bool
> +dll::create_forkable ()
> +{
> +  if (!forkable_ntname || !*forkable_ntname)
> +    return true; /* disabled */
> +
> +  if (!fhandle)
> +    return false; /* impossible */
> +
> +  PWCHAR ntname = forkable_ntname;
> +
> +  PWCHAR last = NULL;
> +  bool success = true;
> +  if (type >= DLL_LOAD)
> +    {
> +      last = wcsrchr (ntname, L'\\');
> +      if (!last)
> +	return false;
> +      *last = L'\0';
> +      success = mkdirs (ntname, 1);
> +      *last = L'\\';
> +      if (!success)
> +	return false;
> +    }
> +
> +  int ntlen = wcslen (ntname);
> +  int bufsize = sizeof (FILE_LINK_INFORMATION) + ntlen * sizeof (*ntname);
> +  PFILE_LINK_INFORMATION pfli = (PFILE_LINK_INFORMATION) alloca (bufsize);
> +
> +  wcscpy (pfli->FileName, ntname);
> +
> +  pfli->FileNameLength = ntlen * sizeof (*ntname);
> +  pfli->ReplaceIfExists = FALSE; /* allow concurrency */
> +  pfli->RootDirectory = NULL;
> +
> +  IO_STATUS_BLOCK iosb;
> +  NTSTATUS status = NtSetInformationFile (fhandle, &iosb, pfli, bufsize,
> +					  FileLinkInformation);
> +  debug_printf ("%y = NtSetInformationFile (%p, FileLink %W, iosb.Status %y)",
> +		status, fhandle, pfli->FileName, iosb.Status);
> +  if (NT_SUCCESS (status) || status == STATUS_OBJECT_NAME_COLLISION)
> +    /* We've not found a performant way yet to protect fork against updates
> +       to main executables and/or dlls that do not reside on the same NTFS
> +       filesystem as the <cygroot>/var/run/cygfork/ directory.
> +       But as long as the main executable can be hardlinked, dll redirection
> +       works for any other hardlink-able dll, while non-hardlink-able dlls
> +       are used from their original location. */
> +    return true;
> +
> +  return false;
> +}
> +
> +/* return the number of characters necessary to store one forkable name */
> +size_t
> +dll_list::forkable_ntnamesize (dll_type type, PCWCHAR fullntname, PCWCHAR modname)
> +{
> +  if (forkables_needs == forkables_impossible)
> +    return 0;
> +
> +  if (!forkables_dirx_size)
> +    {
> +      DWORD forkables_mutex_size = 0;
> +      bool needsep = false;
> +      for (namepart const *part = forkable_nameparts; part->text; ++part)
> +	{
> +	  if (needsep)
> +	    {
> +	      forkables_dirx_size += wcslen (PATHSEP);
> +	      forkables_mutex_size += wcslen (MUTEXSEP);
> +	    }
> +	  needsep = part->mutex_from_dir;
> +	  int len = 0;
> +	  if (part->textfunc)
> +	    len = part->textfunc (NULL, 0);
> +	  else
> +	    len = wcslen (part->text);
> +	  forkables_dirx_size += len;
> +	  forkables_mutex_size += len;
> +	}
> +      /* trailing path sep */
> +      forkables_dirx_size += wcslen (PATHSEP);
> +      /* trailing zeros */
> +      ++forkables_dirx_size;
> +      ++forkables_mutex_size;
> +
> +      /* allocate here, to avoid cygheap size changes during fork */
> +      forkables_dirx_ntname = (PWCHAR) cmalloc (HEAP_2_DLL,
> +	  (forkables_dirx_size + forkables_mutex_size) *
> +	    sizeof (*forkables_dirx_ntname));
> +      *forkables_dirx_ntname = L'\0';
> +
> +      forkables_mutex_name = forkables_dirx_ntname + forkables_dirx_size;
> +      *forkables_mutex_name = L'\0';
> +    }
> +
> +  size_t ret = forkables_dirx_size;
> +  if (type >= DLL_LOAD)
> +    ret += format_IndexNumber (NULL, -1, NULL) + 1; /* one more directory */
> +  return ret + wcslen (modname);
> +}
> +
> +/* Prepare top-level names necessary to nominate individual DLL hardlinks,
> +   eventually releasing/removing previous forkable hardlinks. */
> +void
> +dll_list::prepare_forkables_nomination ()
> +{
> +  if (!forkables_dirx_ntname)
> +    return;
> +
> +  PWCHAR pbuf = nt_max_path_buf ();
> +
> +  bool needsep = false;
> +  bool domutex = false;
> +  namepart const *part;
> +  for (part = forkable_nameparts; part->text; ++part)
> +    {
> +      if (part->mutex_from_dir)
> +	domutex = true; /* mutex naming starts with first mutex_from_dir */
> +      if (!domutex)
> +	continue;
> +      if (needsep)
> +	pbuf += __small_swprintf (pbuf, L"%W", MUTEXSEP);
> +      needsep = part->mutex_from_dir;
> +      if (part->textfunc)
> +	pbuf += part->textfunc (pbuf, -1);
> +      else
> +	pbuf += __small_swprintf (pbuf, L"%W", part->text);
> +    }
> +
> +  if (!wcscmp (forkables_mutex_name, nt_max_path_buf ()))
> +    return; /* nothing changed */
> +
> +  if (*forkables_mutex_name &&
> +      wcscmp (forkables_mutex_name, nt_max_path_buf ()))
> +    {
> +      /* The mutex name has changed since last fork and we either have
> +	 dlopen'ed a more recent or dlclose'd the most recent dll,
> +	 so we will not use the current forkable hardlinks any more.
> +	 Removing from the file system is done later, upon exit. */
> +      close_mutex ();
> +      denominate_forkables ();
> +    }
> +  wcscpy (forkables_mutex_name, nt_max_path_buf ());
> +
> +  pbuf = forkables_dirx_ntname;
> +  needsep = false;
> +  for (namepart const *part = forkable_nameparts; part->text; ++part)
> +    {
> +      if (needsep)
> +	pbuf += __small_swprintf (pbuf, L"%W", PATHSEP);
> +      needsep = part->mutex_from_dir;
> +      if (part->textfunc)
> +	pbuf += part->textfunc (pbuf, -1);
> +      else
> +	pbuf += __small_swprintf (pbuf, L"%W", part->text);
> +    }
> +  pbuf += __small_swprintf (pbuf, L"%W", PATHSEP);
> +
> +  debug_printf ("forkables dir %W", forkables_dirx_ntname);
> +  debug_printf ("forkables mutex %W", forkables_mutex_name);
> +}
> +
> +/* Test if creating hardlinks is necessary. If creating hardlinks is possible
> +   in general, each individual dll is tested if its previously created
> +   hardlink (if any, or the original file) still is the same.
> +   Testing is protected against hardlink removal by concurrent processes. */
> +void
> +dll_list::update_forkables_needs ()
> +{
> +  dll *d;
> +
> +  if (forkables_needs == forkables_unknown)
> +    {
> +      /* check if filesystem of forkables dir is NTFS */
> +      PWCHAR pbuf = nt_max_path_buf ();
> +      for (namepart const *part = forkable_nameparts; part->text; ++part)
> +	{
> +	  if (part->mutex_from_dir)
> +	    break; /* leading non-mutex-naming dirs, must exist anyway */
> +	  if (part->textfunc)
> +	    pbuf += part->textfunc (pbuf, -1);
> +	  else
> +	    pbuf += __small_swprintf (pbuf, L"%W", part->text);
> +	}
> +
> +      UNICODE_STRING fn;
> +      RtlInitUnicodeString (&fn, nt_max_path_buf ());
> +
> +      fs_info fsi;
> +      if (fsi.update (&fn, NULL) &&
> +/* FIXME: !fsi.is_readonly () && */
> +	  fsi.is_ntfs ())
> +	forkables_needs = forkables_disabled; /* check directory itself */
> +      else
> +	{
> +	  debug_printf ("impossible, not on NTFS %W", fn.Buffer);
> +	  forkables_needs = forkables_impossible;
> +	}
> +    }
> +
> +  if (forkables_needs == forkables_impossible)
> +    return; /* we have not created any hardlink, nothing to clean up */
> +
> +  if (forkables_needs == forkables_disabled ||
> +      forkables_needs == forkables_needless ||
> +      forkables_needs == forkables_created)
> +    {
> +      /* (re-)check existence of forkables dir */
> +      PWCHAR pbuf = nt_max_path_buf ();
> +      for (namepart const *part = forkable_nameparts; part->text; ++part)
> +	{
> +	  if (part->textfunc)
> +	    pbuf += part->textfunc (pbuf, -1);
> +	  else
> +	    pbuf += __small_swprintf (pbuf, L"%W", part->text);
> +	  if (part->mutex_from_dir)
> +	    break; /* up to first mutex-naming dir */
> +	}
> +      pbuf = nt_max_path_buf ();
> +
> +      HANDLE dh = ntopenfile (pbuf, NULL, FILE_DIRECTORY_FILE);
> +      if (dh)
> +	{
> +	  NtClose (dh);
> +	  if (forkables_needs == forkables_disabled)
> +	    forkables_needs = forkables_needless;
> +	}
> +      else if (forkables_needs != forkables_disabled)
> +	{
> +	  debug_printf ("disabled, disappearing %W", pbuf);
> +	  close_mutex ();
> +	  denominate_forkables ();
> +	  forkables_needs = forkables_disabled;
> +	}
> +      else
> +	debug_printf ("disabled, missing %W", pbuf);
> +    }
> +
> +  if (forkables_needs == forkables_disabled)
> +    return;
> +
> +  if (forkables_needs == forkables_created)
> +    {
> +      /* already have created hardlinks in this process, ... */
> +      forkables_needs = forkables_needless;
> +      d = &start;
> +      while ((d = d->next) != NULL)
> +	if (d->forkable_ntname && !*d->forkable_ntname)
> +	  {
> +	    /* ... but another dll was loaded since last fork */
> +	    debug_printf ("needed, since last fork loaded %W", d->ntname);
> +	    forkables_needs = forkables_needed;
> +	    break;
> +	  }
> +    }
> +
> +  if (forkables_needs > forkables_needless)
> +    return; /* no need to check anything else */
> +
> +  if (forkables_needs != forkables_needless)
> +    {
> +      /* paranoia */
> +      system_printf ("WARNING: invalid forkables_needs value %d",
> +		     forkables_needs);
> +      return;
> +    }
> +
> +  if (!forkables_mutex)
> +    {
> +      /* debugging: check for ".unchecked" file in toplevel forkables dir,
> +	 that is: always (try to) create the hardlinks.  Need to find out
> +	 if just trying to create the hardlinks is faster than reading
> +	 all the hardlink's internal and basic informations. */
> +      PWCHAR pbuf = nt_max_path_buf ();
> +      for (namepart const *part = forkable_nameparts; part->text; ++part)
> +	{
> +	  if (part->textfunc)
> +	    pbuf += part->textfunc (pbuf, -1);
> +	  else
> +	    pbuf += __small_swprintf (pbuf, L"%W", part->text);
> +	  if (part->mutex_from_dir)
> +	    break; /* up to first mutex-naming dir */
> +	}
> +      pbuf += __small_swprintf (pbuf, L"%W.unchecked", PATHSEP);
> +
> +      pbuf = nt_max_path_buf ();
> +      HANDLE dh = ntopenfile (pbuf);
> +      if (dh)
> +	{
> +	  NtClose (dh);
> +	  forkables_needs = forkables_needed;
> +	  debug_printf ("needed, found %W", pbuf);
> +	  return;
> +	}
> +    }
> +
> +  /* We have to check the main-executable and all dlls loaded so far via
> +     their forked (if available, or their original) filename, if they are
> +     still available for loading again. */
> +  HANDLE fh = NULL;
> +  d = &start;
> +  while ((d = d->next) != NULL)
> +    {
> +      if (!d->fhandle)
> +	continue;
> +
> +      if (fh)
> +	{
> +	  NtClose (fh);
> +	  fh = NULL;
> +	}
> +
> +      PWCHAR ntname = d->forkedntname ();
> +      fh = ntopenfile (ntname);
> +      if (!fh)
> +	{
> +	  debug_printf ("needed, something wrong with %W", ntname);
> +	  forkables_needs = forkables_needed;
> +	  break;
> +	}
> +
> +      FILE_INTERNAL_INFORMATION fii_now;
> +      if (!read_fii (d->fhandle, &d->fii, true) ||
> +	  !read_fii (fh, &fii_now) ||
> +	  d->fii.IndexNumber.QuadPart != fii_now.IndexNumber.QuadPart)
> +        {
> +	  debug_printf ("needed, found modified %W", ntname);
> +	  forkables_needs = forkables_needed;
> +	  break;
> +	}
> +
> +      FILE_BASIC_INFORMATION fbi_now;
> +      if (!read_fbi (d->fhandle, &d->fbi, true) ||
> +	  !read_fbi (fh, &fbi_now) ||
> +	  d->fbi.LastWriteTime.QuadPart != fbi_now.LastWriteTime.QuadPart)
> +	{
> +	  system_printf ("WARNING: changed by same file id %W"
> +			 " (now lastwritetime %016X, old lastwritetime %016X)",
> +			 ntname,
> +			 fbi_now.LastWriteTime.QuadPart,
> +			 d->fbi.LastWriteTime.QuadPart);
> +	  forkables_needs = forkables_needed;
> +	  break;
> +	}
> +    }
> +  if (fh)
> +    NtClose (fh);
> +
> +  if (forkables_needs == forkables_needless && !forkables_mutex)
> +    {
> +      /* debugging: check for ".needed" file in toplevel forkables dir */
> +      PWCHAR pbuf = nt_max_path_buf ();
> +      for (namepart const *part = forkable_nameparts; part->text; ++part)
> +	{
> +	  if (part->textfunc)
> +	    pbuf += part->textfunc (pbuf, -1);
> +	  else
> +	    pbuf += __small_swprintf (pbuf, L"%W", part->text);
> +	  if (part->mutex_from_dir)
> +	    break; /* up to first mutex-naming dir */
> +	}
> +      pbuf += __small_swprintf (pbuf, L"%W.needed", PATHSEP);
> +
> +      pbuf = nt_max_path_buf ();
> +      HANDLE dh = ntopenfile (pbuf);
> +      if (dh)
> +	{
> +	  NtClose (dh);
> +	  forkables_needs = forkables_needed;
> +	  debug_printf ("needed, found %W", pbuf);
> +	}
> +    }
> +}
> +
> +/* Create the nominated forkable hardlinks and directories as necessary,
> +   mutex-protected to avoid concurrent processes removing them. */
> +bool
> +dll_list::update_forkables ()
> +{
> +  /* existence of mutex indicates that we use these hardlinks */
> +  if (!forkables_mutex)
> +    {
> +      /* neither my parent nor myself did have need for hardlinks yet */
> +      forkables_mutex = CreateMutexW (&sec_none, FALSE,
> +				      forkables_mutex_name);
> +      debug_printf ("%p = CreateMutexW (%W): %E",
> +		    forkables_mutex, forkables_mutex_name);
> +      if (!forkables_mutex)
> +	return false;
> +
> +      /* Make sure another process does not rmdirs_synchronized () */
> +      debug_printf ("WFSO (%p, %W, inf)...",
> +		    forkables_mutex, forkables_mutex_name);
> +      DWORD ret = WaitForSingleObject (forkables_mutex, INFINITE);
> +      debug_printf ("%u = WFSO (%p, %W)",
> +		    ret, forkables_mutex, forkables_mutex_name);
> +      switch (ret)
> +	{
> +	case WAIT_OBJECT_0:
> +	case WAIT_ABANDONED:
> +	  break;
> +	default:
> +	  system_printf ("cannot wait for mutex %W: %E",
> +			 forkables_mutex_name);
> +	  return false;
> +	}
> +
> +      BOOL bret = ReleaseMutex (forkables_mutex);
> +      debug_printf ("%d = ReleaseMutex (%p, %W)",
> +		    bret, forkables_mutex, forkables_mutex_name);
> +    }
> +
> +  return create_forkables ();
> +}
> +
> +/* Create the nominated forkable hardlinks and directories as necessary,
> +   as well as the .local file for dll-redirection. */
> +bool
> +dll_list::create_forkables ()
> +{
> +  bool success = true;
> +
> +  int lastsepcount = 1; /* we have trailing pathsep */
> +  for (namepart const *part = forkable_nameparts; part->text; ++part)
> +    if (part->create_dir)
> +      ++lastsepcount;
> +
> +  PWCHAR ntname = nt_max_path_buf ();
> +  wcsncpy (ntname, forkables_dirx_ntname, NT_MAX_PATH);
> +
> +  if (!mkdirs (ntname, lastsepcount))
> +    success = false;
> +
> +  if (success)
> +    {
> +      /* create the DotLocal file as empty file */
> +      wcsncat (ntname, main_executable->modname, NT_MAX_PATH);
> +      wcsncat (ntname, L".local", NT_MAX_PATH);
> +
> +      UNICODE_STRING fn;
> +      RtlInitUnicodeString (&fn, ntname);
> +
> +      OBJECT_ATTRIBUTES oa;
> +      InitializeObjectAttributes (&oa, &fn, 0, NULL,
> +				  sec_none_nih.lpSecurityDescriptor);
> +      HANDLE hlocal = NULL;
> +      NTSTATUS status;
> +      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_NON_DIRECTORY_FILE
> +			     | FILE_SYNCHRONOUS_IO_NONALERT,
> +			     NULL, 0);
> +      if (NT_SUCCESS (status))
> +	CloseHandle (hlocal);
> +      else
> +	success = false;
> +      debug_printf ("%y = NtCreateFile (%p, %W)", status, hlocal, ntname);
> +    }
> +
> +  if (success)
> +    {
> +      dll *d = &start;
> +      while ((d = d->next))
> +	if (!d->create_forkable ())
> +	  d->nominate_forkable (NULL); /* never again */
> +      debug_printf ("hardlinks created");
> +    }
> +
> +  return success;
> +}
> +
> +static void
> +rmdirs_synchronized (WCHAR ntbuf[NT_MAX_PATH], int depth, int maxdepth,
> +		     PFILE_DIRECTORY_INFORMATION pfdi, ULONG fdisize)
> +{
> +  if (depth == maxdepth)
> +    {
> +      debug_printf ("sync on %W", ntbuf);
> +      /* calculate mutex name from path parts, using
> +	 full path name length to allocate mutex name buffer */
> +      WCHAR mutexname[wcslen (ntbuf)];
> +      mutexname[0] = L'\0';
> +      PWCHAR mutexnext = mutexname;
> +
> +      /* mutex name is formed by dir names */
> +      int pathcount = 0;
> +      for (namepart const *part = forkable_nameparts; part->text; ++part)
> +	if (part->mutex_from_dir)
> +	  ++pathcount;
> +
> +      PWCHAR pathseps[pathcount];
> +
> +      /* along the path separators split needed path parts */
> +      int i = pathcount;
> +      while (--i >= 0)
> +	if ((pathseps[i] = wcsrchr (ntbuf, L'\\')))
> +	  *pathseps[i] = L'\0';
> +	else
> +	  return; /* something's wrong */
> +
> +      /* build the mutex name from dir names */
> +      for (i = 0; i < pathcount; ++i)
> +	{
> +	  if (i > 0)
> +	    mutexnext = wcpcpy (mutexnext, MUTEXSEP);
> +	  mutexnext = wcpcpy (mutexnext, &pathseps[i][1]);
> +	  *pathseps[i] = L'\\'; /* restore full path */
> +	}
> +
> +      HANDLE mutex = CreateMutexW (&sec_none_nih, TRUE, mutexname);
> +      DWORD lasterror = GetLastError ();
> +      debug_printf ("%p = CreateMutexW (%W): %E", mutex, mutexname);
> +      if (mutex)
> +	{
> +	  if (lasterror != ERROR_ALREADY_EXISTS)
> +	    rmdirs (ntbuf);
> +	  BOOL bret = CloseHandle (mutex);
> +	  debug_printf ("%d = CloseHandle (%p, %W): %E",
> +			bret, mutex, mutexname);
> +	}
> +      return;
> +    }
> +
> +  IO_STATUS_BLOCK iosb;
> +  NTSTATUS status;
> +
> +  HANDLE hdir = dll_list::ntopenfile (ntbuf, &status,
> +				 FILE_DIRECTORY_FILE |
> +				 (depth ? FILE_DELETE_ON_CLOSE : 0));
> +  if (!hdir)
> +    return;
> +
> +  PWCHAR plast = ntbuf + wcslen (ntbuf);
> +  while (NT_SUCCESS (status = NtQueryDirectoryFile (hdir,
> +						    NULL, NULL, NULL, &iosb,
> +						    pfdi, fdisize,
> +						    FileDirectoryInformation,
> +						    TRUE, NULL, FALSE)))
> +    if (pfdi->FileAttributes & FILE_ATTRIBUTE_DIRECTORY)
> +      {
> +	int namelen = pfdi->FileNameLength / sizeof (WCHAR);
> +	if (!wcsncmp (pfdi->FileName, L".", namelen) ||
> +	    !wcsncmp (pfdi->FileName, L"..", namelen))
> +	  continue;
> +	*plast = L'\\';
> +	wcsncpy (plast+1, pfdi->FileName, namelen);
> +	plast[1+namelen] = L'\0';
> +	rmdirs_synchronized (ntbuf, depth+1, maxdepth, pfdi, fdisize);
> +	*plast = L'\0';
> +      }
> +  if (status != STATUS_NO_MORE_FILES)
> +    debug_printf ("%y = NtQueryDirectoryFile (%p, io %y, info %d)",
> +		  status, hdir, iosb.Status, iosb.Information);
> +  CloseHandle (hdir);
> +}
> +
> +/* Try to lock the mutex handle with almost no timeout, then close the
> +   mutex handle.  Locking before closing is to get the mutex closing
> +   promoted synchronously, otherways we might end up with no one
> +   succeeding in create-with-lock, which is the precondition
> +   to actually remove the hardlinks from the filesystem. */
> +bool
> +dll_list::close_mutex ()
> +{
> +  if (!forkables_mutex || !*forkables_mutex_name)
> +    return false;
> +
> +  HANDLE hmutex = forkables_mutex;
> +  forkables_mutex = NULL;
> +
> +  bool locked = false;
> +  DWORD ret = WaitForSingleObject (hmutex, 1);
> +  debug_printf ("%u = WFSO (%p, %W, 1)",
> +		ret, hmutex, forkables_mutex_name);
> +  switch (ret)
> +    {
> +    case WAIT_OBJECT_0:
> +    case WAIT_ABANDONED:
> +      locked = true;
> +      break;
> +    case WAIT_TIMEOUT:
> +      break;
> +    default:
> +      system_printf ("error locking mutex %W: %E", forkables_mutex_name);
> +      break;
> +    }
> +  BOOL bret = CloseHandle (hmutex);
> +  debug_printf ("%d = CloseHandle (%p, %W): %E",
> +		bret, hmutex, forkables_mutex_name);
> +  return locked;
> +}
> +
> +/* Release the forkable hardlinks, and remove them if the
> +   mutex can be create-locked after locked-closing. */
> +void
> +dll_list::cleanup_forkables ()
> +{
> +  bool locked = close_mutex ();
> +
> +  if (!forkables_dirx_ntname)
> +    return;
> +
> +  /* Start the removal below with current forkables dir,
> +     which is cleaned in denominate_forkables (). */
> +  PWCHAR buf = nt_max_path_buf ();
> +  PWCHAR pathsep = wcpncpy (buf, forkables_dirx_ntname, NT_MAX_PATH);
> +  buf[NT_MAX_PATH-1] = L'\0';
> +
> +  denominate_forkables ();
> +
> +  if (!locked)
> +    return;
> +
> +  /* drop last path separator */
> +  while (--pathsep >= buf && *pathsep != L'\\');
> +  *pathsep = L'\0';
> +
> +  try_remove_forkables (buf, pathsep - buf, NT_MAX_PATH);
> +}
> +
> +void
> +dll_list::try_remove_forkables (PWCHAR dirbuf, size_t dirlen, size_t dirbufsize)
> +{
> +  /* Instead of just the current forkables, try to remove any forkables
> +     found, to ensure some cleanup even in situations like power-loss. */
> +  PWCHAR end = dirbuf + wcslen (dirbuf);
> +  int backcount = 0;
> +  for (namepart const *part = forkable_nameparts; part->text; ++part)
> +    if (part->create_dir)
> +      {
> +	/* drop one path separator per create_dir */
> +	while (--end >= dirbuf && *end != L'\\');
> +	if (end < dirbuf)
> +	  return;
> +	*end = L'\0';
> +	++backcount;
> +      }
> +
> +  /* reading one at a time to reduce stack pressure */
> +  struct {
> +    FILE_DIRECTORY_INFORMATION fdi;
> +    WCHAR buf[NAME_MAX];
> +  } fdibuf;
> +  rmdirs_synchronized (dirbuf, 0, backcount, &fdibuf.fdi, sizeof (fdibuf));
> +}
> +
> +void
> +dll_list::denominate_forkables ()
> +{
> +  if (forkables_dirx_ntname)
> +    {
> +      *forkables_dirx_ntname = L'\0';
> +      *forkables_mutex_name = L'\0';
> +    }
> +
> +  dll *d = &start;
> +  while ((d = d->next))
> +    d->nominate_forkable (forkables_dirx_ntname);
> +}
> +
>  /* Set or clear HANDLE_FLAG_INHERIT for all handles necessary
>     to maintain forkables-hardlinks. */
>  void
> @@ -35,6 +1027,9 @@ dll_list::set_forkables_inheritance (bool inherit)
>    DWORD mask = HANDLE_FLAG_INHERIT;
>    DWORD flags = inherit ? HANDLE_FLAG_INHERIT : 0;
>  
> +  if (forkables_mutex)
> +    SetHandleInformation (forkables_mutex, mask, flags);
> +
>    dll *d = &start;
>    while ((d = d->next))
>      if (d->fhandle)
> @@ -45,11 +1040,52 @@ dll_list::set_forkables_inheritance (bool inherit)
>  void
>  dll_list::request_forkables ()
>  {
> +  /* Even on forkables_impossible, keep the number of open handles
> +     stable across the fork, and close them when releasing only. */
> +  prepare_forkables_nomination ();
> +
> +  update_forkables_needs ();
> +
>    set_forkables_inheritance (true);
> +
> +  if (forkables_needs <= forkables_needless)
> +    return;
> +
> +  dll *d = &start;
> +  while ((d = d->next))
> +    d->nominate_forkable (forkables_dirx_ntname);
> +
> +  bool updated = update_forkables ();
> +
> +  if (!updated)
> +    forkables_needs = forkables_needless;
> +  else
> +    forkables_needs = forkables_created;
>  }
>  
> +
>  void
>  dll_list::release_forkables ()
>  {
>    set_forkables_inheritance (false);
> +
> +  if (forkables_needs == forkables_impossible)
> +    {
> +      cleanup_forkables ();
> +
> +      dll *d = &start;
> +      while ((d = d->next))
> +	if (d->fhandle)
> +	  {
> +	    NtClose (d->fhandle);
> +	    d->fhandle = NULL;
> +	    d->forkable_ntname = NULL;
> +	  }
> +
> +      if (forkables_dirx_ntname) {
> +	cfree (forkables_dirx_ntname);
> +	forkables_dirx_ntname = NULL;
> +	forkables_mutex_name = NULL;
> +      }
> +    }
>  }
> diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
> index d4b2afb..e414a26 100644
> --- a/winsup/cygwin/pinfo.cc
> +++ b/winsup/cygwin/pinfo.cc
> @@ -28,6 +28,7 @@ details. */
>  #include "cygtls.h"
>  #include "tls_pbuf.h"
>  #include "child_info.h"
> +#include "dll_init.h"
>  
>  class pinfo_basic: public _pinfo
>  {
> @@ -225,6 +226,8 @@ pinfo::exit (DWORD n)
>    int exitcode = self->exitcode & 0xffff;
>    if (!self->cygstarted)
>      exitcode = ((exitcode & 0xff) << 8) | ((exitcode >> 8) & 0xff);
> +  sigproc_printf ("Calling dlls.cleanup_forkables n %y, exitcode %y", n, exitcode);
> +  dlls.cleanup_forkables ();
>    sigproc_printf ("Calling ExitProcess n %y, exitcode %y", n, exitcode);
>    if (!TerminateProcess (GetCurrentProcess (), exitcode))
>      system_printf ("TerminateProcess failed, %E");
> diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
> index 15fb8ce..7f5354e 100644
> --- a/winsup/cygwin/syscalls.cc
> +++ b/winsup/cygwin/syscalls.cc
> @@ -689,8 +689,8 @@ check_dir_not_empty (HANDLE dir, path_conv &pc)
>    return STATUS_SUCCESS;
>  }
>  
> -NTSTATUS
> -unlink_nt (path_conv &pc)
> +static NTSTATUS
> +_unlink_nt (path_conv &pc, bool shareable)
>  {
>    NTSTATUS status;
>    HANDLE fh, fh_ro = NULL;
> @@ -771,6 +771,9 @@ retry_open:
>       bin so that it actually disappears from its directory even though its
>       in use.  Otherwise, if opening doesn't fail, the file is not in use and
>       we can go straight to setting the delete disposition flag.
> +     However, while we have the file open with FILE_SHARE_DELETE, using
> +     this file via another hardlink for anything other than DELETE by
> +     concurrent processes fails. The 'shareable' argument is to prevent this.
>  
>       NOTE: The missing sharing modes FILE_SHARE_READ and FILE_SHARE_WRITE do
>  	   NOT result in a STATUS_SHARING_VIOLATION, if another handle is
> @@ -780,7 +783,10 @@ retry_open:
>  	   will succeed.  So, apparently there is no reliable way to find out
>  	   if a file is already open elsewhere for other purposes than
>  	   reading and writing data.  */
> -  status = NtOpenFile (&fh, access, &attr, &io, FILE_SHARE_DELETE, flags);
> +  if (shareable)
> +    status = STATUS_SHARING_VIOLATION;
> +  else
> +    status = NtOpenFile (&fh, access, &attr, &io, FILE_SHARE_DELETE, flags);
>    /* STATUS_SHARING_VIOLATION is what we expect. STATUS_LOCK_NOT_GRANTED can
>       be generated under not quite clear circumstances when trying to open a
>       file on NFS with FILE_SHARE_DELETE only.  This has been observed with
> @@ -1026,6 +1032,18 @@ out:
>    return status;
>  }
>  
> +NTSTATUS
> +unlink_nt (path_conv &pc)
> +{
> +  return _unlink_nt (pc, false);
> +}
> +
> +NTSTATUS
> +unlink_nt_shareable (path_conv &pc)
> +{
> +  return _unlink_nt (pc, true);
> +}
> +
>  extern "C" int
>  unlink (const char *ourname)
>  {
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/6] forkables: Create forkable hardlinks, yet unused.
  2016-03-30 18:54 ` [PATCH 3/6] forkables: Create forkable hardlinks, yet unused Michael Haubenwallner
  2016-11-16 12:25   ` Michael Haubenwallner
@ 2016-11-16 12:34   ` Michael Haubenwallner
  2016-11-17 10:08     ` Corinna Vinschen
  1 sibling, 1 reply; 16+ messages in thread
From: Michael Haubenwallner @ 2016-11-16 12:34 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 3366 bytes --]

(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(-)


[-- Attachment #2: 0001-forkables-fix-creating-dirs-and-.local-file-in-paral.patch --]
[-- Type: text/x-patch, Size: 2247 bytes --]

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 3/6] forkables: Create forkable hardlinks, yet unused.
  2016-11-16 12:34   ` Michael Haubenwallner
@ 2016-11-17 10:08     ` Corinna Vinschen
  0 siblings, 0 replies; 16+ messages in thread
From: Corinna Vinschen @ 2016-11-17 10:08 UTC (permalink / raw)
  To: cygwin-patches

[-- Attachment #1: Type: text/plain, Size: 957 bytes --]

On Nov 16 13:34, Michael Haubenwallner wrote:
> (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...

I'm still pretty unhappy with this patch.  It adds *lots* of code
to handle a seldom border case.

Assuming you perform some action which starts lots of processes.
Like, say, a bigger build.  Let's say, you install the coreutils
source package and run `time cygport coreutils.cygport prep build'.

If you do this thrice, once without your patch, once with your patch
but without utilizing it, and once with your patch and utilizing it,
how do they compare?  Do you have numbers?


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2016-11-17 10:08 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-30 18:54 [PATCH 0/6] Protect fork() against dll- and exe-updates Michael Haubenwallner
2016-03-30 18:54 ` [PATCH 4/6] forkables: Protect fork against dll-, exe-updates Michael Haubenwallner
2016-03-30 19:04   ` Yaakov Selkowitz
2016-03-30 19:12     ` Michael Haubenwallner
2016-04-01 12:20       ` Corinna Vinschen
2016-03-30 18:54 ` [PATCH 1/6] forkables: Store dll file name as full NT path Michael Haubenwallner
2016-03-30 18:54 ` [PATCH 3/6] forkables: Create forkable hardlinks, yet unused Michael Haubenwallner
2016-11-16 12:25   ` Michael Haubenwallner
2016-11-16 12:34   ` Michael Haubenwallner
2016-11-17 10:08     ` Corinna Vinschen
2016-03-30 18:54 ` [PATCH 2/6] forkables: Track main executable and cygwin1.dll Michael Haubenwallner
2016-03-30 18:55 ` [PATCH 0/6] Protect fork() against dll- and exe-updates Daniel Colascione
2016-03-30 19:04   ` Michael Haubenwallner
2016-04-01 12:19   ` Corinna Vinschen
2016-03-30 19:24 ` [PATCH 5/6] forkables: Keep hardlinks disabled via shared mem Michael Haubenwallner
2016-03-30 19:24 ` [PATCH 6/6] forkables: Document hardlink creation at forktime Michael Haubenwallner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).