Win32 stat() didn't handle AF_UNIX socket files #20271

tonycoz · 2022-09-08T08:34:31Z

Unfortunately both symbolic links and sockets can only be "statted" by opening with FILE_FLAG_OPEN_REPARSE_POINT which obviously doesn't follow symbolic links.

So to find if a chain of symbolic links points to a socket, is a broken chain, or loops, we need to follow the chain ourselves.

Fixes #20204

bram-perl · 2022-09-08T15:44:11Z

Just a small heads up: I'm planning on doing some testing with this and some reviewing;

(so if possible: wait with merging until I had a chance to test/review)

tonycoz · 2022-09-12T01:23:16Z

(so if possible: wait with merging until I had a chance to test/review)

That's no problem.

One test is failing, though I don't see why yet.

tonycoz · 2022-09-12T05:30:58Z

One test is failing, though I don't see why yet.

It succeeds sometimes, but mostly fails. I haven't managed to get it to fail locally, and DrMemory (like valgrind) isn't showing anything relevant that I can see.

tonycoz · 2022-09-13T22:47:17Z

It succeeds sometimes, but mostly fails.

Or it was, I can't get it to fail in CI at all now.

sisyphus · 2022-09-15T04:12:07Z

FWIW, this 20204-stat-socket branch is fine for me when I build it on Windows 11 using gcc-12, where _WIN32_WINNT is 0x0a00 - though some of the TODO tests fail, and the only archtype I've tested is MSWin32-x64-multi-thread.

With the same archtype and same compiler on Windows 7, t/win32/stat.t doesn't run to completion.
When run outside the Test::Harness, it terminates with:

....
ok 46 - filename '"' shouldn't exist
ok 47 # skip Cannot create unix socket
Label not found for "last SKIP" at ./test.pl line 535.

Something awry with the SKIP construction ?
It also fails some TODO tests.

Of course, there's a couple of questionable practices being undertaken on Windows 7:

AIUI, Windows 7 is technically not supported any more;
Using a compiler that has _WIN32_WINNT set to 0xa00 (Windows 10) on a Windows 7 machine seems a bit dubious.

But this has not been a problem before with gcc-12 builds of perl on Windows 7 - so long as optimization is set to -Os (which I think has just recently become the default optimization setting in blead's win32/GNUmakefile).

Cheers,
Rob

Unfortunately both symbolic links and sockets can only be "statted" by opening with FILE_FLAG_OPEN_REPARSE_POINT which obviously doesn't follow symbolic links. So to find if a chain of symbolic links points to a socket, is a broken chain, or loops, we need to follow the chain ourselves.

While debugging socket stat()ing I noticed that sometimes the name returned by win32_readlink() was a full pathname rather than the name that the link was created as. Changing this to use the PrintName values changed win32_readlink() to return the create as name, which seems closer to the POSIX readlink.

tonycoz · 2022-09-15T04:41:51Z

The failing TODO tests are for bugs that had been reported against Win32 stat() before I re-worked it, unfortunately they seem to be Windows limits (or bugs, I guess).

bram-perl · 2022-09-18T13:45:07Z

win32/win32.c

        CreateFileA(path, FILE_READ_ATTRIBUTES,
                    FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE,
                    NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL);
+    if (handle == INVALID_HANDLE_VALUE) {
+        /* AF_UNIX sockets need to be opened as a reparse point, but
+           that will also open symlinks rather than following them.
+
+           There may be other reparse points that need similar
+           treatment.
+        */
+        handle = S_follow_symlinks_to(aTHX_ path, &reparse_type);


~~This part of the commit doesn't make sense to me...~~

Outdated comment

Reading https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#symbolic-link-behavior

What I expect when path is a symlink:

if the FILE_FLAG_OPEN_REPARSE_POINT flag is not included then it points to the target of the symlink (i.e. to the destination, i.e. to the real file)

if the FILE_FLAG_OPEN_REPARSE_POINT flag is included then it points to th e symlink

In this code the FILE_FLAG_OPEN_REPARSE_POINT flag is not included in the CreateFileA call so I would expect handle to refer to the target file (i.e. not the symlink) but then the comment and code reads: S_follow_symlinks_to which doesn't make sense: if it was a symlink then handle already points to the destination file.. (assuming I read the MS docs correct)~

Update: Ok, it does make a bit of sense. It's only a bit confusing.

It first calls CreateFileA without FILE_FLAG_OPEN_REPARSE_POINT.
That either returns a valid handle (to the target file) or return an invalid handle.
The invalid handle can mean three different things:

there is no such file with that name

there is a file with that name but it's a symlink and points to a non-existing file

there is a file with that name but it's a unix socket

The code doesn't really check which case applies and (sort of) assumes it might be a symlink and calls S_follow_symlinks_to which calls readlink to follow the link (if it's a link I guess);
Eventually it calls CreateFileA again with the FILE_FLAG_OPEN_REPARSE_POINT flag set.

What I have/had initially in mind for this code was something like: (pseudo-code obviously):

handle = CreateFileA(path, FILE_READ_ATTRIBUTES, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINT, NULL); if (handle != INVALID_HANDLE_VALUE) { result = GetFileInformationByHandleEx(handle, FileBasicInfo, &basicInfo, ...) if (baiscInfo.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) { DeviceIoControl(handle, FSCTL_GET_REPARSE_POINT, ....) .... if (reparse_type == IO_REÄRSE_TAG_SYMLINK) { handle = CreateFileA(path, FILE_READ_ATTRIBUTES, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_FLAG_BACKUP_SEMANTICS, NULL); } } }

That is: first open with FILE_FLAG_OPEN_REPARSE_POINT and if it's a symlink then call CreateFileA again but this time without the FILE_FLAG_OPEN_REPARSE_POINT flag so that it resolves the symlink.

An alternative to the current code could also be to change S_follow_symlinks_to to not follow the symlink chain and instead check the reparse_type.
That is:

if CreateFileA(...., FILE_FLAG_BACKUP_SEMANTICS, ...) fails

and if CreateFileA(...., FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OPEN_REPARSE_POINTS, ...) succeeds

then check if reparse_type is 'symlink'. If it is return ENOENT

Thinking some more.. I guess that won't do the right thing if it's a symlink to a unix socket..
(I'll do some testing tomorrow)

I do the non-reparse-point open first because I expect it to be by far the more common case, and it does the right thing for win32_stat() for valid symlink chains.

I went direct for win32_readlink() mostly out of laziness - I didn't want to include the CreateFile/DeviceIOControl/WideCharToMultiByte() code here (though maybe a win32_readlink_handle() would help), and win32_readlink() does the job.

I don't really consider this code path to be performance critical.

And yes, a symlink chain to a socket fails the non-reparse CreateFile().

bram-perl · 2022-09-18T15:42:31Z

win32/win32.c

+    if (reparse_type) {
+        /* Lie to get to the right place */
+        type = FILE_TYPE_DISK;


In what case was this needed?
When I test with a unix socket it seems GetFileType() returns FILE_TYPE_DISK.
Did you encounter a case in which it returned something else?

This part may be unnecessary, I ass-umed that it would fail, but didn't test.

Thanks for the review.

demerphq

This passes the eyeball test for me. I dont have a win32 box to play with it on however. I need to fix that.

tonycoz added 2 commits September 15, 2022 14:35

tonycoz force-pushed the 20204-stat-socket branch from c735a93 to 33cac23 Compare September 15, 2022 05:37

bram-perl reviewed Sep 18, 2022

View reviewed changes

tonycoz mentioned this pull request Oct 31, 2022

readlink returns malformed path on Windows 11, perl 5.37.5 #20460

Closed

demerphq approved these changes Nov 1, 2022

View reviewed changes

tonycoz merged commit a47628b into Perl:blead Nov 1, 2022

hakonhagland mentioned this pull request Nov 2, 2022

lstat returns incorrect zero size for symlink on Windows 11 #20476

Closed

hakonhagland mentioned this pull request Nov 13, 2022

Forward slashes not recognized in relative symlink targets on Windows #20506

Closed

hakonhagland mentioned this pull request Nov 21, 2022

Windows: Absolute path in target of symlink directory, problem with -e and -d file tests #20533

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Win32 stat() didn't handle AF_UNIX socket files #20271

Win32 stat() didn't handle AF_UNIX socket files #20271

tonycoz commented Sep 8, 2022

bram-perl commented Sep 8, 2022

tonycoz commented Sep 12, 2022

tonycoz commented Sep 12, 2022

tonycoz commented Sep 13, 2022

sisyphus commented Sep 15, 2022

tonycoz commented Sep 15, 2022

bram-perl Sep 18, 2022 •

edited

Loading

bram-perl Sep 18, 2022

bram-perl Sep 18, 2022

bram-perl Sep 18, 2022

tonycoz Sep 18, 2022

bram-perl Sep 18, 2022

tonycoz Sep 18, 2022

demerphq left a comment

Win32 stat() didn't handle AF_UNIX socket files #20271

Win32 stat() didn't handle AF_UNIX socket files #20271

Conversation

tonycoz commented Sep 8, 2022

bram-perl commented Sep 8, 2022

tonycoz commented Sep 12, 2022

tonycoz commented Sep 12, 2022

tonycoz commented Sep 13, 2022

sisyphus commented Sep 15, 2022

tonycoz commented Sep 15, 2022

bram-perl Sep 18, 2022 • edited Loading

Choose a reason for hiding this comment

bram-perl Sep 18, 2022

Choose a reason for hiding this comment

bram-perl Sep 18, 2022

Choose a reason for hiding this comment

bram-perl Sep 18, 2022

Choose a reason for hiding this comment

tonycoz Sep 18, 2022

Choose a reason for hiding this comment

bram-perl Sep 18, 2022

Choose a reason for hiding this comment

tonycoz Sep 18, 2022

Choose a reason for hiding this comment

demerphq left a comment

Choose a reason for hiding this comment

bram-perl Sep 18, 2022 •

edited

Loading