diff options
| -rw-r--r-- | src/System.Private.CoreLib/Resources/Strings.resx | 3 | ||||
| -rw-r--r-- | src/System.Private.CoreLib/System.Private.CoreLib.csproj | 1 | ||||
| -rw-r--r-- | src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems | 1 | ||||
| -rw-r--r-- | src/System.Private.CoreLib/shared/System/Runtime/InteropServices/SafeHandle.cs | 250 | ||||
| -rw-r--r-- | src/System.Private.CoreLib/src/System/Runtime/InteropServices/SafeHandle.cs | 278 | ||||
| -rw-r--r-- | src/vm/ecalllist.h | 9 | ||||
| -rw-r--r-- | src/vm/object.h | 17 | ||||
| -rw-r--r-- | src/vm/safehandle.cpp | 217 |
8 files changed, 263 insertions, 513 deletions
diff --git a/src/System.Private.CoreLib/Resources/Strings.resx b/src/System.Private.CoreLib/Resources/Strings.resx index cd73361b8d..f769f6c834 100644 --- a/src/System.Private.CoreLib/Resources/Strings.resx +++ b/src/System.Private.CoreLib/Resources/Strings.resx @@ -3091,6 +3091,9 @@ <data name="ObjectDisposed_ViewAccessorClosed" xml:space="preserve"> <value>Cannot access a closed accessor.</value> </data> + <data name="ObjectDisposed_SafeHandleClosed" xml:space="preserve"> + <value>Safe handle has been closed.</value> + </data> <data name="OperationCanceled" xml:space="preserve"> <value>The operation was canceled.</value> </data> diff --git a/src/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/System.Private.CoreLib/System.Private.CoreLib.csproj index 7e414048a1..e785601d35 100644 --- a/src/System.Private.CoreLib/System.Private.CoreLib.csproj +++ b/src/System.Private.CoreLib/System.Private.CoreLib.csproj @@ -247,7 +247,6 @@ <Compile Include="$(BclSourcesRoot)\System\Runtime\InteropServices\GCHandle.CoreCLR.cs" /> <Compile Include="$(BclSourcesRoot)\System\Runtime\InteropServices\Marshal.CoreCLR.cs" /> <Compile Include="$(BclSourcesRoot)\System\Runtime\InteropServices\NativeLibrary.cs" /> - <Compile Include="$(BclSourcesRoot)\System\Runtime\InteropServices\SafeHandle.cs" /> <Compile Include="$(BclSourcesRoot)\System\Runtime\Loader\AssemblyLoadContext.cs" /> <Compile Include="$(BclSourcesRoot)\System\Runtime\Loader\AssemblyDependencyResolver.cs" /> <Compile Include="$(BclSourcesRoot)\System\Runtime\Serialization\FormatterServices.cs" /> diff --git a/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems b/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems index 363ad03e3f..21dc2f92e4 100644 --- a/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems +++ b/src/System.Private.CoreLib/shared/System.Private.CoreLib.Shared.projitems @@ -654,6 +654,7 @@ <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\SafeArrayRankMismatchException.cs" /> <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\SafeArrayTypeMismatchException.cs" /> <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\SafeBuffer.cs" /> + <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\SafeHandle.cs" /> <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\SEHException.cs" /> <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\StructLayoutAttribute.cs" /> <Compile Include="$(MSBuildThisFileDirectory)System\Runtime\InteropServices\TypeIdentifierAttribute.cs" /> diff --git a/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/SafeHandle.cs b/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/SafeHandle.cs new file mode 100644 index 0000000000..596279210d --- /dev/null +++ b/src/System.Private.CoreLib/shared/System/Runtime/InteropServices/SafeHandle.cs @@ -0,0 +1,250 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; +using System.Runtime.ConstrainedExecution; +using System.Threading; + +namespace System.Runtime.InteropServices +{ + // This implementation does not employ critical execution regions and thus cannot + // reliably guarantee handle release in the face of thread aborts. + + /// <summary>Represents a wrapper class for operating system handles.</summary> + public abstract class SafeHandle : CriticalFinalizerObject, IDisposable + { + // IMPORTANT: + // - Do not add or rearrange fields as the EE depends on this layout, + // as well as on the values of the StateBits flags. + // - The EE may also perform the same operations using equivalent native + // code, so this managed code must not assume it is the only code + // manipulating _state. + + /// <summary>Specifies the handle to be wrapped.</summary> + [SuppressMessage("Microsoft.Security", "CA2111:PointersShouldNotBeVisible")] + protected IntPtr handle; + /// <summary>Combined ref count and closed/disposed flags (so we can atomically modify them).</summary> + private volatile int _state; + /// <summary>Whether we can release this handle.</summary> + private readonly bool _ownsHandle; + /// <summary>Whether constructor completed.</summary> + private volatile bool _fullyInitialized; + + /// <summary>Bitmasks for the <see cref="_state"/> field.</summary> + /// <remarks> + /// The state field ends up looking like this: + /// + /// 31 2 1 0 + /// +-----------------------------------------------------------+---+---+ + /// | Ref count | D | C | + /// +-----------------------------------------------------------+---+---+ + /// + /// Where D = 1 means a Dispose has been performed and C = 1 means the + /// underlying handle has been (or will be shortly) released. + /// </remarks> + private static class StateBits + { + public const int Closed = 0b01; + public const int Disposed = 0b10; + public const int RefCount = unchecked(~0b11); // 2 bits reserved for closed/disposed; ref count gets 30 bits + public const int RefCountOne = 1 << 2; + }; + + /// <summary>Creates a SafeHandle class.</summary> + protected SafeHandle(IntPtr invalidHandleValue, bool ownsHandle) + { + handle = invalidHandleValue; + _state = StateBits.RefCountOne; // Ref count 1 and not closed or disposed. + _ownsHandle = ownsHandle; + + if (!ownsHandle) + { + GC.SuppressFinalize(this); + } + + _fullyInitialized = true; + } + +#if !CORERT // CoreRT doesn't correctly support CriticalFinalizerObject + ~SafeHandle() + { + if (_fullyInitialized) + { + Dispose(disposing: false); + } + } +#endif + + protected void SetHandle(IntPtr handle) => this.handle = handle; + + public IntPtr DangerousGetHandle() => handle; + + public bool IsClosed => (_state & StateBits.Closed) == StateBits.Closed; + + public abstract bool IsInvalid { get; } + + public void Close() => Dispose(); + + public void Dispose() + { + Dispose(disposing: true); + GC.SuppressFinalize(this); + } + + protected virtual void Dispose(bool disposing) + { + Debug.Assert(_fullyInitialized); + InternalRelease(disposeOrFinalizeOperation: true); + } + + public void SetHandleAsInvalid() + { + Debug.Assert(_fullyInitialized); + + // Attempt to set closed state (low order bit of the _state field). + // Might have to attempt these repeatedly, if the operation suffers + // interference from an AddRef or Release. + int oldState, newState; + do + { + oldState = _state; + newState = oldState | StateBits.Closed; + } while (Interlocked.CompareExchange(ref _state, newState, oldState) != oldState); + + GC.SuppressFinalize(this); + } + + protected abstract bool ReleaseHandle(); + + public void DangerousAddRef(ref bool success) + { + // To prevent handle recycling security attacks we must enforce the + // following invariant: we cannot successfully AddRef a handle on which + // we've committed to the process of releasing. + + // We ensure this by never AddRef'ing a handle that is marked closed and + // never marking a handle as closed while the ref count is non-zero. For + // this to be thread safe we must perform inspection/updates of the two + // values as a single atomic operation. We achieve this by storing them both + // in a single aligned int and modifying the entire state via interlocked + // compare exchange operations. + + // Additionally we have to deal with the problem of the Dispose operation. + // We must assume that this operation is directly exposed to untrusted + // callers and that malicious callers will try and use what is basically a + // Release call to decrement the ref count to zero and free the handle while + // it's still in use (the other way a handle recycling attack can be + // mounted). We combat this by allowing only one Dispose to operate against + // a given safe handle (which balances the creation operation given that + // Dispose suppresses finalization). We record the fact that a Dispose has + // been requested in the same state field as the ref count and closed state. + + Debug.Assert(_fullyInitialized); + + // Might have to perform the following steps multiple times due to + // interference from other AddRef's and Release's. + int oldState, newState; + do + { + // First step is to read the current handle state. We use this as a + // basis to decide whether an AddRef is legal and, if so, to propose an + // update predicated on the initial state (a conditional write). + // Check for closed state. + oldState = _state; + if ((oldState & StateBits.Closed) != 0) + { + throw new ObjectDisposedException(nameof(SafeHandle), SR.ObjectDisposed_SafeHandleClosed); + } + + // Not closed, let's propose an update (to the ref count, just add + // StateBits.RefCountOne to the state to effectively add 1 to the ref count). + // Continue doing this until the update succeeds (because nobody + // modifies the state field between the read and write operations) or + // the state moves to closed. + newState = oldState + StateBits.RefCountOne; + } while (Interlocked.CompareExchange(ref _state, newState, oldState) != oldState); + + // If we got here we managed to update the ref count while the state + // remained non closed. So we're done. + success = true; + } + + public void DangerousRelease() => InternalRelease(disposeOrFinalizeOperation: false); + + private void InternalRelease(bool disposeOrFinalizeOperation) + { + Debug.Assert(_fullyInitialized || disposeOrFinalizeOperation); + + // See AddRef above for the design of the synchronization here. Basically we + // will try to decrement the current ref count and, if that would take us to + // zero refs, set the closed state on the handle as well. + bool performRelease = false; + + // Might have to perform the following steps multiple times due to + // interference from other AddRef's and Release's. + int oldState, newState; + do + { + // First step is to read the current handle state. We use this cached + // value to predicate any modification we might decide to make to the + // state). + oldState = _state; + + // If this is a Dispose operation we have additional requirements (to + // ensure that Dispose happens at most once as the comments in AddRef + // detail). We must check that the dispose bit is not set in the old + // state and, in the case of successful state update, leave the disposed + // bit set. Silently do nothing if Dispose has already been called. + if (disposeOrFinalizeOperation && ((oldState & StateBits.Disposed) != 0)) + { + return; + } + + // We should never see a ref count of zero (that would imply we have + // unbalanced AddRef and Releases). (We might see a closed state before + // hitting zero though -- that can happen if SetHandleAsInvalid is + // used). + if ((oldState & StateBits.RefCount) == 0) + { + throw new ObjectDisposedException(nameof(SafeHandle), SR.ObjectDisposed_SafeHandleClosed); + } + + // If we're proposing a decrement to zero and the handle is not closed + // and we own the handle then we need to release the handle upon a + // successful state update. If so we need to check whether the handle is + // currently invalid by asking the SafeHandle subclass. We must do this before + // transitioning the handle to closed, however, since setting the closed + // state will cause IsInvalid to always return true. + performRelease = ((oldState & (StateBits.RefCount | StateBits.Closed)) == StateBits.RefCountOne) && + _ownsHandle && + !IsInvalid; + + // Attempt the update to the new state, fail and retry if the initial + // state has been modified in the meantime. Decrement the ref count by + // substracting StateBits.RefCountOne from the state then OR in the bits for + // Dispose (if that's the reason for the Release) and closed (if the + // initial ref count was 1). + newState = oldState - StateBits.RefCountOne; + if ((oldState & StateBits.RefCount) == StateBits.RefCountOne) + { + newState |= StateBits.Closed; + } + if (disposeOrFinalizeOperation) + { + newState |= StateBits.Disposed; + } + } while (Interlocked.CompareExchange(ref _state, newState, oldState) != oldState); + + // If we get here we successfully decremented the ref count. Additionally we + // may have decremented it to zero and set the handle state as closed. In + // this case (providng we own the handle) we will call the ReleaseHandle + // method on the SafeHandle subclass. + if (performRelease) + { + ReleaseHandle(); + } + } + } +} diff --git a/src/System.Private.CoreLib/src/System/Runtime/InteropServices/SafeHandle.cs b/src/System.Private.CoreLib/src/System/Runtime/InteropServices/SafeHandle.cs deleted file mode 100644 index 54107803bd..0000000000 --- a/src/System.Private.CoreLib/src/System/Runtime/InteropServices/SafeHandle.cs +++ /dev/null @@ -1,278 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -/*============================================================ -** -** -** -** A specially designed handle wrapper to ensure we never leak -** an OS handle. The runtime treats this class specially during -** P/Invoke marshaling and finalization. Users should write -** subclasses of SafeHandle for each distinct handle type. -** -** -===========================================================*/ - -namespace System.Runtime.InteropServices -{ - using System; - using System.Diagnostics; - using System.Reflection; - using System.Threading; - using System.Runtime; - using System.Runtime.CompilerServices; - using System.IO; - using System.Runtime.ConstrainedExecution; - using System.Runtime.Versioning; - - /* - Problems addressed by the SafeHandle class: - 1) Critical finalization - ensure we never leak OS resources in SQL. Done - without running truly arbitrary & unbounded amounts of managed code. - 2) Reduced graph promotion - during finalization, keep object graph small - 3) GC.KeepAlive behavior - P/Invoke vs. finalizer thread race conditions (HandleRef) - 4) Elimination of security race conditions w/ explicit calls to Close (HandleProtector) - 5) Enforcement of the above via the type system - Don't use IntPtr anymore. - 6) Allows the handle lifetime to be controlled externally via a boolean. - - Subclasses of SafeHandle will implement the ReleaseHandle abstract method - used to execute any code required to free the handle. This method will be - prepared as a constrained execution region at instance construction time - (along with all the methods in its statically determinable call graph). This - implies that we won't get any inconvenient jit allocation errors or rude - thread abort interrupts while releasing the handle but the user must still - write careful code to avoid injecting fault paths of their own (see the CER - spec for more details). In particular, any sub-methods you call should be - decorated with a reliability contract of the appropriate level. In most cases - this should be: - ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success) - - The GC will run ReleaseHandle methods after any normal finalizers have been - run for objects that were collected at the same time. This ensures classes - like FileStream can run a normal finalizer to flush out existing buffered - data. This is key - it means adding this class to a class like FileStream does - not alter our current semantics w.r.t. finalization today. - - Subclasses must also implement the IsInvalid property so that the - infrastructure can tell when critical finalization is actually required. - Again, this method is prepared ahead of time. It's envisioned that direct - subclasses of SafeHandle will provide an IsInvalid implementation that suits - the general type of handle they support (null is invalid, -1 is invalid etc.) - and then these classes will be further derived for specific safe handle types. - - Most classes using SafeHandle should not provide a finalizer. If they do - need to do so (ie, for flushing out file buffers, needing to write some data - back into memory, etc), then they can provide a finalizer that will be - guaranteed to run before the SafeHandle's critical finalizer. - - Note that SafeHandle's ReleaseHandle is called from a constrained execution - region, and is eagerly prepared before we create your class. This means you - should only call methods with an appropriate reliability contract from your - ReleaseHandle method. - - Subclasses are expected to be written as follows (note that - SuppressUnmanagedCodeSecurity should always be used on any P/Invoke methods - invoked as part of ReleaseHandle, in order to switch the security check from - runtime to jit time and thus remove a possible failure path from the - invocation of the method): - - internal sealed MySafeHandleSubclass : SafeHandle { - // Called by P/Invoke when returning SafeHandles - private MySafeHandleSubclass() : base(IntPtr.Zero, true) - { - } - - // If & only if you need to support user-supplied handles - internal MySafeHandleSubclass(IntPtr preexistingHandle, bool ownsHandle) : base(IntPtr.Zero, ownsHandle) - { - SetHandle(preexistingHandle); - } - - // Do not provide a finalizer - SafeHandle's critical finalizer will - // call ReleaseHandle for you. - - public override bool IsInvalid { - get { return handle == IntPtr.Zero; } - } - - override protected bool ReleaseHandle() - { - return MyNativeMethods.CloseHandle(handle); - } - } - - Then elsewhere to create one of these SafeHandles, define a method - with the following type of signature (CreateFile follows this model). - Note that when returning a SafeHandle like this, P/Invoke will call your - class's default constructor. Also, you probably want to define CloseHandle - somewhere, and remember to apply a reliability contract to it. - - internal static class MyNativeMethods { - [DllImport("kernel32")] - private static extern MySafeHandleSubclass CreateHandle(int someState); - - [DllImport("kernel32", SetLastError=true), ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)] - private static extern bool CloseHandle(IntPtr handle); - } - - Drawbacks with this implementation: - 1) Requires some magic to run the critical finalizer. - 2) Requires more memory than just an IntPtr. - 3) If you use DangerousAddRef and forget to call DangerousRelease, you can leak a SafeHandle. Use CER's & don't do that. - */ - - - // This class should not be serializable - it's a handle. We require unmanaged - // code permission to subclass SafeHandle to prevent people from writing a - // subclass and suddenly being able to run arbitrary native code with the - // same signature as CloseHandle. This is technically a little redundant, but - // we'll do this to ensure we've cut off all attack vectors. Similarly, all - // methods have a link demand to ensure untrusted code cannot directly edit - // or alter a handle. - public abstract class SafeHandle : CriticalFinalizerObject, IDisposable - { - // ! Do not add or rearrange fields as the EE depends on this layout. - //------------------------------------------------------------------ - protected IntPtr handle; // this must be protected so derived classes can use out params. - private int _state; // Combined ref count and closed/disposed flags (so we can atomically modify them). - private bool _ownsHandle; // Whether we can release this handle. -#pragma warning disable 414 - private bool _fullyInitialized; // Whether constructor completed. -#pragma warning restore 414 - - // Creates a SafeHandle class. Users must then set the Handle property. - // To prevent the SafeHandle from being freed, write a subclass that - // doesn't define a finalizer. - protected SafeHandle(IntPtr invalidHandleValue, bool ownsHandle) - { - handle = invalidHandleValue; - _state = 4; // Ref count 1 and not closed or disposed. - _ownsHandle = ownsHandle; - - if (!ownsHandle) - GC.SuppressFinalize(this); - - // Set this last to prevent SafeHandle's finalizer from freeing an - // invalid handle. This means we don't have to worry about - // ThreadAbortExceptions interrupting this constructor or the managed - // constructors on subclasses that call this constructor. - _fullyInitialized = true; - } - - // Migrating InheritanceDemands requires this default ctor, so we can mark it critical - protected SafeHandle() - { - Debug.Fail("SafeHandle's protected default ctor should never be used!"); - throw new NotImplementedException(); - } - - ~SafeHandle() - { - Dispose(false); - } - - [MethodImplAttribute(MethodImplOptions.InternalCall)] - private extern void InternalFinalize(); - - protected void SetHandle(IntPtr handle) - { - this.handle = handle; - } - - // This method is necessary for getting an IntPtr out of a SafeHandle. - // Used to tell whether a call to create the handle succeeded by comparing - // the handle against a known invalid value, and for backwards - // compatibility to support the handle properties returning IntPtrs on - // many of our Framework classes. - // Note that this method is dangerous for two reasons: - // 1) If the handle has been marked invalid with SetHandleAsInvalid, - // DangerousGetHandle will still return the original handle value. - // 2) The handle returned may be recycled at any point. At best this means - // the handle might stop working suddenly. At worst, if the handle or - // the resource the handle represents is exposed to untrusted code in - // any way, this can lead to a handle recycling security attack (i.e. an - // untrusted caller can query data on the handle you've just returned - // and get back information for an entirely unrelated resource). - public IntPtr DangerousGetHandle() - { - return handle; - } - - public bool IsClosed - { - get { return (_state & 1) == 1; } - } - - public abstract bool IsInvalid - { - get; - } - - public void Close() - { - Dispose(true); - } - - public void Dispose() - { - Dispose(true); - } - - protected virtual void Dispose(bool disposing) - { - if (disposing) - InternalDispose(); - else - InternalFinalize(); - } - - [MethodImplAttribute(MethodImplOptions.InternalCall)] - private extern void InternalDispose(); - - // This should only be called for cases when you know for a fact that - // your handle is invalid and you want to record that information. - // An example is calling a syscall and getting back ERROR_INVALID_HANDLE. - // This method will normally leak handles! - [MethodImplAttribute(MethodImplOptions.InternalCall)] - public extern void SetHandleAsInvalid(); - - // Implement this abstract method in your derived class to specify how to - // free the handle. Be careful not write any code that's subject to faults - // in this method (the runtime will prepare the infrastructure for you so - // that no jit allocations etc. will occur, but don't allocate memory unless - // you can deal with the failure and still free the handle). - // The boolean returned should be true for success and false if the runtime - // should fire a SafeHandleCriticalFailure MDA (CustomerDebugProbe) if that - // MDA is enabled. - protected abstract bool ReleaseHandle(); - - // Add a reason why this handle should not be relinquished (i.e. have - // ReleaseHandle called on it). This method has dangerous in the name since - // it must always be used carefully (e.g. called within a CER) to avoid - // leakage of the handle. It returns a boolean indicating whether the - // increment was actually performed to make it easy for program logic to - // back out in failure cases (i.e. is a call to DangerousRelease needed). - // It is passed back via a ref parameter rather than as a direct return so - // that callers need not worry about the atomicity of calling the routine - // and assigning the return value to a variable (the variable should be - // explicitly set to false prior to the call). The only failure cases are - // when the method is interrupted prior to processing by a thread abort or - // when the handle has already been (or is in the process of being) - // released. - [MethodImplAttribute(MethodImplOptions.InternalCall)] - public extern void DangerousAddRef(ref bool success); - - // Partner to DangerousAddRef. This should always be successful when used in - // a correct manner (i.e. matching a successful DangerousAddRef and called - // from a region such as a CER where a thread abort cannot interrupt - // processing). In the same way that unbalanced DangerousAddRef calls can - // cause resource leakage, unbalanced DangerousRelease calls may cause - // invalid handle states to become visible to other threads. This - // constitutes a potential security hole (via handle recycling) as well as a - // correctness problem -- so don't ever expose Dangerous* calls out to - // untrusted code. - [MethodImplAttribute(MethodImplOptions.InternalCall)] - public extern void DangerousRelease(); - } -} diff --git a/src/vm/ecalllist.h b/src/vm/ecalllist.h index 9d3a7e9109..bc801315f1 100644 --- a/src/vm/ecalllist.h +++ b/src/vm/ecalllist.h @@ -179,14 +179,6 @@ FCFuncStart(gExceptionFuncs) FCFuncElement("SaveStackTracesFromDeepCopy", ExceptionNative::SaveStackTracesFromDeepCopy) FCFuncEnd() -FCFuncStart(gSafeHandleFuncs) - FCFuncElement("InternalDispose", SafeHandle::DisposeNative) - FCFuncElement("InternalFinalize", SafeHandle::Finalize) - FCFuncElement("SetHandleAsInvalid", SafeHandle::SetHandleAsInvalid) - FCFuncElement("DangerousAddRef", SafeHandle::DangerousAddRef) - FCFuncElement("DangerousRelease", SafeHandle::DangerousRelease) -FCFuncEnd() - FCFuncStart(gCriticalHandleFuncs) FCFuncElement("ReleaseHandleFailed", CriticalHandle::FireCustomerDebugProbe) FCFuncEnd() @@ -1274,7 +1266,6 @@ FCClassElement("RuntimeModule", "System.Reflection", gCOMModuleFuncs) FCClassElement("RuntimeThread", "Internal.Runtime.Augments", gRuntimeThreadFuncs) FCClassElement("RuntimeType", "System", gSystem_RuntimeType) FCClassElement("RuntimeTypeHandle", "System", gCOMTypeHandleFuncs) -FCClassElement("SafeHandle", "System.Runtime.InteropServices", gSafeHandleFuncs) FCClassElement("SafeTypeNameParserHandle", "System", gSafeTypeNameParserHandle) FCClassElement("Signature", "System", gSignatureNative) diff --git a/src/vm/object.h b/src/vm/object.h index d3a3af3cd9..3745e314c1 100644 --- a/src/vm/object.h +++ b/src/vm/object.h @@ -2083,28 +2083,11 @@ class SafeHandle : public Object return m_handle; } - BOOL OwnsHandle() const - { - LIMITED_METHOD_CONTRACT; - return m_ownsHandle; - } - - static size_t GetHandleOffset() { LIMITED_METHOD_CONTRACT; return offsetof(SafeHandle, m_handle); } - void AddRef(); void Release(bool fDispose = false); - void Dispose(); void SetHandle(LPVOID handle); - - static FCDECL1(void, DisposeNative, SafeHandle* refThisUNSAFE); - static FCDECL1(void, Finalize, SafeHandle* refThisUNSAFE); - static FCDECL1(void, SetHandleAsInvalid, SafeHandle* refThisUNSAFE); - static FCDECL2(void, DangerousAddRef, SafeHandle* refThisUNSAFE, CLR_BOOL *pfSuccess); - static FCDECL1(void, DangerousRelease, SafeHandle* refThisUNSAFE); }; -// SAFEHANDLEREF defined above because CompressedStackObject needs it - void AcquireSafeHandle(SAFEHANDLEREF* s); void ReleaseSafeHandle(SAFEHANDLEREF* s); diff --git a/src/vm/safehandle.cpp b/src/vm/safehandle.cpp index 59d622aed1..2e766e740f 100644 --- a/src/vm/safehandle.cpp +++ b/src/vm/safehandle.cpp @@ -50,6 +50,11 @@ void SafeHandle::Init() s_ReleaseHandleMethodSlot = pMD->GetSlot(); } +// These AddRef and Release methods (and supporting functions) also exist with equivalent +// code in SafeHandle.cs. Those implementations are the primary ones used by most code +// and exposed publicly; the implementations here are only for use by the runtime, without +// having to call out to the managed implementations. + void SafeHandle::AddRef() { CONTRACTL { @@ -64,62 +69,19 @@ void SafeHandle::AddRef() _ASSERTE(sh->IsFullyInitialized()); - // To prevent handle recycling security attacks we must enforce the - // following invariant: we cannot successfully AddRef a handle on which - // we've committed to the process of releasing. - - // We ensure this by never AddRef'ing a handle that is marked closed and - // never marking a handle as closed while the ref count is non-zero. For - // this to be thread safe we must perform inspection/updates of the two - // values as a single atomic operation. We achieve this by storing them both - // in a single aligned DWORD and modifying the entire state via interlocked - // compare exchange operations. - - // Additionally we have to deal with the problem of the Dispose operation. - // We must assume that this operation is directly exposed to untrusted - // callers and that malicious callers will try and use what is basically a - // Release call to decrement the ref count to zero and free the handle while - // it's still in use (the other way a handle recycling attack can be - // mounted). We combat this by allowing only one Dispose to operate against - // a given safe handle (which balances the creation operation given that - // Dispose suppresses finalization). We record the fact that a Dispose has - // been requested in the same state field as the ref count and closed state. - - // So the state field ends up looking like this: - // - // 31 2 1 0 - // +-----------------------------------------------------------+---+---+ - // | Ref count | D | C | - // +-----------------------------------------------------------+---+---+ - // - // Where D = 1 means a Dispose has been performed and C = 1 means the - // underlying handle has (or will be shortly) released. - - // Might have to perform the following steps multiple times due to - // interference from other AddRef's and Release's. + // See comments in SafeHandle.cs + INT32 oldState, newState; do { - // First step is to read the current handle state. We use this as a - // basis to decide whether an AddRef is legal and, if so, to propose an - // update predicated on the initial state (a conditional write). oldState = sh->m_state; - // Check for closed state. if (oldState & SH_State_Closed) COMPlusThrow(kObjectDisposedException, IDS_EE_SAFEHANDLECLOSED); - // Not closed, let's propose an update (to the ref count, just add - // SH_RefCountOne to the state to effectively add 1 to the ref count). - // Continue doing this until the update succeeds (because nobody - // modifies the state field between the read and write operations) or - // the state moves to closed. newState = oldState + SH_RefCountOne; } while (InterlockedCompareExchange((LONG*)&sh->m_state, newState, oldState) != oldState); - - // If we got here we managed to update the ref count while the state - // remained non closed. So we're done. } void SafeHandle::Release(bool fDispose) @@ -136,46 +98,22 @@ void SafeHandle::Release(bool fDispose) _ASSERTE(sh->IsFullyInitialized()); - // See AddRef above for the design of the synchronization here. Basically we - // will try to decrement the current ref count and, if that would take us to - // zero refs, set the closed state on the handle as well. + // See comments in SafeHandle.cs + bool fPerformRelease = false; - // Might have to perform the following steps multiple times due to - // interference from other AddRef's and Release's. INT32 oldState, newState; do { - // First step is to read the current handle state. We use this cached - // value to predicate any modification we might decide to make to the - // state). oldState = sh->m_state; - - // If this is a Dispose operation we have additional requirements (to - // ensure that Dispose happens at most once as the comments in AddRef - // detail). We must check that the dispose bit is not set in the old - // state and, in the case of successful state update, leave the disposed - // bit set. Silently do nothing if Dispose has already been called - // (because we advertise that as a semantic of Dispose). if (fDispose && (oldState & SH_State_Disposed)) return; - // We should never see a ref count of zero (that would imply we have - // unbalanced AddRef and Releases). (We might see a closed state before - // hitting zero though -- that can happen if SetHandleAsInvalid is - // used). if ((oldState & SH_State_RefCount) == 0) COMPlusThrow(kObjectDisposedException, IDS_EE_SAFEHANDLECLOSED); - // If we're proposing a decrement to zero and the handle is not closed - // and we own the handle then we need to release the handle upon a - // successful state update. fPerformRelease = ((oldState & (SH_State_RefCount | SH_State_Closed)) == SH_RefCountOne) && m_ownsHandle; - // If so we need to check whether the handle is currently invalid by - // asking the SafeHandle subclass. We must do this *before* - // transitioning the handle to closed, however, since setting the closed - // state will cause IsInvalid to always return true. if (fPerformRelease) { GCPROTECT_BEGIN(sh); @@ -198,49 +136,16 @@ void SafeHandle::Release(bool fDispose) GCPROTECT_END(); } - // Attempt the update to the new state, fail and retry if the initial - // state has been modified in the meantime. Decrement the ref count by - // substracting SH_RefCountOne from the state then OR in the bits for - // Dispose (if that's the reason for the Release) and closed (if the - // initial ref count was 1). newState = (oldState - SH_RefCountOne) | ((oldState & SH_State_RefCount) == SH_RefCountOne ? SH_State_Closed : 0) | (fDispose ? SH_State_Disposed : 0); } while (InterlockedCompareExchange((LONG*)&sh->m_state, newState, oldState) != oldState); - // If we get here we successfully decremented the ref count. Additionally we - // may have decremented it to zero and set the handle state as closed. In - // this case (providng we own the handle) we will call the ReleaseHandle - // method on the SafeHandle subclass. if (fPerformRelease) RunReleaseMethod((SafeHandle*) OBJECTREFToObject(sh)); } -void SafeHandle::Dispose() -{ - CONTRACTL { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - INSTANCE_CHECK; - } CONTRACTL_END; - - // You can't use the "this" pointer after the call to Release because - // Release may trigger a GC. - SAFEHANDLEREF sh(this); - - _ASSERTE(sh->IsFullyInitialized()); - - GCPROTECT_BEGIN(sh); - sh->Release(true); - // Suppress finalization on this object (we may be racing here but the - // operation below is idempotent and a dispose should never race a - // finalization). - GCHeapUtilities::GetGCHeap()->SetFinalizationRun(OBJECTREFToObject(sh)); - GCPROTECT_END(); -} - void SafeHandle::SetHandle(LPVOID handle) { CONTRACTL { @@ -311,115 +216,11 @@ void SafeHandle::RunReleaseMethod(SafeHandle* psh) CRITICAL_CALLSITE; CALL_MANAGED_METHOD(fReleaseHandle, CLR_BOOL, args); - if (!fReleaseHandle) { -#ifdef MDA_SUPPORTED - MDA_TRIGGER_ASSISTANT(ReleaseHandleFailed, ReportViolation(sh->GetTypeHandle(), sh->m_handle)); -#endif - } - pThread->m_dwLastError = dwSavedError; GCPROTECT_END(); } -FCIMPL1(void, SafeHandle::DisposeNative, SafeHandle* refThisUNSAFE) -{ - FCALL_CONTRACT; - - SAFEHANDLEREF sh(refThisUNSAFE); - if (sh == NULL) - FCThrowVoid(kNullReferenceException); - - HELPER_METHOD_FRAME_BEGIN_1(sh); - _ASSERTE(sh->IsFullyInitialized()); - sh->Dispose(); - HELPER_METHOD_FRAME_END(); -} -FCIMPLEND - -FCIMPL1(void, SafeHandle::Finalize, SafeHandle* refThisUNSAFE) -{ - FCALL_CONTRACT; - - SAFEHANDLEREF sh(refThisUNSAFE); - _ASSERTE(sh != NULL); - - HELPER_METHOD_FRAME_BEGIN_1(sh); - - if (sh->IsFullyInitialized()) - sh->Dispose(); - - // By the time we get here we better have gotten rid of any handle resources - // we own (unless we were force finalized during shutdown). - - // It's possible to have a critical finalizer reference a - // safehandle that ends up calling DangerousRelease *after* this finalizer - // is run. In that case we assert since the state is not closed. -// _ASSERTE(!sh->IsFullyInitialized() || (sh->m_state & SH_State_Closed) || g_fEEShutDown); - - HELPER_METHOD_FRAME_END(); -} -FCIMPLEND - -FCIMPL1(void, SafeHandle::SetHandleAsInvalid, SafeHandle* refThisUNSAFE) -{ - FCALL_CONTRACT; - - SAFEHANDLEREF sh(refThisUNSAFE); - _ASSERTE(sh != NULL); - - // Attempt to set closed state (low order bit of the m_state field). - // Might have to attempt these repeatedly, if the operation suffers - // interference from an AddRef or Release. - INT32 oldState, newState; - do { - - // First step is to read the current handle state so we can predicate a - // state update on it. - oldState = sh->m_state; - - // New state has the same ref count but is now closed. Attempt to write - // this new state but fail if the state was updated in the meantime. - newState = oldState | SH_State_Closed; - - } while (InterlockedCompareExchange((LONG*)&sh->m_state, newState, oldState) != oldState); - - GCHeapUtilities::GetGCHeap()->SetFinalizationRun(OBJECTREFToObject(sh)); -} -FCIMPLEND - -FCIMPL2(void, SafeHandle::DangerousAddRef, SafeHandle* refThisUNSAFE, CLR_BOOL *pfSuccess) -{ - FCALL_CONTRACT; - - SAFEHANDLEREF sh(refThisUNSAFE); - - HELPER_METHOD_FRAME_BEGIN_1(sh); - - if (pfSuccess == NULL) - COMPlusThrow(kNullReferenceException); - - sh->AddRef(); - *pfSuccess = TRUE; - - HELPER_METHOD_FRAME_END(); -} -FCIMPLEND - -FCIMPL1(void, SafeHandle::DangerousRelease, SafeHandle* refThisUNSAFE) -{ - FCALL_CONTRACT; - - SAFEHANDLEREF sh(refThisUNSAFE); - - HELPER_METHOD_FRAME_BEGIN_1(sh); - - sh->Release(FALSE); - - HELPER_METHOD_FRAME_END(); -} -FCIMPLEND - FCIMPL1(void, CriticalHandle::FireCustomerDebugProbe, CriticalHandle* refThisUNSAFE) { FCALL_CONTRACT; |