Bug 1285898 - [e10s-multi] LocalStorage e10s multiple write avoidance. r=baku

--HG--
extra : rebase_source : 2f6c19aeea1676ac8bffcd075f0dbaf828481ffa
extra : intermediate-source : bc99844772ee5452c313448bfdb0866428f1bcb1
extra : source : bd68ebab64faaabe9a9d4cc294fc4243ca9d8cc3

dom/storage/Storage.cpp

@@ -294,18 +294,18 @@ Storage::ApplyEvent(StorageEvent* aStorageEvent)
   // No key means clearing the full storage.
   if (key.IsVoid()) {
     MOZ_ASSERT(value.IsVoid());
-    mCache->Clear(this);
+    mCache->Clear(this, StorageCache::E10sPropagated);
     return;
   }
 
   // No new value means removing the key.
   if (value.IsVoid()) {
-    mCache->RemoveItem(this, key, old);
+    mCache->RemoveItem(this, key, old, StorageCache::E10sPropagated);
     return;
   }
 
   // Otherwise, we set the new value.
-  mCache->SetItem(this, key, value, old);
+  mCache->SetItem(this, key, value, old, StorageCache::E10sPropagated);
 }
 
 static const char kPermissionType[] = "cookie";

dom/storage/StorageCache.cpp

@@ -59,7 +59,7 @@ GetDataSetIndex(const Storage* aStorage)
 
 NS_IMPL_ADDREF(StorageCacheBridge)
 
-// Since there is no consumer of return value of Release, we can turn this 
+// Since there is no consumer of return value of Release, we can turn this
 // method to void to make implementation of asynchronous StorageCache::Release
 // much simpler.
 NS_IMETHODIMP_(void) StorageCacheBridge::Release(void)
