returns E_ARGUMENT if the block range extends beyond the volume bound…

…ary (#1475)
ydb-platform · Aug 20, 2024 · 1b18e7d · 1b18e7d
1 parent 8bd8aae
commit 1b18e7d
Show file tree

Hide file tree

Showing 4 changed files with 124 additions and 4 deletions.
diff --git a/cloud/blockstore/libs/storage/volume/volume_actor.cpp b/cloud/blockstore/libs/storage/volume/volume_actor.cpp
@@ -558,6 +558,12 @@ NKikimr::NMetrics::TResourceMetrics* TVolumeActor::GetResourceMetrics()
     return Executor()->GetResourceMetrics();
 }
 
+bool TVolumeActor::CheckReadWriteBlockRange(const TBlockRange64& range) const
+{
+    return TBlockRange64::WithLength(0, State->GetBlocksCount())
+        .Contains(range);
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 
 void TVolumeActor::HandlePoisonPill(

diff --git a/cloud/blockstore/libs/storage/volume/volume_actor.h b/cloud/blockstore/libs/storage/volume/volume_actor.h
@@ -542,6 +542,8 @@ class TVolumeActor final
 
     NKikimr::NMetrics::TResourceMetrics* GetResourceMetrics();
 
+    bool CheckReadWriteBlockRange(const TBlockRange64& range) const;
+
 private:
     STFUNC(StateBoot);
     STFUNC(StateInit);

diff --git a/cloud/blockstore/libs/storage/volume/volume_actor_forward.cpp b/cloud/blockstore/libs/storage/volume/volume_actor_forward.cpp
@@ -771,6 +771,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;
+        }
+    }
+
     /*
      *  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
@@ -5606,16 +5606,20 @@ Y_UNIT_TEST_SUITE(TVolumeTest)
             0);
         volume.AddClient(clientInfo);
 
-        auto request = volume.CreateWriteBlocksRequest(
-            TBlockRange64::MakeClosedInterval(1024 * 0, 1024 * 24),
+        // PartitionRequestActor should return an error,
+        // to do this we will try to write data to disk using a buffer of the
+        // incorrect size
+        TString data(10u, 1);
+        auto request = volume.CreateWriteBlocksLocalRequest(
+            TBlockRange64::MakeClosedInterval(0, 1024 * 5),
             clientInfo.GetClientId(),
-            1
+            data
         );
         request->Record.MutableHeaders()->MutableInternal()->MutableTrace()->SetIsTraced(true);
 
         volume.SendToPipe(std::move(request));
 
-        auto response = volume.RecvWriteBlocksResponse();
+        auto response = volume.RecvWriteBlocksLocalResponse();
 
         UNIT_ASSERT(FAILED(response->GetStatus()));
 
@@ -8205,6 +8209,100 @@ Y_UNIT_TEST_SUITE(TVolumeTest)
         UNIT_ASSERT(describeRequest);
     }
 
+    Y_UNIT_TEST(ShouldReturnErrorOnInvalidBlockRange)
+    {
+        auto runtime = PrepareTestActorRuntime();
+        auto clientInfo = CreateVolumeClientInfo(
+            NProto::VOLUME_ACCESS_READ_WRITE,
+            NProto::VOLUME_MOUNT_LOCAL,
+            0);
+
+        TVolumeClient volume(*runtime);
+        volume.UpdateVolumeConfig(
+            0,                 // maxBandwidth
+            0,                 // maxIops
+            0,                 // burstPercentage
+            0,                 // maxPostponedWeight
+            false,             // throttlingEnabled
+            1,                 // version
+            NCloud::NProto::STORAGE_MEDIA_SSD_NONREPLICATED,
+            2048,              // block count per partition
+            "vol0",            // diskId
+            "cloud",           // cloudId
+            "folder",          // folderId
+            1,                 // partition count
+            2,                 // blocksPerStripe
+            "track-used",      // tags
+            "base_disk",       // baseDiskId
+            "checkpoint"       // baseDiskCheckpointId
+        );
+        volume.AddClient(clientInfo);
+        volume.WaitReady();
+
+        // WriteBlocks
+        {
+            auto request = volume.CreateWriteBlocksRequest(
+                TBlockRange64::WithLength(2048, 1024),
+                clientInfo.GetClientId(),
+                1
+            );
+            volume.SendToPipe(std::move(request));
+            auto response = volume.RecvWriteBlocksResponse();
+            UNIT_ASSERT(response);
+            UNIT_ASSERT_VALUES_EQUAL(E_ARGUMENT, response->GetStatus());
+        }
+
+        // WriteBlocksLocal
+        {
+            TString data(DefaultBlockSize, 2);
+            auto request = volume.CreateWriteBlocksLocalRequest(
+                TBlockRange64::WithLength(1024 * 6, 1024),
+                clientInfo.GetClientId(),
+                data);
+            volume.SendToPipe(std::move(request));
+            auto response = volume.RecvWriteBlocksLocalResponse();
+            UNIT_ASSERT(response);
+            UNIT_ASSERT_VALUES_EQUAL(E_ARGUMENT, response->GetStatus());
+        }
+
+        // ReadBlocks
+        {
+            auto request = volume.CreateReadBlocksRequest(
+                TBlockRange64::WithLength(2048, 1024),
+                clientInfo.GetClientId()
+            );
+            volume.SendToPipe(std::move(request));
+            auto response = volume.RecvReadBlocksResponse();
+            UNIT_ASSERT(response);
+            UNIT_ASSERT_VALUES_EQUAL(E_ARGUMENT, response->GetStatus());
+        }
+
+        // ReadBlocksLocal
+        {
+            TGuardedSgList list;
+            auto request = volume.CreateReadBlocksLocalRequest(
+                TBlockRange64::WithLength(2048, 1024),
+                list,
+                clientInfo.GetClientId()
+            );
+            volume.SendToPipe(std::move(request));
+            auto response = volume.RecvReadBlocksLocalResponse();
+            UNIT_ASSERT(response);
+            UNIT_ASSERT_VALUES_EQUAL(E_ARGUMENT, response->GetStatus());
+        }
+
+        // ZeroRequest
+        {
+            auto request = volume.CreateZeroBlocksRequest(
+                TBlockRange64::WithLength(1024 * 10, 1048),
+                clientInfo.GetClientId());
+            volume.SendToPipe(std::move(request));
+            auto response = volume.RecvZeroBlocksResponse();
+            UNIT_ASSERT(response);
+            UNIT_ASSERT_VALUES_EQUAL(E_ARGUMENT, response->GetStatus());
+        }
+    }
+
     Y_UNIT_TEST(ShouldReportLongRunningForBaseDisk)
     {
         NProto::TStorageServiceConfig storageServiceConfig;