Skip to content

Commit

Permalink
Fix invalid range request duplicates cache poisoning (#2037)
Browse files Browse the repository at this point in the history
  • Loading branch information
drbasic committed Sep 19, 2024
1 parent 28acd18 commit 06126c3
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 14 deletions.
27 changes: 14 additions & 13 deletions cloud/blockstore/libs/storage/volume/volume_actor_forward.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,20 @@ void TVolumeActor::ForwardRequest(
}
}

/*
* Validation of the request blocks range
*/
if constexpr (IsReadOrWriteMethod<TMethod>) {
const auto range = BuildRequestBlockRange(*msg, State->GetBlockSize());
if (!CheckReadWriteBlockRange(range)) {
replyError(MakeError(
E_ARGUMENT,
TStringBuilder()
<< "invalid block range " << DescribeRange(range)));
return;
}
}

/*
* Processing overlapping writes. Overlapping writes should not be sent
* to the underlying (storage) layer.
Expand Down Expand Up @@ -705,19 +719,6 @@ void TVolumeActor::ForwardRequest(
}
}

/*
* Validation of the request blocks range
*/
if constexpr (IsReadOrWriteMethod<TMethod>) {
const auto range = BuildRequestBlockRange(*msg, State->GetBlockSize());
if (!CheckReadWriteBlockRange(range)) {
replyError(MakeError(
E_ARGUMENT,
TStringBuilder()
<< "invalid block range " << DescribeRange(range)));
return;
}
}

/*
* Passing the request to the underlying (storage) layer.
Expand Down
13 changes: 12 additions & 1 deletion cloud/blockstore/libs/storage/volume/volume_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8248,7 +8248,7 @@ Y_UNIT_TEST_SUITE(TVolumeTest)
// WriteBlocks
{
auto request = volume.CreateWriteBlocksRequest(
TBlockRange64::WithLength(2048, 1024),
TBlockRange64::WithLength(1500, 1024),
clientInfo.GetClientId(),
1
);
Expand All @@ -8258,6 +8258,17 @@ Y_UNIT_TEST_SUITE(TVolumeTest)
UNIT_ASSERT_VALUES_EQUAL(E_ARGUMENT, response->GetStatus());
}

// Writing to a valid range that is covered by the range of the
// previously rejected request should be successful.
{
auto response = volume.WriteBlocks(
TBlockRange64::WithLength(1500, 500),
clientInfo.GetClientId(),
1);
UNIT_ASSERT(response);
UNIT_ASSERT_VALUES_EQUAL(S_OK, response->GetStatus());
}

// WriteBlocksLocal
{
TString data(DefaultBlockSize, 2);
Expand Down

0 comments on commit 06126c3

Please sign in to comment.