★ wanayoo — archive 1999 https://github.com/PowerShell/PowerShell/pull/8396Nouvelle recherche | Portail wanayoo
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

Some more changes and cleanup to reduce the startup time #8396

Merged
merged 5 commits into from Dec 6, 2018

Conversation

@daxian-dbw
Copy link
Member

commented Dec 4, 2018 •

PR Summary

This PR includes some changes and cleanups that will reduce the startup time. Each commit is self-contained, and the commit message serves as a summary of the changes in the commit.

  • Rename s_wasSystemPolicyDebugPolicy to s_allowDebugOverridePolicy to make it less confusing. Also slightly refactor HelperSecurity.psm1 and ConstrainedLanguageDebugger.Tests.ps1 to remove unneeded code. There is no functional change in this commit.
  • Remove the unneeded static property IsInbox, as PowerShell Core won't be shipped in-box with Windows in the foreseeable feature. Even if we do in future, we won't be needing it because Windows PowerShell will probably be gone by that time.
    • This static property was hit at startup time when constructing the value for $env:PSModulePath, and it will in turn cause IsNanoServer and IsIoT to be evaluated, all of which will trigger access to the Registry. By removing IsInbox, that can be all avoided.
  • Update 'BindRunspace' to avoid getting all commands and unneeded method calls.
    • BindRunspace retrieves all available commands from the session state, but for the most common scenario of creating/opening a Runspace, the retrieved commands are never get used afterwards. The method is updated to avoid retrieving all commands unless it's necessary.
    • The method is also updated to add check before calling into a ProcessXXX method, so that we can avoid some unneeded method calls.
  • Avoid creating a IsSafeValueVisitor every time when IsScriptBlockInFactASafeHashtable runs.
    • The static method IsSafeValueVisitor.IsAstSafe creates an instance of IsSafeValueVisitor every time it runs. Given that IsScriptBlockInFactASafeHashtable gets called relatively frequently in PerformSecurityCheck, this will result in some GC pressure as it will generate transient objects. This is updated to reuse a IsSafeValueVisitor singleton with the default SafeValueContext.
    • The Default and _safeValueContext fields are made readonly, so it cannot be changed by reflection.
    • The _visitCount is changed to uint type, so an attacker cannot change the value of it to a negative number in order to practically increase the max visit count limit.

PR Checklist

@daxian-dbw daxian-dbw force-pushed the daxian-dbw:debugsyspolicy branch from 9f718e0 to d32458e Dec 4, 2018

@daxian-dbw daxian-dbw self-assigned this Dec 4, 2018

@iSazonov

This comment has been minimized.

Copy link
Collaborator

commented Dec 5, 2018

The _visitCount is changed to uint type, so an attacker cannot change the value of it to a negative number in order to practically increase the max visit count limit.

Currently runtime has Unsafe.As method and similar others. Can these methods be used to change the value with reflection?

@PaulHigin
Copy link
Contributor

left a comment

LGTM

@daxian-dbw

This comment has been minimized.

Copy link
Member Author

commented Dec 6, 2018

Currently runtime has Unsafe.As method and similar others. Can these methods be used to change the value with reflection?

@iSazonov I don't think uint field can be set with a negative value by reflection.

@daxian-dbw daxian-dbw merged commit c1e1716 into PowerShell:master Dec 6, 2018

6 checks passed

CodeFactor 24 issues fixed. 23 issues found.
Details
PowerShell-CI-linux #PR-8396-20181205.01 succeeded
Details
PowerShell-CI-macos #PR-8396-20181205.01 succeeded
Details
PowerShell-CI-spelling #PR-8396-20181205.01 succeeded
Details
PowerShell-CI-windows #PR-8396-20181205.01 succeeded
Details
license/cla All CLA requirements met.
Details

@daxian-dbw daxian-dbw deleted the daxian-dbw:debugsyspolicy branch Dec 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.