[PATCH 08/11] OvmfPkg/XenBus: Change XENBUS_PROTOCOL to not return allocated memory

["Anthony PERARD" <anthony.perard@citrix.com>]

XsRead and XsBackendRead of the XENBUS_PROTOCOL return allocated
memory but this isn't allowed during the ExitBootServices call. We
need XsRead and XsBackendRead to disconnect from the device so
XENBUS_PROTOCOL is changed to use a buffer supplied by a child driver.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
 OvmfPkg/Include/Protocol/XenBus.h | 32 ++++++++++++----------
 OvmfPkg/XenBusDxe/XenStore.c      | 44 +++++-------------------------
 OvmfPkg/XenBusDxe/XenStore.h      |  6 +++--
 OvmfPkg/XenPvBlkDxe/BlockFront.c  | 45 ++++++++++++++++++-------------
 4 files changed, 54 insertions(+), 73 deletions(-)

diff --git a/OvmfPkg/Include/Protocol/XenBus.h b/OvmfPkg/Include/Protocol/XenBus.h
index 8ff5ca3575..c22bdfb368 100644
--- a/OvmfPkg/Include/Protocol/XenBus.h
+++ b/OvmfPkg/Include/Protocol/XenBus.h
@@ -35,6 +35,12 @@ typedef struct
 
 #define XST_NIL ((XENSTORE_TRANSACTION *) NULL)
 
