Re: [PATCH 06/11] Remove always true nonnull check on "this" pointer.
Hi Peter,
On Mar 19 13:45, Peter Foley wrote:
> G++ 6.0 can assert that the this pointer is non-null for member functions.
Maybe, but if it compains, it's bound to find false positives...
> @@ -502,7 +502,7 @@ fhandler_dev_dsp::Audio_out::buf_info (audio_buf_info *p,
> int rate, int bits, int channels)
> {
>p->fragstotal = MAX_BLOCKS;
> - if (this && dev_)
> + if (dev_)
> {
>/* If the device is running we use the internal values,
>possibly set from the wave file. */
This is wrong. fhandler_dev_dsp::Audio_out::buf_info is called from
fhandler_dev_dsp::_ioctl. An application can call ioctl without
an open audio channel. If so, audio_out_ may be NULL and the
Audio_out::buf_info method is called with a NULL this pointer..
> @@ -959,7 +959,7 @@ fhandler_dev_dsp::Audio_in::buf_info (audio_buf_info *p,
> {
>p->fragstotal = MAX_BLOCKS;
>p->fragsize = blockSize (rate, bits, channels);
> - if (this && dev_)
> + if (dev_)
Ditto.
> {
>p->fragments = Qisr2app_->query ();
>if (pHdr_ != NULL)
> diff --git a/winsup/cygwin/path.cc b/winsup/cygwin/path.cc
> index 20391bf..df09d70 100644
> --- a/winsup/cygwin/path.cc
> +++ b/winsup/cygwin/path.cc
> @@ -3937,7 +3937,7 @@ fcwd_access_t::Free (PVOID heap)
> {
>/* Decrement the reference count. If it's down to 0, free
> structure from heap. */
> - if (this && InterlockedDecrement (&ReferenceCount ()) == 0)
> + if (InterlockedDecrement (&ReferenceCount ()) == 0)
Very unlikely, but *fast_cwd_ptr *might be NULL. Better save than sorry.
> {
>/* In contrast to pre-Vista, the handle on init is always a
>fresh one and not the handle inherited from the parent
> diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
> index be32cfd..409a0b7 100644
> --- a/winsup/cygwin/pinfo.cc
> +++ b/winsup/cygwin/pinfo.cc
> @@ -514,7 +514,7 @@ _pinfo::set_ctty (fhandler_termios *fh, int flags)
> bool __reg1
> _pinfo::exists ()
> {
> - return this && process_state && !(process_state & (PID_EXITED | PID_REAPED
> | PID_EXECED));
> + return process_state && !(process_state & (PID_EXITED | PID_REAPED |
> PID_EXECED));
Assuming a pinfo constructor called like this:
pinfo p (non_existent_pid);
then p->_procinfo is NULL. Calling p->exists then calls _pinfo::exists
with a NULL this pointer. Analog for the other _pinfo methods.
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
signature.asc
Description: PGP signature
Re: [PATCH v2 1/3] Add option to not build mingw programs when cross compiling.
On Mar 23 09:34, Peter Foley wrote: > Add an option to not require a mingw compiler when bootstrapping a cross > toolchain. > Defaults to existing behavior. > Also update some obsolete macros. Applied with changes. The below check was skewed. > +if test "x$with_mingw_progs" != xyes; then > +AC_CONFIG_SUBDIRS([utils lsaauth]) > +fi Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat signature.asc Description: PGP signature
Re: [PATCH 3/3] Use just-built gcc for windres
On Mar 23 09:34, Peter Foley wrote: > When building cygwin in a combined tree with binutils, > the just-built windres cannot find the just-buit gcc automatically. > Parse the CC env variable to use the correct compiler, rather then > falling back to the build-system's gcc which does not define the proper > preprocessor macros. > > winsup/cygwin/ChangeLog > mkvers.sh: Manually specify preprocessor based on $CC Are you sure this works as desired? In my standard cross build environment, the only -B option added here is --preprocessor-arg=-B/build/cygwin/x86_64/vanilla/x86_64-pc-cygwin/newlib/ ? Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat signature.asc Description: PGP signature
Re: [PATCH 3/3] Use just-built gcc for windres
On Wed, Mar 30, 2016 at 8:31 AM, Corinna Vinschen wrote: > Are you sure this works as desired? In my standard cross build > environment, the only -B option added here is > > --preprocessor-arg=-B/build/cygwin/x86_64/vanilla/x86_64-pc-cygwin/newlib/ The only case in which -B is needed at all is for an in-tree build. That is, gcc and winsup are in the same source tree, so CC is set to something like ./gcc/xgcc. In that case, the -B is necessary so that gcc knows where to find the cc1 binary. If gcc and winsup are built from separate source trees, it isn't an issue. Thanks, Peter
Re: [PATCH v2 1/3] Add option to not build mingw programs when cross compiling.
On Wed, Mar 30, 2016 at 8:11 AM, Corinna Vinschen wrote: > Applied with changes. The below check was skewed. > >> +if test "x$with_mingw_progs" != xyes; then >> +AC_CONFIG_SUBDIRS([utils lsaauth]) >> +fi Whoops, good catch.
Re: [PATCH 06/11] Remove always true nonnull check on "this" pointer.
On Wed, Mar 30, 2016 at 7:24 AM, Corinna Vinschen wrote: > Hi Peter, > > On Mar 19 13:45, Peter Foley wrote: >> G++ 6.0 can assert that the this pointer is non-null for member functions. > > Maybe, but if it compains, it's bound to find false positives... > Alright, I'll take a closer look at this.
[PATCH] fix typo in netinit/ip.h
The type for the ip_tos member was typoed, fix it.
winsup/cygwin/ChangeLog:
include/netinet/ip.h: fix type of ip_tos
Signed-off-by: Peter Foley
---
winsup/cygwin/include/netinet/ip.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/winsup/cygwin/include/netinet/ip.h
b/winsup/cygwin/include/netinet/ip.h
index b952d53..1f6ebbe 100644
--- a/winsup/cygwin/include/netinet/ip.h
+++ b/winsup/cygwin/include/netinet/ip.h
@@ -63,7 +63,7 @@ struct ip {
ip_hl:4; /* header length */
#endif
#endif /* not _IP_VHL */
- u_int_8 ip_tos; /* type of service */
+ u_int8_t ip_tos; /* type of service */
u_int16_t ip_len; /* total length */
u_int16_t ip_id;/* identification */
u_int16_t ip_off; /* fragment offset field */
--
2.8.0
Re: [PATCH] fix typo in netinit/ip.h
On Mar 30 09:15, Peter Foley wrote: > The type for the ip_tos member was typoed, fix it. > > winsup/cygwin/ChangeLog: > include/netinet/ip.h: fix type of ip_tos Thanks for catching. Applied. Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat signature.asc Description: PGP signature
Re: [PATCH 3/3] Use just-built gcc for windres
On Mar 23 09:34, Peter Foley wrote: > When building cygwin in a combined tree with binutils, > the just-built windres cannot find the just-buit gcc automatically. > Parse the CC env variable to use the correct compiler, rather then > falling back to the build-system's gcc which does not define the proper > preprocessor macros. > > winsup/cygwin/ChangeLog > mkvers.sh: Manually specify preprocessor based on $CC Applied. Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Maintainer cygwin AT cygwin DOT com Red Hat signature.asc Description: PGP signature
[PATCH 2/6] forkables: Track main executable and cygwin1.dll.
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.fix
[PATCH 0/6] Protect fork() against dll- and exe-updates.
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/msg2.html https://cygwin.com/ml/cygwin-developers/2016-03/msg5.html http://thread.gmane.org/gmane.os.cygwin.devel/1378 Thanks for review! /haubi/
[PATCH 4/6] forkables: Protect fork against dll-, exe-updates.
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 ()));
[PATCH 1/6] forkables: Store dll file name as full NT path.
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 */
+
[PATCH 3/6] forkables: Create forkable hardlinks, yet unused.
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';
+ }
+
Re: [PATCH 0/6] Protect fork() against dll- and exe-updates.
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/msg2.html > https://cygwin.com/ml/cygwin-developers/2016-03/msg5.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. signature.asc Description: OpenPGP digital signature
Re: [PATCH 4/6] forkables: Protect fork against dll-, exe-updates.
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
Re: [PATCH 0/6] Protect fork() against dll- and exe-updates.
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/msg2.html >> https://cygwin.com/ml/cygwin-developers/2016-03/msg5.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/
Re: [PATCH 4/6] forkables: Protect fork against dll-, exe-updates.
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/
[PATCH 5/6] forkables: Keep hardlinks disabled via shared mem.
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
[PATCH 6/6] forkables: Document hardlink creation at forktime.
* 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. +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. + 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. +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 . +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 https://social.msdn.microsoft.com/search/en-US?query=dotlocal%20dll%20redirection"; +>DotLocal (.local) Dll Redirection 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. + +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. + +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. + 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
