forkables: About hardlink creation and NTFS transaction in rename()

forkables: About hardlink creation and NTFS transaction in rename()

[Michael Haubenwallner]

Hi Corinna,

now working with the cygfork patches (hardlinks to retain forkability
beyond exe/dll update): when creating the forkable hardlink using the
earlier opened file handle I may get STATUS_TRANSACTION_NOT_ACTIVE.

It turns out that when loaded 'some.dll' was readonly, then
rename("new/some.dll", "some.dll") uses an NTFS-transaction to drop
the readonly attribute, breaking the subsequent hardlink creation
of "/var/run/cygfork/.../soname.dll" via the original file handle.

Now I can see these options:
* As using NTFS transactions seems not to be recommended any more,
  might we drop the NTFS transaction from rename() and _unlink_nt ()?
* Or do I need to create an NTFS-transaction in dll_list::alloc()
  when opening the original dll file handle?

Thoughts?

Thanks!
/haubi/

Re: forkables: About hardlink creation and NTFS transaction in rename()

[Corinna Vinschen]

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

On Nov 21 16:19, Michael Haubenwallner wrote:
> Hi Corinna,
> 
> now working with the cygfork patches (hardlinks to retain forkability
> beyond exe/dll update): when creating the forkable hardlink using the
> earlier opened file handle I may get STATUS_TRANSACTION_NOT_ACTIVE.
> 
> It turns out that when loaded 'some.dll' was readonly, then
> rename("new/some.dll", "some.dll") uses an NTFS-transaction to drop
> the readonly attribute, breaking the subsequent hardlink creation
> of "/var/run/cygfork/.../soname.dll" via the original file handle.

Do not use the DOS R/O attribute on non-FAT in a POSIX context.  Only
Cygwin symlinks of the Windows shortcut type should do that, and then
only for historical reasons.  And don't get me started how bad an idea
Windows shortcut symlinks were in the first place...


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 --]

Re: forkables: About hardlink creation and NTFS transaction in rename()

[Michael Haubenwallner]

On 11/22/2016 06:22 PM, Corinna Vinschen wrote:
> On Nov 21 16:19, Michael Haubenwallner wrote:
>> Hi Corinna,
>>
>> now working with the cygfork patches (hardlinks to retain forkability
>> beyond exe/dll update): when creating the forkable hardlink using the
>> earlier opened file handle I may get STATUS_TRANSACTION_NOT_ACTIVE.
>>
>> It turns out that when loaded 'some.dll' was readonly, then
>> rename("new/some.dll", "some.dll") uses an NTFS-transaction to drop
>> the readonly attribute, breaking the subsequent hardlink creation
>> of "/var/run/cygfork/.../soname.dll" via the original file handle.
> 
> Do not use the DOS R/O attribute on non-FAT in a POSIX context.  Only
> Cygwin symlinks of the Windows shortcut type should do that, and then
> only for historical reasons.  And don't get me started how bad an idea
> Windows shortcut symlinks were in the first place...

While I have not checked yet why the FILE_ATTRIBUTE_READONLY is set:
Is there a difference between the "DOS R/O attribute" and the pc.attribute
FILE_ATTRIBUTE_READONLY?

Combined with FILE_SUPPORTS_TRANSACTIONS this triggers start_transaction()
in rename() in syscalls.cc, breaking the previously opened file handle.

And there are no symlinks involved - I'm talking about hardlinks here.

What am I missing?
/haubi/

Re: forkables: About hardlink creation and NTFS transaction in rename()

[Corinna Vinschen]

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

On Nov 23 10:08, Michael Haubenwallner wrote:
> On 11/22/2016 06:22 PM, Corinna Vinschen wrote:
> > On Nov 21 16:19, Michael Haubenwallner wrote:
> >> Hi Corinna,
> >>
> >> now working with the cygfork patches (hardlinks to retain forkability
> >> beyond exe/dll update): when creating the forkable hardlink using the
> >> earlier opened file handle I may get STATUS_TRANSACTION_NOT_ACTIVE.
> >>
> >> It turns out that when loaded 'some.dll' was readonly, then
> >> rename("new/some.dll", "some.dll") uses an NTFS-transaction to drop
> >> the readonly attribute, breaking the subsequent hardlink creation
> >> of "/var/run/cygfork/.../soname.dll" via the original file handle.
> > 
> > Do not use the DOS R/O attribute on non-FAT in a POSIX context.  Only
> > Cygwin symlinks of the Windows shortcut type should do that, and then
> > only for historical reasons.  And don't get me started how bad an idea
> > Windows shortcut symlinks were in the first place...
> 
> While I have not checked yet why the FILE_ATTRIBUTE_READONLY is set:
> Is there a difference between the "DOS R/O attribute" and the pc.attribute
> FILE_ATTRIBUTE_READONLY?

