[release/3.1] Fix Windows AccessViolationException in FileSystemWatcher when monitoring network share for changes

[carlossanlop]

Customer impact

This issue was reported for 3.1 twice:

• w3wp.exe crash in FileSystemWatcher after 2.2 -> 3.1 migration (c0000005 Access Violation) runtime#40412 - w3wp.exe crash in FileSystemWatcher after 2.2 -> 3.1 migration (c0000005 Access Violation)
• FileSystemWatcher crash UNALIGNED_STACK_POINTER runtime#42108 - FileSystemWatcher crash UNALIGNED_STACK_POINTER

When monitoring a network share with FileSystemWatcher, if there are network issues that could cause the data to arrive malformed, then after casting the byte array to a FILE_NOTIFY_INFORMATION, attempts to access the struct fields can throw an AV.

The 6.0 (master) PR: dotnet/runtime#42419
The 5.0 backport PR: dotnet/runtime#42420

Testing

The issue was also reported internally by one of the customers. Our CSS partner was able to reproduce it locally.

I provided him a private binary of System.IO.FileSystem.Watcher.dll with the changes in this fix, and he confirmed the AV no longer happens.

Risk

Low. We are introducing boundary checks after casting data data coming from a P/Invoke, with the explicit purpose of removing an unexpected AV exception.

[carlossanlop]

I have address all the suggestions received in the master PR dotnet/runtime#42419
I'll update this PR after I merge the master one, to avoid unnecessary CI jobs.

[carlossanlop]

CI failures:

• arm64 release - Unrelated to this change:

Failure in one System.Runtime.Caching test

  C:\dotnetbuild\work\B5F209D8\w\AD6A0948\e>"C:\dotnetbuild\work\B5F209D8\p\dotnet.exe" exec --runtimeconfig System.Runtime.Caching.Tests.runtimeconfig.json xunit.console.dll System.Runtime.Caching.Tests.dll -xml testResults.xml -nologo -nocolor -notrait category=nonnetcoreapptests -notrait category=nonwindowstests -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing  
  Discovering: System.Runtime.Caching.Tests (method display = ClassAndMethod, method display options = None)
  Discovered:  System.Runtime.Caching.Tests (found 30 of 40 test cases)
  Starting:    System.Runtime.Caching.Tests (parallel test collections = on, max threads = 46)
      MonoTests.System.Runtime.Caching.MemoryCacheTest.Trim [FAIL]
      Assert.Equal() Failure
      Expected: 23
      Actual:   22
      Stack Trace:
          /_/src/System.Runtime.Caching/tests/System.Runtime.Caching/MemoryCacheTest.cs(991,0): at MonoTests.System.Runtime.Caching.MemoryCacheTest.Trim()
  Finished:    System.Runtime.Caching.Tests
  === TEST EXECUTION SUMMARY ===
  System.Runtime.Caching.Tests  Total: 30, Errors: 0, Failed: 1, Skipped: 0, Time: 4.652s
  ----- end Fri 09/18/2020 12:48:45.09 ----- exit code 1 ----------------------------------------------------------

[jeffhandley]

This was approved in email on 2020-09-17 and PR feedback has been addressed. The failing check and the remaining check timeouts are unrelated.

[Rayzbam]

@jeffhandley, @carlossanlop Hi Jeff, Carlos, Do you know if it will be in the .Net Core 3.1.9 release that "should" be out in October ?

[jeffhandley]

@Rayzbam Yes, this fix should be included in the October release.

[danmoseley]

@Rayzbam perhaps you could post back here in Oct to confirm it works for you? So we can be sure we are done here.

[Rayzbam]

@danmosemsft Sure.
Actually, i was in contact with MS Support for this issue. They provided me the targeted assembly just to try my case. I can confirm that THIS fix fixed it.
I'm just waiting the official fix to deploy it on our Production servers.
I'll let you know if it's ok as soon as I upgraded the .Net Core version on these servers !