[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] All items in SYSTEM_PROCESSOR array may not be initialized


  • To: <win-pv-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Owen Smith <owen.smith@xxxxxxxxxx>
  • Date: Mon, 28 Feb 2022 11:47:01 +0000
  • Authentication-results: esa4.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: Owen Smith <owen.smith@xxxxxxxxxx>
  • Delivery-date: Mon, 28 Feb 2022 11:47:20 +0000
  • Ironport-data: A9a23:LIHllatyiNPSIqZsLZerxGwrXOfnVDJeMUV32f8akzHdYApBsoF/q tZmKTyBM6uKNzD9LY0iPYq+pB8D7ZLSyNU2TwJo/Co8RHsV+JbJXdiXEBz9bniYRiHhoOOLz Cm8hv3odp1coqr0/0/1WlTZhSAgk/nOHNIQMcacUsxLbVYMpBwJ1FQyw4bVvqYy2YLjW1nX5 ouoyyHiEATNNwBcYzp8B52r8HuDjNyq0N/PlgVjDRzjlAa2e0g9VPrzF4noR5fLatA88tqBb /TC1NmEElbxpH/BPD8HfoHTKSXmSpaKVeSHZ+E/t6KK2nCurQRquko32WZ1he66RFxlkvgoo Oihu6BcRi8DOoaclsMsdiVCLD9mZKF44/z8HVmg5Jn7I03uKxMAwt1rBUAye4YZ5vx2ESdF8 vlwxDIlN07ZwbjsmfTiF7cq1p9LwMrDZevzvlln0jzfS+0jQIzDa67L+cVZzHE7gcUm8fP2O ZVCOGEzMEWojxtnJ0syOpMYtvqS3FajWmVprV2KmoE42j2GpOB2+Oe0a4eEEjCQfu1Kn0Ddv nyD82nnDxUyMN2E1SHD4n+qnvXIny7wRMQVDrLQyxJxqATNnCpJUkRQDAbl56ni4qKjZz5BA 2YQ0XogipkXyFaUUMGkUyOj40W4nwFJDrK8DNYGwA2Kz6PV5SOQCW4FUiNNZbQaiSMmedA5/ gTXxo20XFSDpJXQECvArenM8VteLABIdTdqWMMScecSDzAPSqkXhwmHcNtsGbXdYjbdSWCpm GDiQMTTatwuYS83O0eToAqvb9GE/MGhousJCuP/BD/NAuRRPtPNWmBQwQKHhcus1a7AJrV7g FAKmtKF8McFBoyXmSqGTY0lRe/1uqfVb2SF3QA3T/HNEghBHVb5IOi8BxkkeS9U3jssI2e1M Cc/RysLjHOsAJdaRfAuON/gYyjb5aPhCc7kRpjpgilmOfBMmPu81Hg2Pya4hjm1+GB1yP1XE crLIK6EUCdBYYw6nWXeegvo+eJyrszI7TiIHs6TItXO+ef2WUN5vp9eaAreNr1itfjcyOgXm v4GX/a3J9xkeLWWSkHqHUQ7dDjm8VBT6UjKlvFq
  • Ironport-hdrordr: A9a23:XAdinqxoBgKgwkeuPpmiKrPwIL1zdoMgy1knxilNoRw8SKKlfq eV7ZAmPH7P+VAssR4b+exoVJPtfZq+z+8R3WByB8bAYOCOggLBR+sO0WKL+UyGJ8SUzI9gPM lbHJSWcOeAb2RHsQ==
  • List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>

The SYSTEM_PROCESSOR array is allocated to fit the maximum number of supported
CPUs, but elements are only initialized when the SystemProcessorChangeCallback
callback is called with KeProcessorAddCompleteNotify.
Check if the SYSTEM_PROCESSOR structure is initialized before accessing any
other members, and fail SystemProcessorVcpuId with STATUS_NOT_SUPPORTED for any
uninitialized CPUs

Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>
---
 src/xen/system.c         | 21 ++++++++++++++++++++-
 src/xenbus/evtchn.c      | 22 ++++++++++++++++------
 src/xenbus/evtchn_fifo.c |  8 ++++++--
 src/xenbus/shared_info.c |  2 ++
 4 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/src/xen/system.c b/src/xen/system.c