No, it's one and the same.

> Combined with FILE_SUPPORTS_TRANSACTIONS this triggers start_transaction()
> in rename() in syscalls.cc, breaking the previously opened file handle.
> 
> And there are no symlinks involved - I'm talking about hardlinks here.

I know.  The question is why was FILE_ATTRIBUTE_READONLY set at all?
There's no good reason for that, unless you're on a "noacl" mount and
the file permissions are set to R/O for the supposed owner.  In theory
those files should be left alone anyway.


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 --]

Re: forkables: About hardlink creation and NTFS transaction in rename()

[Michael Haubenwallner]

On 11/21/2016 04:19 PM, Michael Haubenwallner wrote:
> Hi Corinna,
> 
> now working with the cygfork patches (hardlinks to retain forkability
> beyond exe/dll update): when creating the forkable hardlink using the
> earlier opened file handle I may get STATUS_TRANSACTION_NOT_ACTIVE.
> 
> It turns out that when loaded 'some.dll' was readonly, then
> rename("new/some.dll", "some.dll") uses an NTFS-transaction to drop
> the readonly attribute, breaking the subsequent hardlink creation
> of "/var/run/cygfork/.../soname.dll" via the original file handle.

I've been wrong here about the readonly flag:
The transaction is started in rename() where the initial
NtSetInformationFile (Rename) returns STATUS_ACCESS_DENIED,
simply because the target "some.dll" is in use.

Not starting that transaction keeps the previously opened file handle
intact to subsequently create a hardlink.  But the transaction makes
sense - so what can I do for the original file handle?

> Now I can see these options:
> * As using NTFS transactions seems not to be recommended any more,
>   might we drop the NTFS transaction from rename() and _unlink_nt ()?
> * Or do I need to create an NTFS-transaction in dll_list::alloc()
>   when opening the original dll file handle?
> 
> Thoughts?

Thanks!
/haubi/

Re: forkables: About hardlink creation and NTFS transaction in rename()

[Corinna Vinschen]

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

On Nov 25 15:03, Michael Haubenwallner wrote:
> On 11/21/2016 04:19 PM, Michael Haubenwallner wrote:
> > Hi Corinna,
> > 
> > now working with the cygfork patches (hardlinks to retain forkability
> > beyond exe/dll update): when creating the forkable hardlink using the
> > earlier opened file handle I may get STATUS_TRANSACTION_NOT_ACTIVE.
> > 
> > It turns out that when loaded 'some.dll' was readonly, then
> > rename("new/some.dll", "some.dll") uses an NTFS-transaction to drop
> > the readonly attribute, breaking the subsequent hardlink creation
> > of "/var/run/cygfork/.../soname.dll" via the original file handle.
> 
> I've been wrong here about the readonly flag:
> The transaction is started in rename() where the initial
> NtSetInformationFile (Rename) returns STATUS_ACCESS_DENIED,
> simply because the target "some.dll" is in use.

No, wait.  If the DLL is in use, the return code should be
STATUS_SHARING_VIOLATION.


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 --]

Re: forkables: About hardlink creation and NTFS transaction in rename()

[Michael Haubenwallner]

