From 401e3dc9b69c94f4a9390de46de376a315bc2d09 Mon Sep 17 00:00:00 2001 From: "L. David Baron" Date: Thu, 17 Mar 2011 20:14:31 -0700 Subject: [PATCH] Add poisoning for nsRuleData::mValueOffsets. (Bug 636039, patch 19) r=bzbarsky I tested manually that after: (a) removing the |ruleData.mValueOffsets[aSID] = 0;| in nsRuleNode::WalkRuleTree (b) removing the NS_ABORT_IF_FALSE(aRuleData->mValueOffsets[aSID] == 0, ...) from nsRuleNode::CheckSpecifiedProperties and UnsetPropertiesWithoutFlags that we crash dereferencing the poison address in a SetCoord call inside nsRuleNode::ComputeTextResetData --- layout/base/nsPresArena.cpp | 6 ++++++ layout/base/nsPresArena.h | 9 ++++++++ layout/style/nsRuleData.cpp | 43 +++++++++++++++++++++++++++++++++++++ layout/style/nsRuleData.h | 37 ++++++++++++++----------------- layout/style/nsRuleNode.cpp | 40 ++++++++++++++++++++++------------ 5 files changed, 100 insertions(+), 35 deletions(-) diff --git a/layout/base/nsPresArena.cpp b/layout/base/nsPresArena.cpp index 4456cd17b9b1..7371fb400753 100644 --- a/layout/base/nsPresArena.cpp +++ b/layout/base/nsPresArena.cpp @@ -464,3 +464,9 @@ nsPresArena::FreeByCode(nsQueryFrame::FrameIID aCode, void* aPtr) { mState->Free(aCode, aPtr); } + +/* static */ PRUword +nsPresArena::GetPoisonValue() +{ + return ARENA_POISON; +} diff --git a/layout/base/nsPresArena.h b/layout/base/nsPresArena.h index 46a1f2d96fe8..ff5f23966061 100644 --- a/layout/base/nsPresArena.h +++ b/layout/base/nsPresArena.h @@ -76,6 +76,15 @@ public: PRUint32 Size(); + /** + * Get the poison value that can be used to fill a memory space with + * an address that leads to a safe crash when dereferenced. + * + * The caller is responsible for ensuring that a pres shell has been + * initialized before calling this. + */ + static PRUword GetPoisonValue(); + private: struct State; State* mState; diff --git a/layout/style/nsRuleData.cpp b/layout/style/nsRuleData.cpp index a3725fa5cebf..552570992317 100644 --- a/layout/style/nsRuleData.cpp +++ b/layout/style/nsRuleData.cpp @@ -38,3 +38,46 @@ #include "nsRuleData.h" #include "nsCSSProps.h" +#include "nsPresArena.h" + +inline size_t +nsRuleData::GetPoisonOffset() +{ + // Fill in mValueOffsets such that mValueStorage + mValueOffsets[i] + // will yield the frame poison value for all uninitialized value + // offsets. + PR_STATIC_ASSERT(sizeof(PRUword) == sizeof(size_t)); + PR_STATIC_ASSERT(PRUword(-1) > PRUword(0)); + PR_STATIC_ASSERT(size_t(-1) > size_t(0)); + PRUword framePoisonValue = nsPresArena::GetPoisonValue(); + return size_t(framePoisonValue - PRUword(mValueStorage)) / + sizeof(nsCSSValue); +} + +nsRuleData::nsRuleData(PRUint32 aSIDs, nsCSSValue* aValueStorage, + nsPresContext* aContext, nsStyleContext* aStyleContext) + : mSIDs(aSIDs), + mCanStoreInRuleTree(PR_TRUE), + mPresContext(aContext), + mStyleContext(aStyleContext), + mPostResolveCallback(nsnull), + mValueStorage(aValueStorage) +{ + size_t framePoisonOffset = GetPoisonOffset(); + for (size_t i = 0; i < nsStyleStructID_Length; ++i) { + mValueOffsets[i] = framePoisonOffset; + } +} + +#ifdef DEBUG +nsRuleData::~nsRuleData() +{ + // assert nothing in mSIDs has poison value + size_t framePoisonOffset = GetPoisonOffset(); + for (size_t i = 0; i < nsStyleStructID_Length; ++i) { + NS_ABORT_IF_FALSE(!(mSIDs & (1 << i)) || + mValueOffsets[i] != framePoisonOffset, + "value in SIDs was left with poison offset"); + } +} +#endif diff --git a/layout/style/nsRuleData.h b/layout/style/nsRuleData.h index dacf1e645493..adfea177d981 100644 --- a/layout/style/nsRuleData.h +++ b/layout/style/nsRuleData.h @@ -56,13 +56,13 @@ typedef void (*nsPostResolveFunc)(void* aStyleStruct, nsRuleData* aData); struct nsRuleData { - PRUint32 mSIDs; + const PRUint32 mSIDs; PRPackedBool mCanStoreInRuleTree; PRPackedBool mIsImportantRule; PRUint8 mLevel; // an nsStyleSet::sheetType - nsPresContext* mPresContext; - nsStyleContext* mStyleContext; - nsPostResolveFunc mPostResolveCallback; + nsPresContext* const mPresContext; + nsStyleContext* const mStyleContext; + const nsPostResolveFunc mPostResolveCallback; // We store nsCSSValues needed to compute the data for one or more // style structs (specified by the bitfield mSIDs). These are stored @@ -75,25 +75,17 @@ struct nsRuleData // nsRuleNode::HasAuthorSpecifiedRules; therefore some code that we // know is not called from HasAuthorSpecifiedRules assumes that the // mValueOffsets for the one struct in mSIDs is zero. - nsCSSValue* mValueStorage; // our user owns this array + nsCSSValue* const mValueStorage; // our user owns this array size_t mValueOffsets[nsStyleStructID_Length]; - nsRuleData(PRUint32 aSIDs, - nsPresContext* aContext, - nsStyleContext* aStyleContext) - : mSIDs(aSIDs), - mCanStoreInRuleTree(PR_TRUE), - mPresContext(aContext), - mStyleContext(aStyleContext), - mPostResolveCallback(nsnull) - { - // FIXME: fill with poison value? - } - ~nsRuleData() { - #ifdef DEBUG - // FIXME: assert nothing in mSIDs has poison value - #endif - } + nsRuleData(PRUint32 aSIDs, nsCSSValue* aValueStorage, + nsPresContext* aContext, nsStyleContext* aStyleContext); + +#ifdef DEBUG + ~nsRuleData(); +#else + ~nsRuleData() {} +#endif /** * Return a pointer to the value object within |this| corresponding @@ -161,6 +153,9 @@ struct nsRuleData #undef CSS_PROP_DOMPROP_PREFIXED #undef CSS_PROP_BACKENDONLY +private: + inline size_t GetPoisonOffset(); + }; #endif diff --git a/layout/style/nsRuleNode.cpp b/layout/style/nsRuleNode.cpp index 74287b1536e3..f4fee7f9ab25 100644 --- a/layout/style/nsRuleNode.cpp +++ b/layout/style/nsRuleNode.cpp @@ -1728,15 +1728,15 @@ const void* nsRuleNode::WalkRuleTree(const nsStyleStructID aSID, nsStyleContext* aContext) { - nsRuleData ruleData(nsCachedStyleData::GetBitForSID(aSID), - mPresContext, aContext); // use placement new[] on the result of alloca() to allocate a // variable-sized stack array, including execution of constructors, // and use an RAII class to run the destructors too. size_t nprops = nsCSSProps::PropertyCountInStruct(aSID); void* dataStorage = alloca(nprops * sizeof(nsCSSValue)); AutoCSSValueArray dataArray(dataStorage, nprops); - ruleData.mValueStorage = dataArray.get(); + + nsRuleData ruleData(nsCachedStyleData::GetBitForSID(aSID), + dataArray.get(), mPresContext, aContext); ruleData.mValueOffsets[aSID] = 0; // We start at the most specific rule in the tree. @@ -2943,9 +2943,10 @@ nsRuleNode::SetGenericFont(nsPresContext* aPresContext, for (PRInt32 i = contextPath.Length() - 1; i >= 0; --i) { nsStyleContext* context = contextPath[i]; - nsRuleData ruleData(NS_STYLE_INHERIT_BIT(Font), aPresContext, context); AutoCSSValueArray dataArray(dataStorage, nprops); - ruleData.mValueStorage = dataArray.get(); + + nsRuleData ruleData(NS_STYLE_INHERIT_BIT(Font), dataArray.get(), + aPresContext, context); ruleData.mValueOffsets[eStyleStruct_Font] = 0; // Trimmed down version of ::WalkRuleTree() to re-apply the style rules @@ -6483,31 +6484,42 @@ nsRuleNode::HasAuthorSpecifiedRules(nsStyleContext* aStyleContext, if (ruleTypeMask & NS_AUTHOR_SPECIFIED_PADDING) inheritBits |= NS_STYLE_INHERIT_BIT(Padding); - /* We're relying on the use of |aStyleContext| not mutating it! */ - nsRuleData ruleData(inheritBits, - aStyleContext->PresContext(), aStyleContext); - // properties in the SIDS, whether or not we care about them - size_t nprops = 0; + size_t nprops = 0, backgroundOffset, borderOffset, paddingOffset; if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BACKGROUND) { - ruleData.mValueOffsets[eStyleStruct_Background] = nprops; + backgroundOffset = nprops; nprops += nsCSSProps::PropertyCountInStruct(eStyleStruct_Background); } if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BORDER) { - ruleData.mValueOffsets[eStyleStruct_Border] = nprops; + borderOffset = nprops; nprops += nsCSSProps::PropertyCountInStruct(eStyleStruct_Border); } if (ruleTypeMask & NS_AUTHOR_SPECIFIED_PADDING) { - ruleData.mValueOffsets[eStyleStruct_Padding] = nprops; + paddingOffset = nprops; nprops += nsCSSProps::PropertyCountInStruct(eStyleStruct_Padding); } void* dataStorage = alloca(nprops * sizeof(nsCSSValue)); AutoCSSValueArray dataArray(dataStorage, nprops); - ruleData.mValueStorage = dataArray.get(); + + /* We're relying on the use of |aStyleContext| not mutating it! */ + nsRuleData ruleData(inheritBits, dataArray.get(), + aStyleContext->PresContext(), aStyleContext); + + if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BACKGROUND) { + ruleData.mValueOffsets[eStyleStruct_Background] = backgroundOffset; + } + + if (ruleTypeMask & NS_AUTHOR_SPECIFIED_BORDER) { + ruleData.mValueOffsets[eStyleStruct_Border] = borderOffset; + } + + if (ruleTypeMask & NS_AUTHOR_SPECIFIED_PADDING) { + ruleData.mValueOffsets[eStyleStruct_Padding] = paddingOffset; + } static const nsCSSProperty backgroundValues[] = { eCSSProperty_background_color,