@@ -149,7 +149,7 @@ StorageCache::Init(StorageManagerBase* aManager,
   // Check the quota string has (or has not) the identical origin suffix as
   // this storage cache is bound to.
   MOZ_ASSERT(StringBeginsWith(mQuotaOriginScope, mOriginSuffix));
-  MOZ_ASSERT(mOriginSuffix.IsEmpty() != StringBeginsWith(mQuotaOriginScope, 
+  MOZ_ASSERT(mOriginSuffix.IsEmpty() != StringBeginsWith(mQuotaOriginScope,
                                                          NS_LITERAL_CSTRING("^")));
 
   mUsage = aManager->GetOriginUsage(mQuotaOriginScope);
@@ -198,28 +198,33 @@ StorageCache::DataSet(const Storage* aStorage)
 }
 
 bool
-StorageCache::ProcessUsageDelta(const Storage* aStorage, int64_t aDelta)
+StorageCache::ProcessUsageDelta(const Storage* aStorage, int64_t aDelta,
+                                const MutationSource aSource)
 {
-  return ProcessUsageDelta(GetDataSetIndex(aStorage), aDelta);
+  return ProcessUsageDelta(GetDataSetIndex(aStorage), aDelta, aSource);
 }
 
 bool
-StorageCache::ProcessUsageDelta(uint32_t aGetDataSetIndex, const int64_t aDelta)
+StorageCache::ProcessUsageDelta(uint32_t aGetDataSetIndex, const int64_t aDelta,
+                                const MutationSource aSource)
 {
   // Check if we are in a low disk space situation
-  if (aDelta > 0 && mManager && mManager->IsLowDiskSpace()) {
+  if (aSource == ContentMutation &&
+      aDelta > 0 && mManager && mManager->IsLowDiskSpace()) {
     return false;
   }
 
   // Check limit per this origin
   Data& data = mData[aGetDataSetIndex];
   uint64_t newOriginUsage = data.mOriginQuotaUsage + aDelta;
-  if (aDelta > 0 && newOriginUsage > StorageManagerBase::GetQuota()) {
+  if (aSource == ContentMutation &&
+      aDelta > 0 && newOriginUsage > StorageManagerBase::GetQuota()) {
     return false;
   }
 
   // Now check eTLD+1 limit
-  if (mUsage && !mUsage->CheckAndSetETLD1UsageDelta(aGetDataSetIndex, aDelta)) {
+  if (mUsage &&
+      !mUsage->CheckAndSetETLD1UsageDelta(aGetDataSetIndex, aDelta, aSource)) {
     return false;
   }
 
@@ -385,7 +390,8 @@ StorageCache::GetItem(const Storage* aStorage, const nsAString& aKey,
 
 nsresult
 StorageCache::SetItem(const Storage* aStorage, const nsAString& aKey,
-                      const nsString& aValue, nsString& aOld)
+                      const nsString& aValue, nsString& aOld,
+                      const MutationSource aSource)
 {
   // Size of the cache that will change after this action.
   int64_t delta = 0;
@@ -408,7 +414,7 @@ StorageCache::SetItem(const Storage* aStorage, const nsAString& aKey,
   delta += static_cast<int64_t>(aValue.Length()) -
            static_cast<int64_t>(aOld.Length());
 
-  if (!ProcessUsageDelta(aStorage, delta)) {
+  if (!ProcessUsageDelta(aStorage, delta, aSource)) {
     return NS_ERROR_DOM_QUOTA_REACHED;
   }
 
@@ -418,7 +424,7 @@ StorageCache::SetItem(const Storage* aStorage, const nsAString& aKey,
 
   data.mKeys.Put(aKey, aValue);
 
-  if (Persist(aStorage)) {
+  if (aSource == ContentMutation && Persist(aStorage)) {
     if (!sDatabase) {
       NS_ERROR("Writing to localStorage after the database has been shut down"
                ", data lose!");
@@ -437,7 +443,7 @@ StorageCache::SetItem(const Storage* aStorage, const nsAString& aKey,
 
 nsresult
 StorageCache::RemoveItem(const Storage* aStorage, const nsAString& aKey,
-                         nsString& aOld)
+                         nsString& aOld, const MutationSource aSource)
 {
   if (Persist(aStorage)) {
     WaitForPreload(Telemetry::LOCALDOMSTORAGE_REMOVEKEY_BLOCKING_MS);
@@ -455,10 +461,10 @@ StorageCache::RemoveItem(const Storage* aStorage, const nsAString& aKey,
   // Recalculate the cached data size
   const int64_t delta = -(static_cast<int64_t>(aOld.Length()) +
                           static_cast<int64_t>(aKey.Length()));
-  Unused << ProcessUsageDelta(aStorage, delta);
+  Unused << ProcessUsageDelta(aStorage, delta, aSource);
   data.mKeys.Remove(aKey);
 
-  if (Persist(aStorage)) {
+  if (aSource == ContentMutation && Persist(aStorage)) {
     if (!sDatabase) {
       NS_ERROR("Writing to localStorage after the database has been shut down"
                ", data lose!");
@@ -472,7 +478,7 @@ StorageCache::RemoveItem(const Storage* aStorage, const nsAString& aKey,
 }
 
 nsresult
-StorageCache::Clear(const Storage* aStorage)
+StorageCache::Clear(const Storage* aStorage, const MutationSource aSource)
 {
   bool refresh = false;
   if (Persist(aStorage)) {
@@ -494,11 +500,11 @@ StorageCache::Clear(const Storage* aStorage)
   bool hadData = !!data.mKeys.Count();
 
   if (hadData) {
-    Unused << ProcessUsageDelta(aStorage, -data.mOriginQuotaUsage);
+    Unused << ProcessUsageDelta(aStorage, -data.mOriginQuotaUsage, aSource);
     data.mKeys.Clear();
   }
 
-  if (Persist(aStorage) && (refresh || hadData)) {
+  if (aSource == ContentMutation && Persist(aStorage) && (refresh || hadData)) {
     if (!sDatabase) {
       NS_ERROR("Writing to localStorage after the database has been shut down"
                ", data lose!");
@@ -673,12 +679,13 @@ StorageUsage::LoadUsage(const int64_t aUsage)
 
 bool
 StorageUsage::CheckAndSetETLD1UsageDelta(uint32_t aDataSetIndex,
-                                         const int64_t aDelta)
+  const int64_t aDelta, const StorageCache::MutationSource aSource)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   int64_t newUsage = mUsage[aDataSetIndex] + aDelta;
-  if (aDelta > 0 && newUsage > StorageManagerBase::GetQuota()) {
+  if (aSource == StorageCache::ContentMutation &&
+      aDelta > 0 && newUsage > StorageManagerBase::GetQuota()) {
     return false;
   }
 

dom/storage/StorageCache.h

@@ -78,8 +78,24 @@ class StorageCache : public StorageCacheBridge
 public:
   NS_IMETHOD_(void) Release(void);
 
-  // Note: We pass aOriginNoSuffix through the ctor here, because 
-  // StorageCacheHashKey's ctor is creating this class and 
+  enum MutationSource {
+    // The mutation is a result of an explicit JS mutation in this process.
+    // The mutation should be sent to the sDatabase. Quota will be checked and
+    // QuotaExceededError may be returned without the mutation being applied.
+    ContentMutation,
+    // The mutation initially was triggered in a different process and is being
+    // propagated to this cache via Storage::ApplyEvent.  The mutation should
+    // not be sent to the sDatabase because the originating process is already
+    // doing that.  (In addition to the redundant writes being wasteful, there
+    // is the potential for other processes to see inconsistent state from the
+    // database while preloading.)  Quota will be updated but not checked
+    // because it's assumed it was checked in another process and data-coherency
+    // is more important than slightly exceeding quota.
+    E10sPropagated
+  };
+
+  // Note: We pass aOriginNoSuffix through the ctor here, because
+  // StorageCacheHashKey's ctor is creating this class and
   // accepts reversed-origin-no-suffix as an argument - the hashing key.
   explicit StorageCache(const nsACString* aOriginNoSuffix);
 
@@ -105,10 +121,13 @@ public:
   nsresult GetItem(const Storage* aStorage, const nsAString& aKey,
                    nsAString& aRetval);
   nsresult SetItem(const Storage* aStorage, const nsAString& aKey,
-                   const nsString& aValue, nsString& aOld);
+                   const nsString& aValue, nsString& aOld,
+                   const MutationSource aSource=ContentMutation);
   nsresult RemoveItem(const Storage* aStorage, const nsAString& aKey,
-                      nsString& aOld);
-  nsresult Clear(const Storage* aStorage);
+                      nsString& aOld,
+                      const MutationSource aSource=ContentMutation);
+  nsresult Clear(const Storage* aStorage,
+                 const MutationSource aSource=ContentMutation);
 
   void GetKeys(const Storage* aStorage, nsTArray<nsString>& aKeys);
 
@@ -178,8 +197,18 @@ private:
 
   // Changes the quota usage on the given data set if it fits the quota.
   // If not, then false is returned and no change to the set must be done.
-  bool ProcessUsageDelta(uint32_t aGetDataSetIndex, const int64_t aDelta);
-  bool ProcessUsageDelta(const Storage* aStorage, const int64_t aDelta);
+  // A special case is if aSource==E10sPropagated, then we will return true even
+  // if the change would put us over quota.  This is done to ensure coherency of
+  // caches between processes in the face of races.  It does allow an attacker
+  // to potentially use N multiples of the quota storage limit if they can
+  // arrange for their origin to execute code in N processes.  However, this is
+  // not considered a particularly concerning threat model because it's already
+  // very possible for a rogue page to attempt to intentionally fill up the
+  // user's storage through the use of multiple domains.
+  bool ProcessUsageDelta(uint32_t aGetDataSetIndex, const int64_t aDelta,
+                         const MutationSource aSource=ContentMutation);
+  bool ProcessUsageDelta(const Storage* aStorage, const int64_t aDelta,
+                         const MutationSource aSource=ContentMutation);
 
 private:
   // When a cache is reponsible for its life time (in case of localStorage data
@@ -204,7 +233,7 @@ private:
   // The origin attributes suffix
   nsCString mOriginSuffix;
 
-  // The eTLD+1 scope used to count quota usage.  It is in the reversed format 
+  // The eTLD+1 scope used to count quota usage.  It is in the reversed format
   // and contains the origin attributes suffix.
   nsCString mQuotaOriginScope;
 
@@ -270,7 +299,8 @@ class StorageUsage : public StorageUsageBridge
 public:
   explicit StorageUsage(const nsACString& aOriginScope);
 
-  bool CheckAndSetETLD1UsageDelta(uint32_t aDataSetIndex, int64_t aUsageDelta);
+  bool CheckAndSetETLD1UsageDelta(uint32_t aDataSetIndex, int64_t aUsageDelta,
+                                  const StorageCache::MutationSource aSource);
 
 private:
   virtual const nsCString& OriginScope() { return mOriginScope; }