Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BSOD caused when copying to an alternative data stream #24

Open
hamarb123 opened this issue Jun 21, 2023 · 15 comments
Open

BSOD caused when copying to an alternative data stream #24

hamarb123 opened this issue Jun 21, 2023 · 15 comments

Comments

@hamarb123
Copy link

hamarb123 commented Jun 21, 2023

When attempting to copy onto an alternative data stream, a BSOD is caused.

Simple reproduction (F:\ is a Dev Drive):

File.WriteAllText($@"F:\New Text Document.txt", "a");
File.WriteAllText($@"F:\New Text Document 2.txt", "b");
File.WriteAllText($@"F:\New Text Document 2.txt:x", "c");
Microsoft.CopyOnWrite.CopyOnWriteFilesystemFactory.GetInstance().CloneFile($@"F:\New Text Document.txt", $@"F:\New Text Document 2.txt:x");

I discovered this while trying to implement dotnet/runtime#86681, can you please raise this internally so we can try to get it quickly fixed in Windows, after verifying it on your end. For now, I will add a workaround to skip for ADS and revert all of my testing changes on my local machine, so I can hopefully get a PR in a workable state soon.

Note, that my local version doesn't have the issue where it may get confused about which volume it's on by a path like this, it should definitely know it's on F:\ as it uses Windows APIs to correctly determine the volume. I'm happy to triple check this if you'd like, but a BSOD should obviously not be caused regardless.

Windows version:
Edition: Windows 11 Pro Insider Preview
Version: 22H2
OS build: 23481.1000
Experience: Windows Feature Experience Pack 1000.23481.1000.0

