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

App Crash. Unhandled NullReference Exception while copying to remote session #16542

Closed
5 tasks done
SergeyZalyadeev opened this issue Dec 2, 2021 · 5 comments
Closed
5 tasks done
Labels
Issue-Bug Issue has been identified as a bug in the product Resolution-Fixed The issue is fixed. WG-Remoting PSRP issues with any transport layer

Comments

@SergeyZalyadeev
Copy link
Contributor

Prerequisites

Steps to reproduce

$cred=[PSCredential]::New('user',(ConvertTo-SecureString 'password' -AsPlainText -Force))
$session = New-PSSession -Credential $cred
Copy-Item PathToLocalFile_10MB RemotePath -ToSession $session

Expected behavior

No crash

Actual behavior

Pwsh terminated due to unhandled NullReferenceException 
 System.Management.Automation.Remoting.PrioritySendDataCollection.ReadOrRegisterCallback(OnDataAvailableCallback callback, DataPriorityType& priorityType)
at System.Management.Automation.Remoting.Client.WSManClientCommandTransportManager.SendOneItem()
at System.Management.Automation.Remoting.Client.WSManClientCommandTransportManager.OnRemoteCmdSendCompleted(IntPtr operationContext, Int32 flags, IntPtr error, IntPtr shellOperationHandle, IntPtr commandOperationHandle, IntPtr operationHandle, IntPtr data)

Error details

No response

Environment data

Name                           Value
----                           -----
PSVersion                      7.1.4
PSEdition                      Core
GitCommitId                    7.1.4
OS                             Microsoft Windows 10.0.19042
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0…}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

Visuals

image

Race condition in threads

Dereference the member variable _dataToBeSent[1] when it has been cleared by the different thread that called Clear() method.
https://github.com/PowerShell/PowerShell/blob/master/src/System.Management.Automation/engine/remoting/fanin/PriorityCollection.cs#L217-L226

image

The stack of thread that cleared _dataToBeSent[1] member.

image

Simple Fix:
Insert lock and null check before dereferencing at line 226
https://github.com/PowerShell/PowerShell/blob/master/src/System.Management.Automation/engine/remoting/fanin/PriorityCollection.cs#L226

            lock (_dataSyncObjects[promptResponseIndex])
            {
                if (_dataToBeSent[promptResponseIndex] != null)
@SergeyZalyadeev SergeyZalyadeev added the Needs-Triage The issue is new and needs to be triaged by a work group. label Dec 2, 2021
@iSazonov iSazonov added the WG-Remoting PSRP issues with any transport layer label Dec 3, 2021
@iSazonov
Copy link
Collaborator

iSazonov commented Dec 3, 2021

@SergeyZalyadeev Thanks for your investigations! Do you want to create PR?

@SergeyZalyadeev
Copy link
Contributor Author

I'll try

SergeyZalyadeev pushed a commit to SergeyZalyadeev/PowerShell that referenced this issue Dec 30, 2021
@PaulHigin PaulHigin removed the Needs-Triage The issue is new and needs to be triaged by a work group. label Jan 11, 2022
@PaulHigin
Copy link
Contributor

@WG-Remoting, Remoting working group agrees with the scenario and crash analysis. Thanks!

@iSazonov iSazonov added the Issue-Bug Issue has been identified as a bug in the product label Jan 11, 2022
SergeyZalyadeev pushed a commit to SergeyZalyadeev/PowerShell that referenced this issue Jan 13, 2022
PaulHigin pushed a commit that referenced this issue Jan 13, 2022
* fix crash Copy-Item to remote session (#16542)

* update comments

* remove lock (#16542)

Co-authored-by: Sergey Zalyadeev <sergey.zalyadeev@cayosoft.com>
@PaulHigin
Copy link
Contributor

Adding simple null checks appears to have resolved this issue.

@iSazonov iSazonov added the Resolution-Fixed The issue is fixed. label Jan 14, 2022
TrapGodBrim pushed a commit to TrapGodBrim/PowerShell that referenced this issue Jan 19, 2022
…erShell#16683)

* fix crash Copy-Item to remote session (PowerShell#16542)

* update comments

* remove lock (PowerShell#16542)

Co-authored-by: Sergey Zalyadeev <sergey.zalyadeev@cayosoft.com>
@ghost
Copy link

ghost commented Feb 24, 2022

🎉v7.3.0-preview.2 has been released which incorporates this pull request.:tada:

Handy links:

SergeyZalyadeev added a commit to SergeyZalyadeev/PowerShell that referenced this issue Apr 4, 2022
…erShell#16683)

* fix crash Copy-Item to remote session (PowerShell#16542)

* update comments

* remove lock (PowerShell#16542)

Co-authored-by: Sergey Zalyadeev <sergey.zalyadeev@cayosoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Issue-Bug Issue has been identified as a bug in the product Resolution-Fixed The issue is fixed. WG-Remoting PSRP issues with any transport layer
Projects
None yet
Development

No branches or pull requests

3 participants