index f85efc4..7220faa 100644
--- a/src/xen/system.c
+++ b/src/xen/system.c
@@ -54,6 +54,7 @@ typedef struct _SYSTEM_PROCESSOR {
     CHAR        Manufacturer[13];
     UCHAR       ApicID;
     UCHAR       ProcessorID;
+    BOOLEAN     Initialized;
     NTSTATUS    Status;
     KEVENT      Event;
     vcpu_info_t *Vcpu;
@@ -643,9 +644,14 @@ SystemProcessorVcpuId(
     if (Cpu >= Context->ProcessorCount)
         goto fail1;
 
+    status = STATUS_NOT_SUPPORTED;
+    if (!Processor->Initialized)
+        goto fail2;
+
     *vcpu_id = Processor->ProcessorID;
     return STATUS_SUCCESS;
 
+fail2:
 fail1:
     return status;
 }
@@ -666,14 +672,18 @@ SystemProcessorVcpuInfo(
         goto fail1;
 
     status = STATUS_NOT_SUPPORTED;
-    if (Processor->Registered == NULL)
+    if (!Processor->Initialized)
         goto fail2;
 
+    if (Processor->Registered == NULL)
+        goto fail3;
+
     ASSERT(*Processor->Registered);
     *Vcpu = Processor->Vcpu;
 
     return STATUS_SUCCESS;
 
+fail3:
 fail2:
 fail1:
     return status;
@@ -702,6 +712,8 @@ SystemProcessorRegisterVcpuInfo(
     if (Cpu >= Context->ProcessorCount)
         goto fail1;
 
+    ASSERT(Processor->Initialized);
+
     ASSERT(Mdl->MdlFlags & MDL_MAPPED_TO_SYSTEM_VA);
     MdlMappedSystemVa = Mdl->MappedSystemVa;
 
@@ -864,6 +876,7 @@ SystemProcessorChangeCallback(
         Processor = &Context->Processor[Cpu];
 
         KeInitializeEvent(&Processor->Event, NotificationEvent, FALSE);
+        Processor->Initialized = TRUE;
 
         KeInitializeDpc(&Processor->Dpc, SystemProcessorDpc, NULL);
         KeSetImportanceDpc(&Processor->Dpc, HighImportance);
@@ -991,9 +1004,12 @@ SystemDeregisterProcessorChangeCallback(
     for (Cpu = 0; Cpu < Context->ProcessorCount; Cpu++) {
         PSYSTEM_PROCESSOR   Processor = &Context->Processor[Cpu];
 
+        // Should check Processor->Initialized, but these operations are 
harmless to
+        // uninitialized SYSTEM_PROCESSOR structures.
         SystemProcessorDeregisterVcpuInfo(Cpu);
         SystemProcessorTeardown(Cpu);
 
+        Processor->Initialized = FALSE;
         RtlZeroMemory(&Processor->Dpc, sizeof (KDPC));
         RtlZeroMemory(&Processor->Event, sizeof (KEVENT));
         Processor->Status = 0;
@@ -1215,6 +1231,9 @@ SystemCheckProcessors(
     {
         PSYSTEM_PROCESSOR   Processor = &Context->Processor[Cpu];
 
+        if (!Processor->Initialized)
+            continue;
+
         (VOID) KeWaitForSingleObject(&Processor->Event,
                                      Executive,
                                      KernelMode,
diff --git a/src/xenbus/evtchn.c b/src/xenbus/evtchn.c
index 8942cdf..ccc8f2e 100644
--- a/src/xenbus/evtchn.c
+++ b/src/xenbus/evtchn.c
@@ -284,11 +284,12 @@ EvtchnOpenVirq(
         goto fail1;
 
     status = SystemProcessorVcpuId(Cpu, &vcpu_id);
-    ASSERT(NT_SUCCESS(status));
+    if (!NT_SUCCESS(status))
+        goto fail2;
 
     status = EventChannelBindVirq(Index, vcpu_id, &LocalPort);
     if (!NT_SUCCESS(status))
-        goto fail2;
+        goto fail3;
 
     Channel->Parameters.Virq.Index = Index;
 
@@ -296,6 +297,9 @@ EvtchnOpenVirq(
 
     return STATUS_SUCCESS;
 
+fail3:
+    Error("fail3\n");
+
 fail2:
     Error("fail2\n");
 
@@ -769,11 +773,12 @@ EvtchnBind(
     LocalPort = Channel->LocalPort;
 
     status = SystemProcessorVcpuId(Cpu, &vcpu_id);
-    ASSERT(NT_SUCCESS(status));
+    if (!NT_SUCCESS(status))
+        goto fail2;
 
     status = EventChannelBindVirtualCpu(LocalPort, vcpu_id);
     if (!NT_SUCCESS(status))
-        goto fail2;
+        goto fail3;
 
     Channel->Cpu = Cpu;
 
@@ -784,6 +789,9 @@ done:
 
     return STATUS_SUCCESS;
 
+fail3:
+    Error("fail3\n");
+
 fail2:
     Error("fail2\n");
 
@@ -1292,7 +1300,8 @@ EvtchnInterruptEnable(
             continue;
 
         status = SystemProcessorVcpuId(Cpu, &vcpu_id);
-        ASSERT(NT_SUCCESS(status));
+        if (!NT_SUCCESS(status))
+            continue;
 
         Vector = FdoGetInterruptVector(Context->Fdo, Processor->Interrupt);
 
@@ -1359,7 +1368,8 @@ EvtchnInterruptDisable(
             continue;
 
         status = SystemProcessorVcpuId(Cpu, &vcpu_id);
-        ASSERT(NT_SUCCESS(status));
+        if (!NT_SUCCESS(status))
+            continue;
 
         (VOID) HvmSetEvtchnUpcallVector(vcpu_id, 0);
         Processor->UpcallEnabled = FALSE;
diff --git a/src/xenbus/evtchn_fifo.c b/src/xenbus/evtchn_fifo.c
index 3b3f493..a8dab8c 100644
--- a/src/xenbus/evtchn_fifo.c
+++ b/src/xenbus/evtchn_fifo.c
@@ -514,13 +514,14 @@ EvtchnFifoAcquire(
             goto fail1;
 
         status = SystemProcessorVcpuId(Index, &vcpu_id);
-        ASSERT(NT_SUCCESS(status));
+        if (!NT_SUCCESS(status))
+            goto fail2;
 
         Pfn = MmGetMdlPfnArray(Mdl)[0];
 
         status = EventChannelInitControl(Pfn, vcpu_id);
         if (!NT_SUCCESS(status))
-            goto fail2;
+            goto fail3;
 
         Address.QuadPart = (ULONGLONG)Pfn << PAGE_SHIFT;
 
@@ -542,6 +543,9 @@ done:
 
     return STATUS_SUCCESS;
 
+fail3:
+    Error("fail3\n");
+
 fail2:
     Error("fail2\n");
 
diff --git a/src/xenbus/shared_info.c b/src/xenbus/shared_info.c
index 9f11979..786ffcf 100644
--- a/src/xenbus/shared_info.c
+++ b/src/xenbus/shared_info.c
@@ -639,6 +639,8 @@ SharedInfoAcquire(
         Processor = &Context->Processor[Index];
 
         status = SystemProcessorVcpuId(Index, &Processor->vcpu_id);
+        if (status == STATUS_NOT_SUPPORTED)
+            continue;
         if (!NT_SUCCESS(status))
             goto fail7;
 
-- 
2.33.0.windows.2




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.