Image of BSOD if you want it for some reason (which I have because it's in a VM):
image

@erikmav
Copy link
Contributor

erikmav commented Jun 22, 2023

Issue submitted to Win Filesystems team. A hotfix that returns Not Supported will go into the Dev Drive (23H2 prerelease) channel as well as backported to earlier versions of Windows (as this occurs on ReFS on Win11 22H2 as well). More complete details to follow. There is a proposal to enable this functionality in a future version of Windows but there is no committed milestone.

Ideally you should handle this at the .NET code level by detecting alt stream usage and throwing a NotSupportedException or switching to a regular Copy command, at least until the eventual feature is available on the user's OS version. The hotfix will effectively provide the same result by returning a winerror or hresult of the same value from a lower layer.

@hamarb123
Copy link
Author

This will be handled by falling back to regular copy. I'm already planning to basically discard most errors and fall back in the event of them (since many of them could be caused by an infinite list of reasons), after checking it works correctly in an environment using ReFS.

Will you be able to let me/someone know when if and when it's actually properly implemented, so that the workaround can be removed for Windows versions that meet the minimum version?

Will implement a workaround which will check the source and destination filenames, and check if they have a colon, do you know if this is definitely a correct check to perform, which would catch every ADS? Another important aspect would be if you can have a symlink to an ADS, in which case I would need to deal with that also for the destination file. If you know of an API which tells you whether an open file is an ADS, that would work too, and be the easiest I think.

Thanks!

@erikmav
Copy link
Contributor

erikmav commented Jun 23, 2023

The colon in the filename (as opposed to the C:\ beginning) is the right way to detect, per the FS team. As part of validating their commit I'll be adding tests to the CopyOnWrite repo and cross-link the PR here.

@erikmav
Copy link
Contributor

erikmav commented Jun 27, 2023

Verified that the linked PR code works on an internal early 23H2 image containing the hotfix that returned ERROR_NOT_SUPPORTED instead of bluescreening. The hotfix should be available within 1-2 Patch Tuesdays. I would bet real support for this scenario will be at least a year away but we'll see.

I'll commit the PR code when GitHub Actions' Server2022 VM successfully runs without bluescreening.

@hamarb123
Copy link
Author

It seems like if I do this:

File.WriteAllText(@"C:\Users\Hamish\Desktop\test.txt", "");
File.WriteAllText(@"C:\Users\Hamish\Desktop\test.txt:link", "");
File.CreateSymbolicLink(@"C:\Users\Hamish\Desktop\test2.txt", @"C:\Users\Hamish\Desktop\test.txt:link");

I can make a symlink to an ADS, which looks like a normal file (some programs struggle with it, but I could edit in notepad++ for example). This is a problem since .NET File.Copy copies onto symlink's destination, hence we need a way to ensure it's not an ADS, do you know of a cheap way to check this? I can think of using FILE_FLAG_OPEN_REPARSE_POINT when opening and then checking FILE_ATTRIBUTE_REPARSE_POINT (which I'm reading to check sparseness anyway), and falling back to a slower path for links.

@erikmav
Copy link
Contributor

erikmav commented Jun 28, 2023

Asking about this case, I'm stumped. Back when I have any info to share.

@erikmav
Copy link
Contributor

erikmav commented Jun 28, 2023

There are few docs but take a look at AlternateDataStream in FILE_STANDARD_INFORMATION_EX. On ReFS this will be set TRUE for ADS. For non-ReFS it varies so you'll need to check this only after ensuring it's a ReFS/Dev Drive volume. Docs: https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/wdm/ns-wdm-_file_standard_information_ex

@hamarb123
Copy link
Author

hamarb123 commented Jun 28, 2023

What API would I use to get FILE_STANDARD_INFORMATION_EX, I can't seem to find any to that reference it in the docs elsewhere. I'm currently using https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-getfileinformationbyhandleex, and it doesn't seem to mention it. If it's that I just use FileStandardInfo, with the different structure and size, this would still need to be documented on the docs to be counted as a non-private API, since .NET doesn't use APIs which aren't documented is my understanding.

Also, you say I need to check if it's on ReFS first, but I'm already checking FILE_SUPPORTS_BLOCK_REFCOUNTING anyway, so do I actually need to check if it's ReFS, or will it work properly on everything?

Once a specific version of Windows is known to not have the BSOD crash, we may skip the check on that version and higher, since there does not seem to be a particularly cheap way to check it (it seems to cost at least 1 syscall, which is not ideal); this is because there is not much point slowing down the general case to support the extremely rare case of copying to an ADS on ReFS. Will measure this when up to that stage to see if it makes a worthwhile difference though.

On ReFS this will be set TRUE for ADS. For non-ReFS it varies so you'll need to check this only after ensuring it's a ReFS/Dev Drive volume.

Also, it would be good if it documented with AlternateStream that it's not always true on every filesystem if this is the case.

@MichalPetryka
Copy link

What API would I use to get FILE_STANDARD_INFORMATION_EX, I can't seem to find any to that reference it in the docs elsewhere.

NtQueryInformationFile

@hamarb123
Copy link
Author

@MichalPetryka NtQueryInformationFile also doesn't seem to document any reference to FILE_STANDARD_INFORMATION_EX. Am I missing something obvious?

@MichalPetryka
Copy link

@MichalPetryka NtQueryInformationFile also doesn't seem to document any reference to FILE_STANDARD_INFORMATION_EX. Am I missing something obvious?

I'd assume it's just FileStandardInformation there but you pass in the EX, not the normal one?

@hamarb123
Copy link
Author

I'd assume it's just FileStandardInformation there but you pass in the EX, not the normal one?

If this is the case, it still probably needs to be in the docs, otherwise it probably counts as undocumented. And it seems to me like there is no reason that GetFileInformationByHandleEx wouldn't work if that's the case (unless it has some other issue).

@hamarb123
Copy link
Author

The reason I bring up the documentation is based on this conversation: dotnet/runtime#79243 (comment)

@danmoseley
Copy link
Member

The hotfix should be available within 1-2 Patch Tuesdays. I would bet real support for this scenario will be at least a year away but we'll see.

@erikmav if there isn't a bulletproof certain way to know that calling the API won't bluescreen, we I'd be uncomfortable exposing it at all in .NET, which would be sad for everyone built on the ecosystem, including Powershell, MSBuild etc. Is there an API we could call to determine whether the machine is patched for this specifically? Maybe it's something like "is OS version 11.1234 or later, or 10 with patch ABC123, or 11 with patch DEF234". Would you be able to ask internally?

@danmoseley
Copy link
Member

If this is the case, it still probably needs to be in the docs, otherwise it probably counts as undocumented. And it seems to me like there is no reason that GetFileInformationByHandleEx wouldn't work if that's the case (unless it has some other issue).

It looks like FindFirstStreamW is documented.
https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-findfirststreamw
https://learn.microsoft.com/en-us/windows/win32/fileio/file-streams
That only reads the $DATA streams -- is that sufficient?

Perhaps you can rip off code from Powershell --
https://github.com/powershell/powershell/blob/30098c744adb52ea14f29f03897487fecc1dc4f0/src/System.Management.Automation/namespaces/FileSystemProvider.cs#L8098-L8162

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants