Protect fork() against dll- and exe-updates.

[Corinna Vinschen]

On Nov 16 10:01, Michael Haubenwallner wrote:
> Hi Corinna,
> 
> On 11/14/2015 11:25 AM, Corinna Vinschen wrote:
> > On Nov 13 17:12, Michael Haubenwallner wrote:
> 
> >> have reworked the hardlink-creation from scratch as discussed before,
> >> now using /var/run/cygfork/ as the top-level hardlinks directory.
> >>
> >> * At process start and during LoadLibrary, handles to all the loaded
> >>   dlls (including cygwin1.dll) and the main executable are opened.
> 
> >> * If they are not identical (any more), hardlinks to these dlls are
> >>   created in subdirectories into /var/run/cygfork/<sid>/.
> > 
> > This description puzzles me a bit.  If the DLLs have changed, isn't
> > it too late to create the hardlink?!?  I guess I'm missing something.
> 
> The hardlink is created using the handle opened during dll-initialization
> (as we have discussed that before).

That's what I missed.  Duh.

> Agreed there is the short time frame
> between process start (where windows does open the dll) and opening the
> file handle in dll-initialization, where the dll could be replaced - but
> I'm not sure if that really will be that much of a problem.
> 
> >> * For the timing: Building cygwin-2.4.0-0.2 three times, the duration
> >>   difference is in the range of measuring fault - almost identical for
> >>   each possible variant vanilla,disabled,enabled,forced.
> > 
> > Wow.  Really?
> > 
> >> More thoughts?
> > 
> > Yes, but I'm not sure yet.  I'm just wondering if /var/run or any
> > other such dir is such a good idea after all.  I have this vague
> > notion that a subdir of the directory the DLL is installed in might
> > be safer in the log run (for DLLs in /bin a subdir of /bin, etc).
> 
> While I don't really care about /var/run, I doubt that /bin necessarily
> is writeable for each user using a particular cygwin instance - or we
> would have to create some cygfork directory with a=rwxt permission there
> beforehand.

Same could happen with a subdir under /var/run in theory.  Well, Setup
creates the dir with 1777 perms of course, but in a corp environment you
can't be sure installations are performed with Setup at all.  But, yes,
I see the point.

> Also, for the dll-redirection to work, all the linktime
> dll's need to be available nearby app.exe and the app.exe.local file,

Ok.  We're talking about pre-installed binaries in the first place,
right?  I'm not yet clear on how this is going to handle binaries
installed to another drive...

> either as hardlink, copy, or symlink(?). Otherways we might have to
> create some app.exe.manifest file - I've got lost in that docs...

Heh, yeah.  The manifest is pretty ugly, IMHO, especially the
compatibility stuff since Windows 8.1.

> >> More to discuss?
> > 
> > I'm a bit swamped ATM, but I'll try to take a look into your patch next
> > week or the week after.  As for discussing, yes, I think so.  Please
> > keep in mind that your patch is pretty intrusive, so there's certainly
> > something to discuss :}
> 
> ATM, there's one thing I'm unsure whether it is necessary at all, and how
> to do if yes: Purging the bin - where the hardlinks get moved to.

We look into that when we get to it.

Btw., I just skimmed your patch a bit and I stumbled over the usage of
GetFileInformationByHandle.

In dll_list::ctimename, please use NtQueryInformationFile with
FileBasicInformation instead.  It allows to reduce the filetime
comparison to a single test using the QuadPart of the timestamp and
__small_swprintf simply uses a %016X.

Analogue in dll_list::update_forkables_needs.  Do you really need to
check the volume serial number?  Otherwise, just call
NtQueryInformationFile(FileInternalInformation).

> > I'm planning to fold in the cygwin-acl branch the week after and then
> > release 2.4.0.  From there on we should create a feature branch from
> > mainline and apply your changes to perform further testing and tweaking
> > as necessary.
> > 
> > Sounds ok?
> 
> Absolutely fine with me!

Cool, thanks!


Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-developers/attachments/20151116/d2a35f10/attachment.sig>