+//
+// When reading a node from xenstore, if the size of the data to be read is
+// unknown, this value can be use for the size of the buffer.
+//
+#define XENSTORE_PAYLOAD_MAX 4096
+
 typedef enum {
   XENSTORE_STATUS_SUCCESS = 0,
   XENSTORE_STATUS_FAIL,
@@ -64,19 +70,17 @@ typedef enum {
 ///
 
 /**
-  Get the contents of the node Node of the PV device. Returns the contents in
-  *Result which should be freed after use.
+  Get the contents of the node Node of the PV device.
 
   @param This           A pointer to XENBUS_PROTOCOL instance.
   @param Transaction    The XenStore transaction covering this request.
   @param Node           The basename of the file to read.
-  @param Result         The returned contents from this file.
+  @param BufferSize     On input, a pointer to the size of the buffer at Buffer.
+                        On output, the size of the data written to Buffer.
+  @param Buffer         A pointer to a buffer into which the data read will be saved.
 
   @return  On success, XENSTORE_STATUS_SUCCESS. Otherwise an errno value
            indicating the type of failure.
-
-  @note The results buffer is malloced and should be free'd by the
-        caller.
 **/
 typedef
 XENSTORE_STATUS
@@ -84,23 +88,22 @@ XENSTORE_STATUS
   IN  XENBUS_PROTOCOL       *This,
   IN  CONST XENSTORE_TRANSACTION *Transaction,
   IN  CONST CHAR8           *Node,
-  OUT VOID                  **Result
+  IN OUT UINTN              *BufferSize,
+  OUT VOID                  *Buffer
   );
 
 /**
-  Get the contents of the node Node of the PV device's backend. Returns the
-  contents in *Result which should be freed after use.
+  Get the contents of the node Node of the PV device's backend.
 
   @param This           A pointer to XENBUS_PROTOCOL instance.
   @param Transaction    The XenStore transaction covering this request.
   @param Node           The basename of the file to read.
-  @param Result         The returned contents from this file.
+  @param BufferSize     On input, a pointer to the size of the buffer at Buffer.
+                        On output, the size of the data written to Buffer.
+  @param Buffer         A pointer to a buffer into which the data read will be saved.
 
   @return  On success, XENSTORE_STATUS_SUCCESS. Otherwise an errno value
            indicating the type of failure.
-
-  @note The results buffer is malloced and should be free'd by the
-        caller.
 **/
 typedef
 XENSTORE_STATUS
@@ -108,7 +111,8 @@ XENSTORE_STATUS
   IN  XENBUS_PROTOCOL       *This,
   IN  CONST XENSTORE_TRANSACTION *Transaction,
   IN  CONST CHAR8           *Node,
-  OUT VOID                  **Result
+  IN OUT UINTN              *BufferSize,
+  OUT VOID                  *Buffer
   );
 
 /**
diff --git a/OvmfPkg/XenBusDxe/XenStore.c b/OvmfPkg/XenBusDxe/XenStore.c
index b9588bb8c6..cb2d9e1215 100644
--- a/OvmfPkg/XenBusDxe/XenStore.c
+++ b/OvmfPkg/XenBusDxe/XenStore.c
@@ -1336,27 +1336,11 @@ XenBusXenStoreRead (
   IN  XENBUS_PROTOCOL       *This,
   IN  CONST XENSTORE_TRANSACTION *Transaction,
   IN  CONST CHAR8           *Node,
-  OUT VOID                  **Value
+  IN OUT UINTN              *BufferSize,
+  OUT VOID                  *Buffer
   )
 {
-  XENSTORE_STATUS Status;
-  UINTN           BufferSize;
-  VOID            *Buffer;
-
-  BufferSize = XENSTORE_PAYLOAD_MAX + 1;
-  Buffer = AllocatePool (BufferSize);
-  if (Buffer == NULL) {
-    return XENSTORE_STATUS_ENOMEM;
-  }
-
-  Status = XenStoreRead (Transaction, This->Node, Node, &BufferSize, Buffer);
-
-  if (Status == XENSTORE_STATUS_SUCCESS) {
-    *Value = Buffer;
-  } else {
-    FreePool (Buffer);
-  }
-  return Status;
+  return XenStoreRead (Transaction, This->Node, Node, BufferSize, Buffer);
 }
 
 XENSTORE_STATUS
@@ -1365,27 +1349,11 @@ XenBusXenStoreBackendRead (
   IN  XENBUS_PROTOCOL       *This,
   IN  CONST XENSTORE_TRANSACTION *Transaction,
   IN  CONST CHAR8           *Node,
-  OUT VOID                  **Value
+  IN OUT UINTN              *BufferSize,
+  OUT VOID                  *Buffer
   )
 {
-  XENSTORE_STATUS Status;
-  UINTN           BufferSize;
-  VOID            *Buffer;
-
-  BufferSize = XENSTORE_PAYLOAD_MAX + 1;
-  Buffer = AllocatePool (BufferSize);
-  if (Buffer == NULL) {
-    return XENSTORE_STATUS_ENOMEM;
-  }
-
-  Status = XenStoreRead (Transaction, This->Backend, Node, &BufferSize, Buffer);
-
-  if (Status == XENSTORE_STATUS_SUCCESS) {
-    *Value = Buffer;
-  } else {
-    FreePool (Buffer);
-  }
-  return Status;
+  return XenStoreRead (Transaction, This->Backend, Node, BufferSize, Buffer);
 }
 
 XENSTORE_STATUS
diff --git a/OvmfPkg/XenBusDxe/XenStore.h b/OvmfPkg/XenBusDxe/XenStore.h
index 13f7d132e6..ca8c080433 100644
--- a/OvmfPkg/XenBusDxe/XenStore.h
+++ b/OvmfPkg/XenBusDxe/XenStore.h
@@ -289,7 +289,8 @@ XenBusXenStoreRead (
   IN  XENBUS_PROTOCOL       *This,
   IN  CONST XENSTORE_TRANSACTION *Transaction,
   IN  CONST CHAR8           *Node,
-  OUT VOID                  **Value
+  IN OUT UINTN              *BufferSize,
+  OUT VOID                  *Buffer
   );
 
 XENSTORE_STATUS
@@ -298,7 +299,8 @@ XenBusXenStoreBackendRead (
   IN  XENBUS_PROTOCOL       *This,
   IN  CONST XENSTORE_TRANSACTION *Transaction,
   IN  CONST CHAR8           *Node,
-  OUT VOID                  **Value
+  IN OUT UINTN              *BufferSize,
+  OUT VOID                  *Buffer
   );
 
 XENSTORE_STATUS
diff --git a/OvmfPkg/XenPvBlkDxe/BlockFront.c b/OvmfPkg/XenPvBlkDxe/BlockFront.c
index 8dca4c82f0..25a398ccc4 100644
--- a/OvmfPkg/XenPvBlkDxe/BlockFront.c
+++ b/OvmfPkg/XenPvBlkDxe/BlockFront.c
@@ -41,19 +41,22 @@ XenBusReadUint64 (
   )
 {
   XENSTORE_STATUS Status;
-  CHAR8 *Ptr;
+  CHAR8           Buffer[22];
+  UINTN           BufferSize;
+
+  BufferSize = sizeof (Buffer) - 1;
 
   if (!FromBackend) {
-    Status = This->XsRead (This, XST_NIL, Node, (VOID**)&Ptr);
+    Status = This->XsRead (This, XST_NIL, Node, &BufferSize, Buffer);
   } else {
-    Status = This->XsBackendRead (This, XST_NIL, Node, (VOID**)&Ptr);
+    Status = This->XsBackendRead (This, XST_NIL, Node, &BufferSize, Buffer);
   }
   if (Status != XENSTORE_STATUS_SUCCESS) {
     return Status;
   }
+  Buffer[BufferSize] = '\0';
   // AsciiStrDecimalToUint64 will ASSERT if Ptr overflow UINT64.
-  *ValuePtr = AsciiStrDecimalToUint64 (Ptr);
-  FreePool (Ptr);
+  *ValuePtr = AsciiStrDecimalToUint64 (Buffer);
   return Status;
 }
 
@@ -143,43 +146,47 @@ XenPvBlockFrontInitialization (
   OUT XEN_BLOCK_FRONT_DEVICE  **DevPtr
   )
 {
-  XENSTORE_TRANSACTION Transaction;
-  CHAR8 *DeviceType;
-  blkif_sring_t *SharedRing;
-  XENSTORE_STATUS Status;
+  XENSTORE_TRANSACTION   Transaction;
+  CHAR8                  Buffer[XENSTORE_PAYLOAD_MAX + 1];
+  UINTN                  BufferSize;
+  blkif_sring_t          *SharedRing;
+  XENSTORE_STATUS        Status;
   XEN_BLOCK_FRONT_DEVICE *Dev;
-  XenbusState State;
-  UINT64 Value;
-  CHAR8 *Params;
+  XenbusState            State;
+  UINT64                 Value;
 
   ASSERT (NodeName != NULL);
 
+  BufferSize = sizeof (Buffer) - 1;
+
   Dev = AllocateZeroPool (sizeof (XEN_BLOCK_FRONT_DEVICE));
   Dev->Signature = XEN_BLOCK_FRONT_SIGNATURE;
   Dev->NodeName = NodeName;
   Dev->XenBusIo = XenBusIo;
   Dev->DeviceId = XenBusIo->DeviceId;
 
-  XenBusIo->XsRead (XenBusIo, XST_NIL, "device-type", (VOID**)&DeviceType);
-  if (AsciiStrCmp (DeviceType, "cdrom") == 0) {
+  BufferSize = sizeof (Buffer) - 1;
+  XenBusIo->XsRead (XenBusIo, XST_NIL, "device-type", &BufferSize, Buffer);
+  Buffer[BufferSize] = '\0';
+  if (AsciiStrCmp (Buffer, "cdrom") == 0) {
     Dev->MediaInfo.CdRom = TRUE;
   } else {
     Dev->MediaInfo.CdRom = FALSE;
   }
-  FreePool (DeviceType);
 
   if (Dev->MediaInfo.CdRom) {
-    Status = XenBusIo->XsBackendRead (XenBusIo, XST_NIL, "params", (VOID**)&Params);
+    BufferSize = sizeof (Buffer) - 1;
+    Status = XenBusIo->XsBackendRead (XenBusIo, XST_NIL, "params",
+      &BufferSize, Buffer);
     if (Status != XENSTORE_STATUS_SUCCESS) {
       DEBUG ((EFI_D_ERROR, "%a: Failed to read params (%d)\n", __FUNCTION__, Status));
       goto Error;
     }
-    if (AsciiStrLen (Params) == 0 || AsciiStrCmp (Params, "aio:") == 0) {
-      FreePool (Params);
+    Buffer[BufferSize] = '\0';
+    if (AsciiStrLen (Buffer) == 0 || AsciiStrCmp (Buffer, "aio:") == 0) {
       DEBUG ((EFI_D_INFO, "%a: Empty cdrom\n", __FUNCTION__));
       goto Error;
     }
-    FreePool (Params);
   }
 
   Status = XenBusReadUint64 (XenBusIo, "backend-id", FALSE, &Value);
-- 
Anthony PERARD