From 339694220684c079698bb0e3656011b9522d26f3 Mon Sep 17 00:00:00 2001 From: Steve MacLean Date: Wed, 5 Dec 2018 13:03:56 -0500 Subject: [PATCH] Remove FromGAC & OnTPAList (#21371) * Remove FromGAC & OnTPAList * PR feedback --- src/binder/clrprivbindercoreclr.cpp | 14 -------- src/binder/inc/clrprivbindercoreclr.h | 2 -- src/vm/appdomain.cpp | 18 ---------- src/vm/appdomain.hpp | 2 -- src/vm/assemblynative.cpp | 65 +++++++++++++++++------------------ src/vm/clrprivbinderwinrt.cpp | 4 +-- src/vm/coreassemblyspec.cpp | 29 ++-------------- src/vm/coreclr/corebindresult.h | 6 +--- src/vm/coreclr/corebindresult.inl | 18 +--------- src/vm/pefile.cpp | 58 ++----------------------------- src/vm/pefile.h | 6 ---- 11 files changed, 40 insertions(+), 182 deletions(-) diff --git a/src/binder/clrprivbindercoreclr.cpp b/src/binder/clrprivbindercoreclr.cpp index 9bad805..eaa0607 100644 --- a/src/binder/clrprivbindercoreclr.cpp +++ b/src/binder/clrprivbindercoreclr.cpp @@ -230,20 +230,6 @@ HRESULT CLRPrivBinderCoreCLR::SetupBindingPaths(SString &sTrustedPlatformAssemb return hr; } -bool CLRPrivBinderCoreCLR::IsInTpaList(const SString &sFileName) -{ - bool fIsFileOnTpaList = false; - - TpaFileNameHash * tpaFileNameMap = m_appContext.GetTpaFileNameList(); - if (tpaFileNameMap != nullptr) - { - const FileNameMapEntry *pTpaEntry = tpaFileNameMap->LookupPtr(sFileName.GetUnicode()); - fIsFileOnTpaList = (pTpaEntry != nullptr); - } - - return fIsFileOnTpaList; -} - // See code:BINDER_SPACE::AssemblyBinder::GetAssembly for info on fNgenExplicitBind // and fExplicitBindToNativeImage, and see code:CEECompileInfo::LoadAssemblyByPath // for an example of how they're used. diff --git a/src/binder/inc/clrprivbindercoreclr.h b/src/binder/inc/clrprivbindercoreclr.h index e0e8286..e8e26a0 100644 --- a/src/binder/inc/clrprivbindercoreclr.h +++ b/src/binder/inc/clrprivbindercoreclr.h @@ -52,8 +52,6 @@ public: SString &sAppPaths, SString &sAppNiPaths); - bool IsInTpaList(const SString &sFileName); - inline BINDER_SPACE::ApplicationContext *GetAppContext() { return &m_appContext; diff --git a/src/vm/appdomain.cpp b/src/vm/appdomain.cpp index 54f66ca..e852402 100644 --- a/src/vm/appdomain.cpp +++ b/src/vm/appdomain.cpp @@ -8277,24 +8277,6 @@ AppDomain::AssemblyIterator::Next_UnsafeNoAddRef( } // AppDomain::AssemblyIterator::Next_UnsafeNoAddRef -//--------------------------------------------------------------------------------------- -// -BOOL AppDomain::IsImageFromTrustedPath(PEImage* pPEImage) -{ - CONTRACTL - { - MODE_ANY; - GC_TRIGGERS; - THROWS; - PRECONDITION(CheckPointer(pPEImage)); - } - CONTRACTL_END; - - const SString &sImagePath = pPEImage->GetPath(); - - return !sImagePath.IsEmpty(); -} - #endif //!DACCESS_COMPILE #if !defined(DACCESS_COMPILE) && !defined(CROSSGEN_COMPILE) diff --git a/src/vm/appdomain.hpp b/src/vm/appdomain.hpp index 389ce47..58cf11b 100644 --- a/src/vm/appdomain.hpp +++ b/src/vm/appdomain.hpp @@ -3179,8 +3179,6 @@ public: public: - BOOL IsImageFromTrustedPath(PEImage* pImage); - #ifdef FEATURE_TYPEEQUIVALENCE private: VolatilePtr m_pTypeEquivalenceTable; diff --git a/src/vm/assemblynative.cpp b/src/vm/assemblynative.cpp index 2f89be1..14bef33 100644 --- a/src/vm/assemblynative.cpp +++ b/src/vm/assemblynative.cpp @@ -1294,42 +1294,39 @@ INT_PTR QCALLTYPE AssemblyNative::GetLoadContextForAssembly(QCall::AssemblyHandl PTR_PEAssembly pPEAssembly = pPEFile->AsAssembly(); _ASSERTE(pAssembly != NULL); - // Platform assemblies are semantically bound against the "Default" binder which could be the TPA Binder or - // the overridden binder. In either case, the reference to the same will be returned when this QCall returns. - if (!pPEAssembly->IsProfileAssembly()) - { - // Get the binding context for the assembly. - // - ICLRPrivBinder *pOpaqueBinder = nullptr; - AppDomain *pCurDomain = AppDomain::GetCurrentDomain(); - CLRPrivBinderCoreCLR *pTPABinder = pCurDomain->GetTPABinderContext(); + // Platform assemblies are semantically bound against the "Default" binder. + // The reference to the same will be returned when this QCall returns. - - // GetBindingContext returns a ICLRPrivAssembly which can be used to get access to the - // actual ICLRPrivBinder instance in which the assembly was loaded. - PTR_ICLRPrivBinder pBindingContext = pPEAssembly->GetBindingContext(); - UINT_PTR assemblyBinderID = 0; - IfFailThrow(pBindingContext->GetBinderID(&assemblyBinderID)); - - // If the assembly was bound using the TPA binder, - // then we will return the reference to "Default" binder from the managed implementation when this QCall returns. - // - // See earlier comment about "Default" binder for additional context. - pOpaqueBinder = reinterpret_cast(assemblyBinderID); - - // We should have a load context binder at this point. - _ASSERTE(pOpaqueBinder != nullptr); + // Get the binding context for the assembly. + // + ICLRPrivBinder *pOpaqueBinder = nullptr; + AppDomain *pCurDomain = AppDomain::GetCurrentDomain(); + CLRPrivBinderCoreCLR *pTPABinder = pCurDomain->GetTPABinderContext(); - if (!AreSameBinderInstance(pTPABinder, pOpaqueBinder)) - { - // Only CLRPrivBinderAssemblyLoadContext instance contains the reference to its - // corresponding managed instance. - CLRPrivBinderAssemblyLoadContext *pBinder = (CLRPrivBinderAssemblyLoadContext *)(pOpaqueBinder); - - // Fetch the managed binder reference from the native binder instance - ptrManagedAssemblyLoadContext = pBinder->GetManagedAssemblyLoadContext(); - _ASSERTE(ptrManagedAssemblyLoadContext != NULL); - } + // GetBindingContext returns a ICLRPrivAssembly which can be used to get access to the + // actual ICLRPrivBinder instance in which the assembly was loaded. + PTR_ICLRPrivBinder pBindingContext = pPEAssembly->GetBindingContext(); + UINT_PTR assemblyBinderID = 0; + IfFailThrow(pBindingContext->GetBinderID(&assemblyBinderID)); + + // If the assembly was bound using the TPA binder, + // then we will return the reference to "Default" binder from the managed implementation when this QCall returns. + // + // See earlier comment about "Default" binder for additional context. + pOpaqueBinder = reinterpret_cast(assemblyBinderID); + + // We should have a load context binder at this point. + _ASSERTE(pOpaqueBinder != nullptr); + + if (!AreSameBinderInstance(pTPABinder, pOpaqueBinder)) + { + // Only CLRPrivBinderAssemblyLoadContext instance contains the reference to its + // corresponding managed instance. + CLRPrivBinderAssemblyLoadContext *pBinder = (CLRPrivBinderAssemblyLoadContext *)(pOpaqueBinder); + + // Fetch the managed binder reference from the native binder instance + ptrManagedAssemblyLoadContext = pBinder->GetManagedAssemblyLoadContext(); + _ASSERTE(ptrManagedAssemblyLoadContext != NULL); } END_QCALL; diff --git a/src/vm/clrprivbinderwinrt.cpp b/src/vm/clrprivbinderwinrt.cpp index a4d009b..23bf652 100644 --- a/src/vm/clrprivbinderwinrt.cpp +++ b/src/vm/clrprivbinderwinrt.cpp @@ -338,9 +338,7 @@ HRESULT CLRPrivBinderWinRT::BindWinRTAssemblyByName( IfFailGo(GetAssemblyAndTryFindNativeImage(sAssemblyPath, wszFileNameStripped, &pBinderAssembly)); - // We have set bInGac to TRUE here because the plan is full trust for WinRT. If this changes, we may need to check with - // AppDomain to verify trust based on the WinMD's path - pBindResult->Init(pBinderAssembly, TRUE); + pBindResult->Init(pBinderAssembly); NewHolder pNewAssembly( new CLRPrivAssemblyWinRT(this, pResource, pBindResult, fIsWindowsNamespace)); diff --git a/src/vm/coreassemblyspec.cpp b/src/vm/coreassemblyspec.cpp index 5b606dc..832b33e 100644 --- a/src/vm/coreassemblyspec.cpp +++ b/src/vm/coreassemblyspec.cpp @@ -181,38 +181,15 @@ VOID AssemblySpec::Bind(AppDomain *pAppDomain, &pPrivAsm); } - bool fBoundUsingTPABinder = false; - if(SUCCEEDED(hr)) + pResult->SetHRBindResult(hr); + if (SUCCEEDED(hr)) { _ASSERTE(pPrivAsm != nullptr); - if (AreSameBinderInstance(pTPABinder, reinterpret_cast(pPrivAsm.Extract()))) - { - fBoundUsingTPABinder = true; - } - result = BINDER_SPACE::GetAssemblyFromPrivAssemblyFast(pPrivAsm.Extract()); _ASSERTE(result != nullptr); - } - - pResult->SetHRBindResult(hr); - if (SUCCEEDED(hr)) - { - BOOL fIsInGAC = FALSE; - BOOL fIsOnTpaList = FALSE; - - // Only initialize TPA/GAC status if we bound using DefaultContext - if (fBoundUsingTPABinder == true) - { - fIsInGAC = pAppDomain->IsImageFromTrustedPath(result->GetNativeOrILPEImage()); - const SString &sImagePath = result->GetNativeOrILPEImage()->GetPath(); - if (pTPABinder->IsInTpaList(sImagePath)) - { - fIsOnTpaList = TRUE; - } - } - pResult->Init(result,fIsInGAC, fIsOnTpaList); + pResult->Init(result); } else if (FAILED(hr) && (fThrowOnFileNotFound || (!Assembly::FileNotFound(hr)))) { diff --git a/src/vm/coreclr/corebindresult.h b/src/vm/coreclr/corebindresult.h index 3823bfe..12e6d45 100644 --- a/src/vm/coreclr/corebindresult.h +++ b/src/vm/coreclr/corebindresult.h @@ -21,8 +21,6 @@ struct CoreBindResult : public IUnknown { protected: ReleaseHolder m_pAssembly; - BOOL m_bIsFromGAC; - BOOL m_bIsOnTpaList; HRESULT m_hrBindResult; LONG m_cRef; @@ -38,13 +36,11 @@ public: CoreBindResult() : m_cRef(1) {} virtual ~CoreBindResult() {} - void Init(ICLRPrivAssembly* pAssembly, BOOL bFromGAC, BOOL bIsOnTpaList); + void Init(ICLRPrivAssembly* pAssembly); void Reset(); BOOL Found(); PEImage* GetPEImage(); - BOOL IsFromGAC(); - BOOL IsOnTpaList(); BOOL IsMscorlib(); void GetBindAssembly(ICLRPrivAssembly** ppAssembly); #ifdef FEATURE_PREJIT diff --git a/src/vm/coreclr/corebindresult.inl b/src/vm/coreclr/corebindresult.inl index 2cd2651..51a47d1 100644 --- a/src/vm/coreclr/corebindresult.inl +++ b/src/vm/coreclr/corebindresult.inl @@ -15,18 +15,6 @@ #include "clrprivbinderutil.h" -inline BOOL CoreBindResult::IsFromGAC() -{ - LIMITED_METHOD_CONTRACT; - return m_bIsFromGAC; -}; - -inline BOOL CoreBindResult::IsOnTpaList() -{ - LIMITED_METHOD_CONTRACT; - return m_bIsOnTpaList; -}; - inline BOOL CoreBindResult::Found() { LIMITED_METHOD_CONTRACT; @@ -72,14 +60,12 @@ inline PEImage* CoreBindResult::GetPEImage() return m_pAssembly?BINDER_SPACE::GetAssemblyFromPrivAssemblyFast(m_pAssembly)->GetNativeOrILPEImage():NULL; }; -inline void CoreBindResult::Init(ICLRPrivAssembly* pAssembly, BOOL bFromGAC, BOOL bOnTpaList = FALSE) +inline void CoreBindResult::Init(ICLRPrivAssembly* pAssembly) { WRAPPER_NO_CONTRACT; m_pAssembly=pAssembly; if(pAssembly) pAssembly->AddRef(); - m_bIsFromGAC=bFromGAC; - m_bIsOnTpaList = bOnTpaList; m_hrBindResult = S_OK; } @@ -87,8 +73,6 @@ inline void CoreBindResult::Reset() { WRAPPER_NO_CONTRACT; m_pAssembly=NULL; - m_bIsFromGAC=FALSE; - m_bIsOnTpaList=FALSE; m_hrBindResult = S_OK; } #ifdef FEATURE_PREJIT diff --git a/src/vm/pefile.cpp b/src/vm/pefile.cpp index d485e19..fa5b1d4 100644 --- a/src/vm/pefile.cpp +++ b/src/vm/pefile.cpp @@ -1927,10 +1927,7 @@ PEAssembly::PEAssembly( : PEFile(pBindResultInfo ? (pBindResultInfo->GetPEImage() ? pBindResultInfo->GetPEImage() : (pBindResultInfo->HasNativeImage() ? pBindResultInfo->GetNativeImage() : NULL) ): pPEImageIL? pPEImageIL:(pPEImageNI? pPEImageNI:NULL), FALSE), - m_creator(clr::SafeAddRef(creator)), - m_bIsFromGAC(FALSE), - m_bIsOnTpaList(FALSE) - ,m_fProfileAssembly(0) + m_creator(clr::SafeAddRef(creator)) { CONTRACTL { @@ -1962,14 +1959,6 @@ PEAssembly::PEAssembly( if (!HasNativeImage() || !IsILOnly()) EnsureImageOpened(); - // Initialize the status of the assembly being in the GAC, or being part of the TPA list, before - // we start to do work (like strong name verification) that relies on those states to be valid. - if(pBindResultInfo != nullptr) - { - m_bIsFromGAC = pBindResultInfo->IsFromGAC(); - m_bIsOnTpaList = pBindResultInfo->IsOnTpaList(); - } - // Open metadata eagerly to minimize failure windows if (pEmit == NULL) OpenMDImport_Unsafe(); //constructor, cannot race with anything @@ -2130,7 +2119,7 @@ PEAssembly *PEAssembly::DoOpenSystem(IUnknown * pAppCtx) IfFailThrow(CCoreCLRBinderHelper::BindToSystem(&pPrivAsm, !IsCompilationProcess() || g_fAllowNativeImages)); if(pPrivAsm != NULL) { - bindResult.Init(pPrivAsm, TRUE, TRUE); + bindResult.Init(pPrivAsm); } RETURN new PEAssembly(&bindResult, NULL, NULL, TRUE, FALSE); @@ -2217,7 +2206,7 @@ PEAssembly *PEAssembly::DoOpenMemory( CoreBindResult bindResult; ReleaseHolder assembly; IfFailThrow(CCoreCLRBinderHelper::GetAssemblyFromImage(image, NULL, &assembly)); - bindResult.Init(assembly,FALSE,FALSE); + bindResult.Init(assembly); RETURN new PEAssembly(&bindResult, NULL, pParentAssembly, FALSE); } @@ -2300,53 +2289,12 @@ void PEAssembly::SetNativeImage(PEImage * image) #endif // FEATURE_PREJIT - -BOOL PEAssembly::IsSourceGAC() -{ - WRAPPER_NO_CONTRACT; - return m_bIsFromGAC; -}; - - #endif // #ifndef DACCESS_COMPILE #ifndef DACCESS_COMPILE -BOOL PEAssembly::IsProfileAssembly() -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_ANY; - } - CONTRACTL_END; - - // - // For now, cache the result of the check below. This cache should be removed once/if the check below - // becomes cheap (e.g. does not access metadata anymore). - // - if (VolatileLoadWithoutBarrier(&m_fProfileAssembly) != 0) - { - return m_fProfileAssembly > 0; - } - - // - // In order to be a platform (profile) assembly, you must be from a trusted location (TPA list) - // If we are binding by TPA list and this assembly is on it, IsSourceGAC is true => Assembly is Profile - // If the assembly is a WinMD, it is automatically trusted since all WinMD scenarios are full trust scenarios. - // - // The check for Silverlight strongname platform assemblies is legacy backdoor. It was introduced by accidental abstraction leak - // from the old Silverlight binder, people took advantage of it and we cannot easily get rid of it now. See DevDiv #710462. - // - BOOL bProfileAssembly = IsSourceGAC() && (IsSystem() || m_bIsOnTpaList); - - m_fProfileAssembly = bProfileAssembly ? 1 : -1; - return bProfileAssembly; -} - // ------------------------------------------------------------ // Descriptive strings // ------------------------------------------------------------ diff --git a/src/vm/pefile.h b/src/vm/pefile.h index 0baaaea..b4bbfb6 100644 --- a/src/vm/pefile.h +++ b/src/vm/pefile.h @@ -678,9 +678,6 @@ class PEAssembly : public PEFile // binding & source // ------------------------------------------------------------ - BOOL IsSourceGAC(); - BOOL IsProfileAssembly(); - ULONG HashIdentity(); #ifndef DACCESS_COMPILE @@ -770,13 +767,10 @@ class PEAssembly : public PEFile // ------------------------------------------------------------ PTR_PEFile m_creator; - BOOL m_bIsFromGAC; - BOOL m_bIsOnTpaList; // Using a separate entry and not m_pHostAssembly because otherwise // HasHostAssembly becomes true that trips various other code paths resulting in bad // things SString m_sTextualIdentity; - int m_fProfileAssembly; // Tri-state cache public: PTR_PEFile GetCreator() -- 2.7.4