[PATCH 4/6] Centralize VBD extent checking

[Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>]

Introduce a DiskInfoIsValidExtent function that checks if a LBA range
is within the virtual disk's sector count.

Replace the inlined checks in target.c and ring.c with this function.

Add an extent check in TargetSyncCache.

Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
---
 src/xenvbd/frontend.h | 26 ++++++++++++++++++++++++++
 src/xenvbd/ring.c     | 18 +++++++++++-------
 src/xenvbd/target.c   | 32 +++++++++++++++++---------------
 3 files changed, 54 insertions(+), 22 deletions(-)

diff --git a/src/xenvbd/frontend.h b/src/xenvbd/frontend.h
index 3c304f0..6c7665d 100644
--- a/src/xenvbd/frontend.h
+++ b/src/xenvbd/frontend.h
@@ -34,6 +34,7 @@
 #define _XENVBD_FRONTEND_H
 
 #include <ntddk.h>
+#include <limits.h>
 
 // This is the fixed size, in bytes, of a sector used in the blkif protocol.
 #define BLKIF_SECTOR_SHIFT      9
@@ -175,4 +176,29 @@ FRONTEND_GET_PROPERTY(DiskInfo, PXENVBD_DISKINFO)
 
 #undef FRONTEND_GET_PROPERTY
 
+static inline BOOLEAN
+DiskInfoIsValidExtent(
+    _In_ PXENVBD_DISKINFO   DiskInfo,
+    _In_ ULONG64            StartLBA,
+    _In_ ULONG64            CountLBA
+    )
+{
+    ULONG64                 BlkifSectorStart;
+    ULONG64                 BlkifSectorEnd;
+
+    if (StartLBA > (ULLONG_MAX >> DiskInfo->SectorShift) ||
+        CountLBA > (ULLONG_MAX >> DiskInfo->SectorShift))
+        return FALSE;
+
+    BlkifSectorStart = StartLBA << DiskInfo->SectorShift;
+    BlkifSectorEnd = BlkifSectorStart + (CountLBA << DiskInfo->SectorShift);
+    if (BlkifSectorEnd < BlkifSectorStart)
+        return FALSE;
+
+    if (BlkifSectorEnd > DiskInfo->BlkifSectorCount)
+        return FALSE;
+
+    return TRUE;
+}
+
 #endif // _XENVBD_FRONTEND_H
