Commit da226512 authored by Austin Sullivan's avatar Austin Sullivan Committed by Commit Bot

added overflow protection for in-flight memory

uses saturated_cast

Bug: 116075
Change-Id: I228a00f77bd2df3b429b16a58c15eb47285c055b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2435282Reviewed-by: default avatarenne <enne@chromium.org>
Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
Commit-Queue: enne <enne@chromium.org>
Commit-Queue: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812866}
parent bdbeca36
...@@ -1140,8 +1140,8 @@ Status IndexedDBDatabase::PutOperation( ...@@ -1140,8 +1140,8 @@ Status IndexedDBDatabase::PutOperation(
DCHECK_NE(transaction->mode(), blink::mojom::IDBTransactionMode::ReadOnly); DCHECK_NE(transaction->mode(), blink::mojom::IDBTransactionMode::ReadOnly);
bool key_was_generated = false; bool key_was_generated = false;
Status s = Status::OK(); Status s = Status::OK();
transaction->set_in_flight_memory(transaction->in_flight_memory() - transaction->in_flight_memory() -= params->value.SizeEstimate();
params->value.SizeEstimate()); DCHECK(transaction->in_flight_memory().IsValid());
if (!IsObjectStoreIdInMetadata(params->object_store_id)) { if (!IsObjectStoreIdInMetadata(params->object_store_id)) {
IndexedDBDatabaseError error = CreateError( IndexedDBDatabaseError error = CreateError(
...@@ -1285,11 +1285,8 @@ Status IndexedDBDatabase::PutAllOperation( ...@@ -1285,11 +1285,8 @@ Status IndexedDBDatabase::PutAllOperation(
DCHECK_NE(transaction->mode(), blink::mojom::IDBTransactionMode::ReadOnly); DCHECK_NE(transaction->mode(), blink::mojom::IDBTransactionMode::ReadOnly);
bool key_was_generated = false; bool key_was_generated = false;
Status s = Status::OK(); Status s = Status::OK();
// TODO(nums): Add checks to prevent overflow and underflow transaction->in_flight_memory() -= size_estimate.ValueOrDefault(0);
// https://crbug.com/1116075 DCHECK(transaction->in_flight_memory().IsValid());
transaction->set_in_flight_memory(
transaction->in_flight_memory() -
base::checked_cast<int64_t>(size_estimate.ValueOrDie()));
if (!IsObjectStoreIdInMetadata(object_store_id)) { if (!IsObjectStoreIdInMetadata(object_store_id)) {
IndexedDBDatabaseError error = CreateError( IndexedDBDatabaseError error = CreateError(
......
...@@ -680,6 +680,10 @@ TEST_F(IndexedDBDatabaseOperationTest, CreatePutDelete) { ...@@ -680,6 +680,10 @@ TEST_F(IndexedDBDatabaseOperationTest, CreatePutDelete) {
std::vector<IndexedDBIndexKeys> index_keys; std::vector<IndexedDBIndexKeys> index_keys;
base::MockCallback<blink::mojom::IDBTransaction::PutCallback> callback; base::MockCallback<blink::mojom::IDBTransaction::PutCallback> callback;
// Set in-flight memory to a reasonably large number to prevent underflow in
// |PutOperation|
transaction_->in_flight_memory() += 1000;
auto put_params = std::make_unique<IndexedDBDatabase::PutOperationParams>(); auto put_params = std::make_unique<IndexedDBDatabase::PutOperationParams>();
put_params->object_store_id = store_id; put_params->object_store_id = store_id;
put_params->value = value; put_params->value = value;
......
...@@ -151,10 +151,7 @@ class CONTENT_EXPORT IndexedDBTransaction { ...@@ -151,10 +151,7 @@ class CONTENT_EXPORT IndexedDBTransaction {
// in_flight_memory() is used to keep track of all memory scheduled to be // in_flight_memory() is used to keep track of all memory scheduled to be
// written using ScheduleTask. This is reported to memory dumps. // written using ScheduleTask. This is reported to memory dumps.
int64_t in_flight_memory() const { return in_flight_memory_; } base::CheckedNumeric<size_t>& in_flight_memory() { return in_flight_memory_; }
void set_in_flight_memory(int64_t in_flight_memory) {
in_flight_memory_ = in_flight_memory;
}
protected: protected:
// Test classes may derive, but most creation should be done via // Test classes may derive, but most creation should be done via
...@@ -247,7 +244,7 @@ class CONTENT_EXPORT IndexedDBTransaction { ...@@ -247,7 +244,7 @@ class CONTENT_EXPORT IndexedDBTransaction {
// Metrics for quota. // Metrics for quota.
int64_t size_ = 0; int64_t size_ = 0;
int64_t in_flight_memory_ = 0; base::CheckedNumeric<size_t> in_flight_memory_ = 0;
class TaskQueue { class TaskQueue {
public: public:
......
...@@ -148,8 +148,7 @@ void TransactionImpl::Put( ...@@ -148,8 +148,7 @@ void TransactionImpl::Put(
params->callback = std::move(aborting_callback); params->callback = std::move(aborting_callback);
params->index_keys = index_keys; params->index_keys = index_keys;
// This is decremented in IndexedDBDatabase::PutOperation. // This is decremented in IndexedDBDatabase::PutOperation.
transaction_->set_in_flight_memory(transaction_->in_flight_memory() + transaction_->in_flight_memory() += output_value.SizeEstimate();
output_value.SizeEstimate());
transaction_->ScheduleTask(BindWeakOperation( transaction_->ScheduleTask(BindWeakOperation(
&IndexedDBDatabase::PutOperation, connection->database()->AsWeakPtr(), &IndexedDBDatabase::PutOperation, connection->database()->AsWeakPtr(),
std::move(params))); std::move(params)));
...@@ -218,11 +217,8 @@ void TransactionImpl::PutAll(int64_t object_store_id, ...@@ -218,11 +217,8 @@ void TransactionImpl::PutAll(int64_t object_store_id,
blink::mojom::IDBTransactionPutAllResultPtr>( blink::mojom::IDBTransactionPutAllResultPtr>(
std::move(callback), transaction_->AsWeakPtr()); std::move(callback), transaction_->AsWeakPtr());
// TODO(nums): Add checks to prevent overflow and underflow transaction_->in_flight_memory() += size_estimate.ValueOrDefault(0);
// https://crbug.com/1116075 DCHECK(transaction_->in_flight_memory().IsValid());
transaction_->set_in_flight_memory(
transaction_->in_flight_memory() +
base::checked_cast<int64_t>(size_estimate.ValueOrDie()));
transaction_->ScheduleTask(BindWeakOperation( transaction_->ScheduleTask(BindWeakOperation(
&IndexedDBDatabase::PutAllOperation, connection->database()->AsWeakPtr(), &IndexedDBDatabase::PutAllOperation, connection->database()->AsWeakPtr(),
object_store_id, std::move(put_params), std::move(aborting_callback))); object_store_id, std::move(put_params), std::move(aborting_callback)));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment