Re: [PATCH] Compatibility improvement to reparse point handling, v2

[Joe Lowe <joe at pismotec dot com>]

Corinna, 

I will work through the code and apply your feedback. A few
additional comments below.

On 2017-06-12 03:27, Corinna Vinschen wrote:
> On Jun  9 15:44, Joe Lowe wrote:
> > 2nd pass at reparse point handling patch.
> > [...]
> >   static inline int
> >   readdir_check_reparse_point (POBJECT_ATTRIBUTES attr)
> >   {
> > -  DWORD ret = DT_UNKNOWN;
> > +  int ret = DT_UNKNOWN;
>
> Given that d_type is an unsigned char type, maybe it would be better to
> change the return type of the function (and `ret') accordingly instead.

The current readdir() behavior in Cygwin for mount points is to
return the INO of the target volume root directory. This behavior
does not match Linux or MacOS. I verified with some test code,
attached, output below.

# Cygwin
joe@dev ~/code/t $ ./a.exe /cygdrive/c/Volumes
# ...
scratch
 d_type             = DT_DIR (4)
 d_ino              = 0x51F2BEF5F00CEBB0
 lstat dev/rdev/ino = 0xBC7144D6 / 0x0 / 0x51F2BEF5F00CEBB0
 stat  dev/rdev/ino = 0xBC7144D6 / 0x0 / 0x51F2BEF5F00CEBB0

# Linux
joe@omega:~/code/t $ ./a.out /media
# ...
scratch
 d_type             = DT_DIR (4)
 d_ino              = 0x480099
 lstat dev/rdev/ino = 0x2A / 0x0 / 0x2
 stat  dev/rdev/ino = 0x2A / 0x0 / 0x2

# MacOS
joe@macdev:~/code/t $ ./a.out /Volumes
# ...
scratch
 d_type             = DT_DIR (4)
 d_ino              = 0xA6F3C8
 lstat dev/rdev/ino = 0x2D000003 / 0x0 / 0x2
 stat  dev/rdev/ino = 0x2D000003 / 0x0 / 0x2

Unless there is some known case where Cygwin needs to behave
differently here than Linux and MacOS, I would like to change
readdir() to always return the INO of the reparse point.

> > @@ -2944,22 +3017,25 @@ restart:
> >   		&= ~FILE_ATTRIBUTE_DIRECTORY;
> >   	      break;
> >   	    }
> > -	  else
> > +	  else if (res == -1)
> >   	    {
> > -	      /* Volume moint point or unrecognized reparse point type.
> > +	      /* Volume moint point or unhandled reparse point.
> >   		 Make sure the open handle is not used in later stat calls.
> >   		 The handle has been opened with the FILE_OPEN_REPARSE_POINT
> >   		 flag, so it's a handle to the reparse point, not a handle
> > -		 to the volumes root dir. */
> > +		 to the reparse point target. */
> >   	      pflags &= ~PC_KEEP_HANDLE;
> > -	      /* Volume mount point:  The filesystem information for the top
> > -		 level directory should be for the volume top level directory,
> > -		 rather than for the reparse point itself.  So we fetch the
> > -		 filesystem information again, but with a NULL handle.
> > -		 This does what we want because fs_info::update opens the
> > -		 handle without FILE_OPEN_REPARSE_POINT. */
> > -	      if (res == -1)
> > -		fs.update (&upath, NULL);
> > +	      /* The filesystem information should be for the target of
> > +		 the reparse point rather than for the reparse point itself.
> > +		 So we fetch the filesystem information again, but with a
> > +		 NULL handle. This does what we want because fs_info::update
> > +		 opens the handle without FILE_OPEN_REPARSE_POINT. */
> > +	      fs.update (&upath, NULL);
> > +	    }
> > +	  else
> > +	    {
> > +	      /* Unknown reparse point type: HSM, dedup, compression, ...
> > +	         Treat as normal directory. */
> >   	    }
>
> Nothing against reordering the code, but this drops removing
> PC_KEEP_HANDLE from pflags if res == 0, i.e., for unknown reparse
> points.

Changing the handling for unknown (3rd party) reparse tags is
not the primary point of the patch. But the fallback handling
for reparse tags should be to do what most win32 apps would do,
treat as a normal file/directory and open without the
FILE_OPEN_REPARSE_TAG flag.

If an application opens a file/directory without
FILE_OPEN_REPARSE_TAG that is a reparse point, the related
3rd party file system filter can do its job and make things
work. Example, a transparent file compression system extension
would decompress the file and remove the reparse point when
the file is opened for data access. A cloud storage file
system would retrieve the file data from a remote server and
then remove the reparse point.

In practice, I suspect that most 3rd party system extensions
that utilize custom reparse tags resort to hiding their reparse
point data from user mode entirely.


Joe L.

Attachment: test.c
Description: Text document