diff --git a/src/xenvbd/ring.c b/src/xenvbd/ring.c
index 2a4eb56..10ca6ee 100644
--- a/src/xenvbd/ring.c
+++ b/src/xenvbd/ring.c
@@ -846,12 +846,12 @@ BlkifRingPrepareUnmap(
 {
     PXENVBD_RING            Ring = BlkifRing->Ring;
     PXENVBD_FRONTEND        Frontend = Ring->Frontend;
+    PXENVBD_DISKINFO        DiskInfo = FrontendGetDiskInfo(Frontend);
     PSCSI_REQUEST_BLOCK     Srb = SrbExt->Srb;
     PUNMAP_LIST_HEADER      Unmap = Srb->DataBuffer;
     ULONG                   Count;
     ULONG                   Index;
     LIST_ENTRY              List;
-    const ULONG             SectorShift = 
FrontendGetDiskInfo(Frontend)->SectorShift;
 
     InitializeListHead(&List);
 
@@ -863,8 +863,8 @@ BlkifRingPrepareUnmap(
     for (Index = 0; Index < Count; ++Index) {
         PUNMAP_BLOCK_DESCRIPTOR Descr = &Unmap->Descriptors[Index];
         PXENVBD_REQUEST         Request;
-        ULONG64                 FirstSector;
-        ULONG                   NrSectors;
+        ULONG64                 StartLBA;
+        ULONG                   CountLBA;
 
         Request = BlkifRingGetRequest(BlkifRing);
         if (Request == NULL)
@@ -872,19 +872,23 @@ BlkifRingPrepareUnmap(
         InsertTailList(&List, &Request->ListEntry);
         SrbExt->RequestCount++;
 
-        FirstSector = _byteswap_uint64(*(PULONG64)Descr->StartingLba) << 
SectorShift;
-        NrSectors = _byteswap_ulong(*(PULONG)Descr->LbaCount) << SectorShift;
+        StartLBA = _byteswap_uint64(*(PULONG64)Descr->StartingLba);
+        CountLBA = _byteswap_ulong(*(PULONG)Descr->LbaCount);
+
+        if (!DiskInfoIsValidExtent(DiskInfo, StartLBA, CountLBA))
+            goto fail2;
 
         Request->SrbExt = SrbExt;
         Request->Operation = BLKIF_OP_DISCARD;
-        Request->FirstSector = FirstSector;
-        Request->NrSectors = NrSectors;
+        Request->FirstSector = StartLBA << DiskInfo->SectorShift;
+        Request->NrSectors = CountLBA << DiskInfo->SectorShift;
         Request->Flags = 0;
     }
 
     BlkifRingQueueRequests(BlkifRing, &List);
     return STATUS_SUCCESS;
 
+fail2:
 fail1:
     BlkifRingUnprepareRequest(BlkifRing, &List);
     SrbExt->RequestCount = 0;
diff --git a/src/xenvbd/target.c b/src/xenvbd/target.c
index 128ae63..ce688f5 100644
--- a/src/xenvbd/target.c
+++ b/src/xenvbd/target.c
@@ -218,10 +218,7 @@ TargetReadWrite(
     PXENVBD_SRBEXT          SrbExt = Srb->SrbExtension;
     PXENVBD_FRONTEND        Frontend = Target->Frontend;
     PXENVBD_RING            Ring = FrontendGetRing(Frontend);
-    PXENVBD_DISKINFO        DiskInfo = FrontendGetDiskInfo(Target->Frontend);
-    ULONG64                 SectorCount;
-    ULONG64                 SectorStart;
-    ULONG                   NumSectors;
+    PXENVBD_DISKINFO        DiskInfo = FrontendGetDiskInfo(Frontend);
 
     Srb->SrbStatus = SRB_STATUS_ERROR;
     if (!FrontendGetCaps(Frontend)->Connected)
@@ -232,21 +229,15 @@ TargetReadWrite(
         Cdb_OperationEx(Srb) == SCSIOP_WRITE)
         goto fail2;
 
-    // check Sectors requested is on the disk
-    SectorCount = DiskInfo->BlkifSectorCount;
-    SectorStart = Cdb_LogicalBlock(Srb) << DiskInfo->SectorShift;
-    NumSectors = Cdb_TransferBlock(Srb) << DiskInfo->SectorShift;
-
-    if (SectorStart >= SectorCount)
+    Srb->SrbStatus = SRB_STATUS_INVALID_REQUEST;
+    if (!DiskInfoIsValidExtent(DiskInfo,
+                               Cdb_LogicalBlock(Srb),
+                               Cdb_TransferBlock(Srb)))
         goto fail3;
-    if ((SectorStart + NumSectors) > SectorCount)
-        goto fail4;
 
     Srb->SrbStatus = SRB_STATUS_PENDING;
     return RingQueueRequest(Ring, SrbExt);
 
-fail4:
-     Error("fail4\n");
 fail3:
     Error("fail3\n");
 fail2:
@@ -265,14 +256,21 @@ TargetSyncCache(
     PXENVBD_SRBEXT          SrbExt = Srb->SrbExtension;
     PXENVBD_FRONTEND        Frontend = Target->Frontend;
     PXENVBD_RING            Ring = FrontendGetRing(Frontend);
+    PXENVBD_DISKINFO        DiskInfo = FrontendGetDiskInfo(Frontend);
 
     Srb->SrbStatus = SRB_STATUS_ERROR;
     if (!FrontendGetCaps(Frontend)->Connected)
         goto fail1;
 
-    if (FrontendGetDiskInfo(Frontend)->DiskInfo & VDISK_READONLY)
+    if (DiskInfo->DiskInfo & VDISK_READONLY)
         goto fail2;
 
+    Srb->SrbStatus = SRB_STATUS_INVALID_REQUEST;
+    if (!DiskInfoIsValidExtent(DiskInfo,
+                               Cdb_LogicalBlock(Srb),
+                               Cdb_TransferBlock(Srb)))
+        goto fail3;
+
     // If neither FLUSH or BARRIER is supported, just succceed the SRB
     if (!(FrontendGetFeatures(Frontend)->FlushCache ||
           FrontendGetFeatures(Frontend)->Barrier))
@@ -285,6 +283,8 @@ succeed:
     Srb->SrbStatus = SRB_STATUS_SUCCESS;
     return FALSE; // not-queued
 
+fail3:
+    Error("fail3\n");
 fail2:
     Error("fail2\n");
 fail1:
@@ -309,6 +309,8 @@ TargetUnmap(
     if (FrontendGetDiskInfo(Frontend)->DiskInfo & VDISK_READONLY)
         goto fail2;
 
+    // Extents are checked in BlkifRingPrepareUnmap.
+
     if (!FrontendGetFeatures(Frontend)->Discard)
         goto succeed;
 
-- 
2.54.0.windows.1



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech