[win-pv-devel] [PATCH] Alloc/Free memory for FrontendPath and TargetPath

[Owen Smith <owen.smith@xxxxxxxxxx>]

Using a static sized buffer for FrontendPath triggered failures when
the DeviceId is longer than 8 characters, as the RtlStringCbPrintfA call
failed to prefent a buffer overflow.

Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
---
 src/xenvbd/frontend.c | 63 +++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 49 insertions(+), 14 deletions(-)

diff --git a/src/xenvbd/frontend.c b/src/xenvbd/frontend.c
index 3fa19a0..f6df88d 100644
--- a/src/xenvbd/frontend.c
+++ b/src/xenvbd/frontend.c
@@ -62,9 +62,9 @@ struct _XENVBD_FRONTEND {
     PXENVBD_TARGET              Target;
     ULONG                       TargetId;
     ULONG                       DeviceId;
-    CHAR                        FrontendPath[sizeof("device/vbd/XXXXXXXX")];
+    PCHAR                       FrontendPath;
     PCHAR                       BackendPath;
-    CHAR                        TargetPath[sizeof("data/scsi/target/XXXX")];
+    PCHAR                       TargetPath;
     USHORT                      BackendDomain;
     XENVBD_STATE                State;
     KSPIN_LOCK                  StateLock;
@@ -1615,7 +1615,7 @@ FrontendDebugCallback(
     XENBUS_DEBUG(Printf,
                  &Frontend->DebugInterface,
                  "FrontendPath: %s\n",
-                 Frontend->FrontendPath);
+                 Frontend->FrontendPath ? Frontend->FrontendPath : "NULL");
     XENBUS_DEBUG(Printf,
                  &Frontend->DebugInterface,
                  "BackendPath: %s\n",
@@ -1623,7 +1623,7 @@ FrontendDebugCallback(
     XENBUS_DEBUG(Printf,
                  &Frontend->DebugInterface,
                  "TargetPath: %s\n",
-                 Frontend->TargetPath);
+                 Frontend->TargetPath ? Frontend->TargetPath : "NULL");
     XENBUS_DEBUG(Printf,
                  &Frontend->DebugInterface,
                  "State: %s\n",
@@ -1879,6 +1879,7 @@ FrontendCreate(
     )
 {
     PXENVBD_FRONTEND        Frontend;
+    ULONG                   Size;
     NTSTATUS                status;
 
     Trace("Target[%d] @ (%d) =====>\n", TargetId, KeGetCurrentIrql());
@@ -1902,31 +1903,51 @@ FrontendCreate(
             __FrontendGetTargetId(Frontend),
             Frontend->MaxQueues);
 
+    Size = (ULONG)(strlen("device/vbd/") +
+                   strlen(DeviceId) +
+                   1) * sizeof(CHAR);
+
+    Frontend->FrontendPath = __FrontendAlloc(Size);
+
+    status = STATUS_NO_MEMORY;
+    if (Frontend->FrontendPath == NULL)
+        goto fail2;
+
     status = RtlStringCbPrintfA(Frontend->FrontendPath,
-                                sizeof(Frontend->FrontendPath),
+                                Size,
                                 "device/vbd/%u",
                                 Frontend->DeviceId);
     if (!NT_SUCCESS(status))
-        goto fail2;
+        goto fail3;
+
+    Size = (ULONG)(strlen("data/scsi/target/") +
+                   strlen("XXXX") +
+                   1) * sizeof(CHAR);
+
+    Frontend->TargetPath = __FrontendAlloc(Size);
+
+    status = STATUS_NO_MEMORY;
+    if (Frontend->TargetPath == NULL)
+        goto fail4;
 
     status = RtlStringCbPrintfA(Frontend->TargetPath,
                                 sizeof(Frontend->TargetPath),
                                 "data/scsi/target/%u",
                                 TargetId);
     if (!NT_SUCCESS(status))
-        goto fail3;
+        goto fail5;
 
     status = RingCreate(Frontend, &Frontend->Ring);
     if (!NT_SUCCESS(status))
-        goto fail4;
+        goto fail6;
 
     status = GranterCreate(Frontend, &Frontend->Granter);
     if (!NT_SUCCESS(status))
-        goto fail5;
+        goto fail7;
 
     status = ThreadCreate(FrontendBackend, Frontend, &Frontend->BackendThread);
     if (!NT_SUCCESS(status))
-        goto fail6;
+        goto fail8;
 
     // kernel objects
     KeInitializeSpinLock(&Frontend->StateLock);
@@ -1935,18 +1956,26 @@ FrontendCreate(
     *_Frontend = Frontend;
     return STATUS_SUCCESS;
 
-fail6:
-    Error("fail6\n");
+fail8:
+    Error("fail8\n");
     GranterDestroy(Frontend->Granter);
     Frontend->Granter = NULL;
-fail5:
-    Error("fail5\n");
+fail7:
+    Error("fail7\n");
     RingDestroy(Frontend->Ring);
     Frontend->Ring = NULL;
+fail6:
+    Error("fail6\n");
+fail5:
+    Error("fail5\n");
+    __FrontendFree(Frontend->TargetPath);
+    Frontend->TargetPath = NULL;
 fail4:
     Error("fail4\n");
 fail3:
     Error("fail3\n");
+    __FrontendFree(Frontend->FrontendPath);
+    Frontend->FrontendPath = NULL;
 fail2:
     Error("Fail2\n");
     Frontend->Target = NULL;
@@ -1991,6 +2020,12 @@ FrontendDestroy(
     RingDestroy(Frontend->Ring);
     Frontend->Ring = NULL;
 
+    __FrontendFree(Frontend->TargetPath);
+    Frontend->TargetPath = NULL;
+
+    __FrontendFree(Frontend->FrontendPath);
+    Frontend->FrontendPath = NULL;
+
     Frontend->MaxQueues = 0;
 
     ASSERT3P(Frontend->BackendPath, ==, NULL);
-- 
2.16.2.windows.1


_______________________________________________
win-pv-devel mailing list
win-pv-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/win-pv-devel