[08/11] OvmfPkg/XenBus: Change XENBUS_PROTOCOL to not return allocated memory
diff mbox series

Message ID 20190913145100.303433-9-anthony.perard@citrix.com
State New
Headers show
Series
  • OvmfPkg/XenBusDxe: Fix ExitBootServices handler to avoid allocation
Related show

Commit Message

Anthony PERARD Sept. 13, 2019, 2:50 p.m. UTC
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(-)

Comments

Laszlo Ersek Sept. 16, 2019, 4:16 p.m. UTC | #1
On 09/13/19 16:50, Anthony PERARD wrote:
> 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
> +

This macro is already defined in "IndustryStandard/Xen/io/xs_wire.h".
Can we get it from there?

The extra documentation is OK, of course (replacing "this value" with
"XENSTORE_PAYLOAD_MAX").

Other than that, I'm going to have to ACK this after a brief skim only.

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo

>  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);
>

Patch
diff mbox series

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);