This is the mail archive of the cygwin-developers mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Nov 16 19:07, Michael Haubenwallner wrote:
> On 11/16/2015 11:22 AM, Corinna Vinschen wrote:
> > On Nov 16 10:01, Michael Haubenwallner wrote:
> >> On 11/14/2015 11:25 AM, Corinna Vinschen wrote:
> >>> On Nov 13 17:12, Michael Haubenwallner wrote:
> 
> >> 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?
> 
> What do you call pre-installed binaries? It doesn't matter for the
> binary when and by whom it was installed, either setup.exe - which
> is non-cygwin, or some in-cygwin package manager - which relies on
> the update-safe fork when replacing in-use binaries.

What I meant was binaries in the distro tree in contrast to out-of-tree
binaries.  I was just thinking aloud about the need (or lack thereof) to
handle binaries installed on drives other than the Cygwin installation
drive.

> > I'm not yet clear on how this is going to handle binaries
> > installed to another drive...
> 
> For now, when NtSetInformationFile (FileLinkInformation) does fail
> for whatever reason (I cannot think of anything else than another
> drive), this file is not hardlinked and will be used from its original
> location. Must admit I haven't tested this scenario.

Ok, something to keep in mind.

> >> either as hardlink, copy, or symlink(?).
> 
> However, I can imagine two different cross-drive approaches:
> * Either create some cygfork-directory on each drive to hold the
>   hardlinks (still requires NTFS), and put symlinks into /var/run/
> * or do a file-copy using the pre-opened handle into /var/run/
>   (should work even with FAT, but would be slow).

I realize that we don't have to handle this at all.  The problem your
patch is supposed to handle are installation issues, which usually only
occur for installer-provided binaries, so only distro binaries should be
targeted.

> >> 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?
> 
> >> 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.
> 
> Ok then.
> 
> > Btw., I just skimmed your patch a bit and I stumbled over the usage of
> > GetFileInformationByHandle.
> 
> Is there something wrong with GetFileInformationByHandle I should know of?
> It can return FileId, LastWriteTime, and FileSize in one call...

Not as such, no.  It's just that Cygwin isn't using the Windows calls.
Under the hood the GetFileInformationByHandle is calling
NtQueryVolumeInformationFile(FileFsVolumeInformation) and
NtQueryInformationFile(FileAllInformation).

Sidenote:

We're not using FileAllInformation anymore because in my (very, very
long ago) testing, FileAllInformation needed a big buffer to store the
file's pathname.  However, I just tried it again while looking into
this, and it turns out that you can call FileAllInformation with a
buffer of size FILE_ALL_INFORMATION (which only has room for the first
character of the path).  It returns STATUS_BUFFER_OVERFLOW, but it also
returns all the data in the structure.  Yay.

I think I did some tests way back when, which showed that
FileAllInformation could be slower than three single calls
FileBasicInformation, FileStandardInformation and
FileInternalInformation combined.  But that was back in XP times.  If my
test results are outdated and FileAllInformation is quick enough, we
should contemplate to use FileAllInformation instead of
FileNetworkOpenInforation in class path_conv.  This may drop some
conditional code paths and generally simply the code a bit...

> > 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.
> 
> Actually the FileInformation queried in dll_list::ctimename is reused
> in dll_list::update_forkables_needs - have added a comment now.

Thanks.

> > Analogue in dll_list::update_forkables_needs.  Do you really need to
> > check the volume serial number?  Otherwise, just call
> > NtQueryInformationFile(FileInternalInformation).
> 
> The idea behind checking the volume serial number is that I'm unsure
> whether dlls can be replaced by symlinks to other drives as well, so
> the FileId probably could become equal for different files...

Symlinks can't be used for various reasons, not the least of them
the fact that non-admin users can't create them by default.

> BTW: MSDN document for FILE_INTERNAL_INFORMATION structure tells about
> IndexNumber as the member - and so does /usr/include/w32api/, but
> winsup/cygwin/ntdll.h does name it FileId - caused me some confusion...
> Is that intentionally?

It's historical.  FileId is the name of the field as used by Gary
Nebbett's book "Windows NT/2000 Native API Reference", heavily used when
MSDN had even less documentation of the NT API.  Unfortunately there was
never a followup book :(

I'd be not too sad to get patches changing this to the currently blessed
name...


Corinna


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

Attachment: pgpwDfoABjkhE.pgp
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]