From 06126c3bb2d164e179cfae61e6e89dec07aa80ae Mon Sep 17 00:00:00 2001 From: Kirill Pleshivtsev Date: Mon, 16 Sep 2024 16:10:22 +0700 Subject: [PATCH] Fix invalid range request duplicates cache poisoning (#2037) --- .../storage/volume/volume_actor_forward.cpp | 27 ++++++++++--------- .../libs/storage/volume/volume_ut.cpp | 13 ++++++++- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/cloud/blockstore/libs/storage/volume/volume_actor_forward.cpp b/cloud/blockstore/libs/storage/volume/volume_actor_forward.cpp index 501c6e6ea01..2ab5a3c3ccc 100644 --- a/cloud/blockstore/libs/storage/volume/volume_actor_forward.cpp +++ b/cloud/blockstore/libs/storage/volume/volume_actor_forward.cpp @@ -654,6 +654,20 @@ void TVolumeActor::ForwardRequest( } } + /* + * Validation of the request blocks range + */ + if constexpr (IsReadOrWriteMethod) { + 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. @@ -705,19 +719,6 @@ void TVolumeActor::ForwardRequest( } } - /* - * Validation of the request blocks range - */ - if constexpr (IsReadOrWriteMethod) { - 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. diff --git a/cloud/blockstore/libs/storage/volume/volume_ut.cpp b/cloud/blockstore/libs/storage/volume/volume_ut.cpp index c74e22a10ea..a8a1b603066 100644 --- a/cloud/blockstore/libs/storage/volume/volume_ut.cpp +++ b/cloud/blockstore/libs/storage/volume/volume_ut.cpp @@ -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 ); @@ -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);