On 11/28/2016 10:47 AM, Corinna Vinschen wrote:
> On Nov 25 15:03, Michael Haubenwallner wrote:
>> On 11/21/2016 04:19 PM, Michael Haubenwallner wrote:
>>> Hi Corinna,
>>>
>>> now working with the cygfork patches (hardlinks to retain forkability
>>> beyond exe/dll update): when creating the forkable hardlink using the
>>> earlier opened file handle I may get STATUS_TRANSACTION_NOT_ACTIVE.
>>>
>>> It turns out that when loaded 'some.dll' was readonly, then
>>> rename("new/some.dll", "some.dll") uses an NTFS-transaction to drop
>>> the readonly attribute, breaking the subsequent hardlink creation
>>> of "/var/run/cygfork/.../soname.dll" via the original file handle.
>>
>> I've been wrong here about the readonly flag:
>> The transaction is started in rename() where the initial
>> NtSetInformationFile (Rename) returns STATUS_ACCESS_DENIED,
>> simply because the target "some.dll" is in use.
> 
> No, wait.  If the DLL is in use, the return code should be
> STATUS_SHARING_VIOLATION.

Hmm, no:
In rename() the first try for NtSetInformationFile(FileRenameInformation)
returns STATUS_ACCESS_DENIED.
Also, rename() won't handle STATUS_SHARING_VIOLATION right now.

It is unlink_nt() that would handle STATUS_SHARING_VIOLATION, but
NtOpenFile(FILE_SHARE_DELETE) succeeds even for a dll in use,
although a dll cannot be used while open with FILE_SHARE_DELETE.

Anyway: Looking at the /proc/pid/maps handler I've learned that
NtQueryVirtualMemory(hmodule, MemorySectionName) yields the current
dll's filename (as \Device\HarddiskVolume...) - even after renaming,
at least on Server 2012r2.

So for creating the hardlink I could open the current MemorySectionName,
rather than early open the GetModuleFileNameW(), so I won't be affected
by rename's transaction at all.

But what do you think about NtQueryVirtualMemory(MemorySectionName) here?

/haubi/

Re: forkables: About hardlink creation and NTFS transaction in rename()

[Michael Haubenwallner]

On 11/21/2016 04:19 PM, Michael Haubenwallner wrote:
> Hi Corinna,
> 
> now working with the cygfork patches (hardlinks to retain forkability
> beyond exe/dll update): when creating the forkable hardlink using the
> earlier opened file handle I may get STATUS_TRANSACTION_NOT_ACTIVE.
> 
> It turns out that when loaded 'some.dll' was readonly, then
> rename("new/some.dll", "some.dll") uses an NTFS-transaction to drop
> the readonly attribute, breaking the subsequent hardlink creation
> of "/var/run/cygfork/.../soname.dll" via the original file handle.

It turns out that when NTFS transactions are involved with the dll,
creating a hardlink requires the FILE_WRITE_ATTRIBUTES access.

Strange enough, this also applies when the file handle used to create
the hardlink is opened _after_ the rename transaction has finished.

On the other hand, opening the file handle with FILE_WRITE_ATTRIBUTES
access _before_ the rename(), the transactional unlink_nt fails with
STATUS_TRANSACTIONAL_CONFLICT.

> Now I can see these options:
> * As using NTFS transactions seems not to be recommended any more,
>   might we drop the NTFS transaction from rename() and _unlink_nt ()?
> * Or do I need to create an NTFS-transaction in dll_list::alloc()
>   when opening the original dll file handle?

some more now:

* Just for creating the hardlink open the file by id, determined
  in dll_list::alloc(), with immediately closing the file handle.
  However, I've not managed yet to open a file by id...

* When checking for, hardlinks are needed when the MemorySectionName
  resolves different than the (Get)ModuleFileName, and just for
  creating the hardlink open the file by current MemorySectionName.

> Thoughts?

Thanks!
/haubi/

Re: forkables: About hardlink creation and NTFS transaction in rename()

[Corinna Vinschen]

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

On Nov 29 10:47, Michael Haubenwallner wrote:
> On 11/28/2016 10:47 AM, Corinna Vinschen wrote:
> > On Nov 25 15:03, Michael Haubenwallner wrote:
> >> On 11/21/2016 04:19 PM, Michael Haubenwallner wrote:
> >>> Hi Corinna,
> >>>
> >>> now working with the cygfork patches (hardlinks to retain forkability
> >>> beyond exe/dll update): when creating the forkable hardlink using the
> >>> earlier opened file handle I may get STATUS_TRANSACTION_NOT_ACTIVE.
> >>>
> >>> It turns out that when loaded 'some.dll' was readonly, then
> >>> rename("new/some.dll", "some.dll") uses an NTFS-transaction to drop
> >>> the readonly attribute, breaking the subsequent hardlink creation
> >>> of "/var/run/cygfork/.../soname.dll" via the original file handle.
> >>
> >> I've been wrong here about the readonly flag:
> >> The transaction is started in rename() where the initial
> >> NtSetInformationFile (Rename) returns STATUS_ACCESS_DENIED,
> >> simply because the target "some.dll" is in use.
> > 
> > No, wait.  If the DLL is in use, the return code should be
> > STATUS_SHARING_VIOLATION.
> 
> Hmm, no:
> In rename() the first try for NtSetInformationFile(FileRenameInformation)
> returns STATUS_ACCESS_DENIED.
> Also, rename() won't handle STATUS_SHARING_VIOLATION right now.

It does, just not in terms of transactions.  STATUS_SHARING_VIOLATION is
not supposed to start a transaction.

> NtOpenFile(FILE_SHARE_DELETE) succeeds even for a dll in use,
> although a dll cannot be used while open with FILE_SHARE_DELETE.

That's unfortunate.

> Anyway: Looking at the /proc/pid/maps handler I've learned that
> NtQueryVirtualMemory(hmodule, MemorySectionName) yields the current
> dll's filename (as \Device\HarddiskVolume...) - even after renaming,
> at least on Server 2012r2.
> 
> So for creating the hardlink I could open the current MemorySectionName,
> rather than early open the GetModuleFileNameW(), so I won't be affected
> by rename's transaction at all.
> 
> But what do you think about NtQueryVirtualMemory(MemorySectionName) here?

Well, if it does its job...


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 --]

Re: forkables: About hardlink creation and NTFS transaction in rename()

[Corinna Vinschen]

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

On Nov 29 14:47, Michael Haubenwallner wrote:
> On 11/21/2016 04:19 PM, Michael Haubenwallner wrote:
> > Hi Corinna,
> > 
> > now working with the cygfork patches (hardlinks to retain forkability
> > beyond exe/dll update): when creating the forkable hardlink using the
> > earlier opened file handle I may get STATUS_TRANSACTION_NOT_ACTIVE.
> > 
> > It turns out that when loaded 'some.dll' was readonly, then
> > rename("new/some.dll", "some.dll") uses an NTFS-transaction to drop
> > the readonly attribute, breaking the subsequent hardlink creation
> > of "/var/run/cygfork/.../soname.dll" via the original file handle.
> 
> It turns out that when NTFS transactions are involved with the dll,
> creating a hardlink requires the FILE_WRITE_ATTRIBUTES access.
> 
> Strange enough, this also applies when the file handle used to create
> the hardlink is opened _after_ the rename transaction has finished.
> 
> On the other hand, opening the file handle with FILE_WRITE_ATTRIBUTES
> access _before_ the rename(), the transactional unlink_nt fails with
> STATUS_TRANSACTIONAL_CONFLICT.

I wonder how Microsoft implemented NTFS transactions.  Something
seems a little bit off here.

> > Now I can see these options:
> > * As using NTFS transactions seems not to be recommended any more,
> >   might we drop the NTFS transaction from rename() and _unlink_nt ()?
> > * Or do I need to create an NTFS-transaction in dll_list::alloc()
> >   when opening the original dll file handle?
> 
> some more now:
> 
> * Just for creating the hardlink open the file by id, determined
>   in dll_list::alloc(), with immediately closing the file handle.
>   However, I've not managed yet to open a file by id...

  You need a handle to some file on the same file system to identify
  the file system.  Let's say the handle is called vol_hdl.  Then:

  path_conv pc ("my.dll");
  OBJECT_ATTRIBUTES attr;
  IO_STATUS_BLOCK io;
  NTSTATUS status;

  LARGE_INTEGER &file_id = pc.fai ()->InternalInformation.IndexNumber;
  UNICODE_STRING ufid = { sizeof file_id, sizeof file_id, (PWSTR) &file_id };
  InitializeObjectAttributes (&attr, &ufid, pc.objcaseinsensitive (),
			      vol_hdl, NULL);
  status = NtOpenFile (&hdl, access, &attr, &io, share_flags, options);


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 --]