Use `GetTempPath2W` if available. #72452

teo-tsirpanis · 2022-07-19T13:30:26Z

Since Windows 11 there is a new API to get the temporary files path called GetTempPath2. This API returns a directory inaccessible to non-SYSTEM processes if the calling process runs as SYSTEM, and it is recommended to call this function instead of GetTempPath.

This PR tries to find GetTempPath2W and uses that, otherwise it falls back to GetTempPathW.

I also fixed a bug where the original GetTempPathW P/Invoke was not marked with SetLastError and would fail with Operation Completed Successfuly in case of a failure.

ghost · 2022-07-19T13:30:38Z

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Since Windows 11 there is a new API to get the temporary files path called GetTempPath2. This API returns a directory inaccessible to non-SYSTEM processes if the calling process runs as SYSTEM, and it is recommended to call this function instead of GetTempPath.

This PR tries to find GetTempPath2W and uses that, otherwise it falls back to GetTempPath.

I also fixed a bug where the original GetTempPathW P/Invoke was not marked with SetLastError and would fail with Operation Completed Successfuly in case of a failure.

Author:	teo-tsirpanis
Assignees:	-
Labels:	`area-System.IO`
Milestone:	-

jkotas · 2022-07-19T15:00:25Z

Does this behavior change need to be treated as a breaking change?

src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs

…ure`.

src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs

danmoseley · 2022-07-19T20:10:42Z

Does this behavior change need to be treated as a breaking change?

This would be something like -- I wrote a file to temp, upgrade my .NET or OS, and try to read it? That seems unlikely to be something we want to guarantee. In general it is good to not assume that a reboot will preserve temp.

teo-tsirpanis · 2022-07-19T20:17:16Z

And the path would change only if the program runs as SYSTEM. Most user apps are unaffected.

danmoseley · 2022-07-19T20:23:18Z

Have people tried running .NET code as SYSTEM?

GrabYourPitchforks · 2022-07-19T22:35:59Z

It's not uncommon for Windows services to run as SYSTEM. It's also possible that GetTempPath2W uses the current impersonation token instead of the process's own token, which could means the answer could change while the application is running. But I don't have a good way of testing this right now.

src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs

…s.cs Co-authored-by: Stephen Toub <stoub@microsoft.com>

src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs

…s.cs Co-authored-by: David Cantú <dacantu@microsoft.com>

danmoseley · 2022-08-10T21:48:37Z

GetGCMemoryInfo failure is already resolved.

danmoseley · 2022-08-10T21:50:25Z

@teo-tsirpanis could you please resolve the conflict so we can merge?

teo-tsirpanis · 2022-08-10T21:52:26Z

Conflicts were resolved.

danmoseley · 2022-08-11T02:10:22Z

Failure is #73679

danmoseley · 2022-08-11T02:14:15Z

...ries/System.Private.CoreLib/src/System/Runtime/InteropServices/RuntimeInformation.Windows.cs

@@ -42,7 +42,8 @@ public static unsafe Architecture OSArchitecture
                    // If we are running an x64 process on a non-x64 windows machine, we will report x64 as OS architecture.
                    //
                    // IsWow64Process2 is only available on Windows 10+, so we will perform run-time introspection via indirect load
-                    if (NativeLibrary.TryGetExport(NativeLibrary.Load(Interop.Libraries.Kernel32), "IsWow64Process2", out IntPtr isWow64Process2Ptr))
+                    IntPtr kernel32 = Interop.Kernel32.LoadLibraryEx(Interop.Libraries.Kernel32, 0, Interop.Kernel32.LOAD_LIBRARY_SEARCH_SYSTEM32);


Unrelated change to make load library more secure?

Yes: #72452 (comment)

adamsitnik

LGTM, thank you for your contribution @teo-tsirpanis !

adamsitnik · 2022-08-11T09:33:51Z

The failure is unrelated (#73679), merging

dotnet-issue-labeler bot added the area-System.IO label Jul 19, 2022

ghost added the community-contribution Indicates that the PR has been added by a community member label Jul 19, 2022

jkotas reviewed Jul 19, 2022

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs Outdated Show resolved Hide resolved

jkotas reviewed Jul 19, 2022

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs Outdated Show resolved Hide resolved

teo-tsirpanis added 2 commits July 19, 2022 18:23

Use GetTempPath2W if available.

9ebb601

Avoid calling NativeLibrary.Load in `RuntimeInformation.OSArchitect…

c1c76ca

…ure`.

teo-tsirpanis force-pushed the gettemppath2 branch from d966908 to c1c76ca Compare July 19, 2022 15:27

stephentoub reviewed Jul 19, 2022

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs Show resolved Hide resolved

Make the native function pointer field volatile.

f8c5521

runfoapp bot mentioned this pull request Jul 20, 2022

Methodical_others test JIT/Methodical/Coverage/copy_prop_byref_to_native_int crashing #69832

Open

stephentoub reviewed Jul 22, 2022

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs Show resolved Hide resolved

stephentoub approved these changes Jul 22, 2022

View reviewed changes

Update src/libraries/System.Private.CoreLib/src/System/IO/Path.Window…

b18a9e3

…s.cs Co-authored-by: Stephen Toub <stoub@microsoft.com>

jozkee reviewed Aug 5, 2022

View reviewed changes

src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs Outdated Show resolved Hide resolved

src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs Show resolved Hide resolved

teo-tsirpanis and others added 2 commits August 5, 2022 22:59

Update src/libraries/System.Private.CoreLib/src/System/IO/Path.Window…

4d30146

…s.cs Co-authored-by: David Cantú <dacantu@microsoft.com>

Remove whitespace

14d3230

This was referenced Aug 8, 2022

Infra improvements for Helix #68176

Closed

GC/API/GC/GetGCMemoryInfo/GetGCMemoryInfo.sh test failing intermittently on CoreCLR Linux ARM32 #73247

Closed

Merge branch 'main' into gettemppath2

1414031

danmoseley reviewed Aug 11, 2022

View reviewed changes

adamsitnik approved these changes Aug 11, 2022

View reviewed changes

adamsitnik merged commit cbf2e32 into dotnet:main Aug 11, 2022

adamsitnik added this to the 7.0.0 milestone Aug 11, 2022

teo-tsirpanis deleted the gettemppath2 branch August 11, 2022 12:14

DrewScoggins mentioned this pull request Aug 23, 2022

[Perf] Windows 10.0.25094/arm64 : Regression on 8/11/2022 10:02:44 AM #74440

Closed

ghost locked as resolved and limited conversation to collaborators Sep 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `GetTempPath2W` if available. #72452

Use `GetTempPath2W` if available. #72452

teo-tsirpanis commented Jul 19, 2022 •

edited

Loading

ghost commented Jul 19, 2022

jkotas commented Jul 19, 2022

danmoseley commented Jul 19, 2022

teo-tsirpanis commented Jul 19, 2022

danmoseley commented Jul 19, 2022

GrabYourPitchforks commented Jul 19, 2022

danmoseley commented Aug 10, 2022

danmoseley commented Aug 10, 2022

teo-tsirpanis commented Aug 10, 2022

danmoseley commented Aug 11, 2022

danmoseley Aug 11, 2022

jkotas Aug 11, 2022

adamsitnik left a comment

adamsitnik commented Aug 11, 2022

Use GetTempPath2W if available. #72452

Use GetTempPath2W if available. #72452

Conversation

teo-tsirpanis commented Jul 19, 2022 • edited Loading

ghost commented Jul 19, 2022

jkotas commented Jul 19, 2022

danmoseley commented Jul 19, 2022

teo-tsirpanis commented Jul 19, 2022

danmoseley commented Jul 19, 2022

GrabYourPitchforks commented Jul 19, 2022

danmoseley commented Aug 10, 2022

danmoseley commented Aug 10, 2022

teo-tsirpanis commented Aug 10, 2022

danmoseley commented Aug 11, 2022

danmoseley Aug 11, 2022

Choose a reason for hiding this comment

jkotas Aug 11, 2022

Choose a reason for hiding this comment

adamsitnik left a comment

Choose a reason for hiding this comment

adamsitnik commented Aug 11, 2022

Use `GetTempPath2W` if available. #72452

Use `GetTempPath2W` if available. #72452

teo-tsirpanis commented Jul 19, 2022 •

edited

Loading