From: "Anthony PERARD" <anthony.perard@citrix.com>
To: <devel@edk2.groups.io>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Julien Grall <julien.grall@arm.com>,
Jordan Justen <jordan.l.justen@intel.com>,
<xen-devel@lists.xenproject.org>,
Anthony Perard <anthony.perard@citrix.com>,
Laszlo Ersek <lersek@redhat.com>
Subject: [PATCH 07/11] OvmfPkg/XenBusDxe: Use on stack buffer in internal functions
Date: Fri, 13 Sep 2019 15:50:56 +0100 [thread overview]
Message-ID: <20190913145100.303433-8-anthony.perard@citrix.com> (raw)
In-Reply-To: <20190913145100.303433-1-anthony.perard@citrix.com>
We will use a buffer on the stack instead of allocating memory for
internal functions that are expecting a reply from xenstore.
The external interface XENBUS_PROTOCOL isn't changed yet, so
allocation are made for XsRead and XsBackendRead.
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2190
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
OvmfPkg/XenBusDxe/XenBus.c | 40 ++++++------
OvmfPkg/XenBusDxe/XenStore.c | 115 ++++++++++++++++++++---------------
OvmfPkg/XenBusDxe/XenStore.h | 17 +++---
3 files changed, 95 insertions(+), 77 deletions(-)
diff --git a/OvmfPkg/XenBusDxe/XenBus.c b/OvmfPkg/XenBusDxe/XenBus.c
index bb8ddbc4d4..78835ec7b3 100644
--- a/OvmfPkg/XenBusDxe/XenBus.c
+++ b/OvmfPkg/XenBusDxe/XenBus.c
@@ -89,19 +89,18 @@ XenBusReadDriverState (
IN CONST CHAR8 *Path
)
{
- XenbusState State;
- CHAR8 *Ptr = NULL;
+ XenbusState State;
+ CHAR8 Buffer[4];
+ UINTN BufferSize;
XENSTORE_STATUS Status;
- Status = XenStoreRead (XST_NIL, Path, "state", NULL, (VOID **)&Ptr);
+ BufferSize = sizeof (Buffer) - 1;
+ Status = XenStoreRead (XST_NIL, Path, "state", &BufferSize, Buffer);
if (Status != XENSTORE_STATUS_SUCCESS) {
State = XenbusStateClosed;
} else {
- State = AsciiStrDecimalToUintn (Ptr);
- }
-
- if (Ptr != NULL) {
- FreePool (Ptr);
+ Buffer[BufferSize] = '\0';
+ State = AsciiStrDecimalToUintn (Buffer);
}
return State;
@@ -129,8 +128,11 @@ XenBusAddDevice (
if (XenStorePathExists (XST_NIL, DevicePath, "")) {
XENBUS_PRIVATE_DATA *Child;
- enum xenbus_state State;
- CHAR8 *BackendPath;
+ enum xenbus_state State;
+ CHAR8 BackendPath[XENSTORE_ABS_PATH_MAX + 1];
+ UINTN BackendPathSize;
+
+ BackendPathSize = sizeof (BackendPath);
Child = XenBusDeviceInitialized (Dev, DevicePath);
if (Child != NULL) {
@@ -155,17 +157,18 @@ XenBusAddDevice (
}
StatusXenStore = XenStoreRead (XST_NIL, DevicePath, "backend",
- NULL, (VOID **) &BackendPath);
+ &BackendPathSize, BackendPath);
if (StatusXenStore != XENSTORE_STATUS_SUCCESS) {
DEBUG ((EFI_D_ERROR, "xenbus: %a no backend path.\n", DevicePath));
Status = EFI_NOT_FOUND;
goto out;
}
+ BackendPath[BackendPathSize] = '\0';
Private = AllocateCopyPool (sizeof (*Private), &gXenBusPrivateData);
Private->XenBusIo.Type = AsciiStrDup (Type);
Private->XenBusIo.Node = AsciiStrDup (DevicePath);
- Private->XenBusIo.Backend = BackendPath;
+ Private->XenBusIo.Backend = AsciiStrDup (BackendPath);
Private->XenBusIo.DeviceId = (UINT16)AsciiStrDecimalToUintn (Id);
Private->Dev = Dev;
@@ -309,17 +312,20 @@ XenBusSetState (
)
{
enum xenbus_state CurrentState;
- XENSTORE_STATUS Status;
- CHAR8 *Temp;
+ XENSTORE_STATUS Status;
+ CHAR8 Buffer[4];
+ UINTN BufferSize;
+
+ BufferSize = sizeof (Buffer) - 1;
DEBUG ((EFI_D_INFO, "XenBus: Set state to %d\n", NewState));
- Status = XenStoreRead (Transaction, This->Node, "state", NULL, (VOID **)&Temp);
+ Status = XenStoreRead (Transaction, This->Node, "state", &BufferSize, Buffer);
if (Status != XENSTORE_STATUS_SUCCESS) {
goto Out;
}
- CurrentState = AsciiStrDecimalToUintn (Temp);
- FreePool (Temp);
+ Buffer[BufferSize] = '\0';
+ CurrentState = AsciiStrDecimalToUintn (Buffer);
if (CurrentState == NewState) {
goto Out;
}
diff --git a/OvmfPkg/XenBusDxe/XenStore.c b/OvmfPkg/XenBusDxe/XenStore.c
index 004d3b6022..b9588bb8c6 100644
--- a/OvmfPkg/XenBusDxe/XenStore.c
+++ b/OvmfPkg/XenBusDxe/XenStore.c
@@ -756,8 +756,9 @@ XenStoreGetError (
@param RequestType The type of message to send.
@param WriteRequest Pointers to the body sections of the request.
@param NumRequests The number of body sections in the request.
- @param LenPtr The returned length of the reply.
- @param ResultPtr The returned body of the reply.
+ @param BufferSize IN: size of the buffer
+ OUT: The returned length of the reply.
+ @param Buffer The returned body of the reply.
@return XENSTORE_STATUS_SUCCESS on success. Otherwise an errno indicating
the cause of failure.
@@ -769,15 +770,13 @@ XenStoreTalkv (
IN enum xsd_sockmsg_type RequestType,
IN CONST WRITE_REQUEST *WriteRequest,
IN UINT32 NumRequests,
- OUT UINT32 *LenPtr OPTIONAL,
- OUT VOID **ResultPtr OPTIONAL
+ IN OUT UINTN *BufferSize OPTIONAL,
+ OUT VOID *Buffer OPTIONAL
)
{
struct xsd_sockmsg Message;
UINTN Index;
XENSTORE_STATUS Status;
- VOID *Buffer;
- UINTN BufferSize;
if (Transaction == XST_NIL) {
Message.tx_id = 0;
@@ -805,32 +804,15 @@ XenStoreTalkv (
}
}
- if (ResultPtr) {
- Buffer = AllocatePool (XENSTORE_PAYLOAD_MAX + 1);
- BufferSize = XENSTORE_PAYLOAD_MAX;
- } else {
- Buffer = NULL;
- BufferSize = 0;
- }
-
//
// Wait for a reply to our request
//
Status = XenStoreProcessMessage (Message.req_id, Message.tx_id,
- &BufferSize, Buffer);
+ BufferSize, Buffer);
if (Status != XENSTORE_STATUS_SUCCESS) {
DEBUG ((DEBUG_ERROR, "XenStore, error while reading the ring (%d).\n",
Status));
- FreePool (Buffer);
- return Status;
- }
-
- if (ResultPtr) {
- *ResultPtr = Buffer;
- if (LenPtr) {
- *LenPtr = BufferSize;
- }
}
Error:
@@ -848,8 +830,9 @@ XenStoreTalkv (
@param RequestType The type of message to send.
@param Body The body of the request.
@param SubPath If !NULL and not "", "/$SubPath" is append to Body.
- @param LenPtr The returned length of the reply.
- @param Result The returned body of the reply.
+ @param BufferSize IN: sizef of the buffer
+ OUT: The returned length of the reply.
+ @param Buffer The returned body of the reply.
@return 0 on success. Otherwise an errno indicating
the cause of failure.
@@ -861,8 +844,8 @@ XenStoreSingle (
IN enum xsd_sockmsg_type RequestType,
IN CONST CHAR8 *Body,
IN CONST CHAR8 *SubPath OPTIONAL,
- OUT UINT32 *LenPtr OPTIONAL,
- OUT VOID **Result OPTIONAL
+ IN OUT UINTN *BufferSize OPTIONAL,
+ OUT VOID *Buffer OPTIONAL
)
{
WRITE_REQUEST WriteRequest[3];
@@ -870,7 +853,7 @@ XenStoreSingle (
XenStorePrepareWriteRequest (WriteRequest, Body, SubPath);
return XenStoreTalkv (Transaction, RequestType, WriteRequest, 3,
- LenPtr, Result);
+ BufferSize, Buffer);
}
//
@@ -1106,13 +1089,16 @@ XenStoreListDirectory (
OUT CONST CHAR8 ***DirectoryListPtr
)
{
- CHAR8 *TempStr;
- UINT32 Len = 0;
+ CHAR8 *TempStr;
+ UINTN Len;
XENSTORE_STATUS Status;
+ TempStr = AllocatePool (XENSTORE_PAYLOAD_MAX);
+ Len = XENSTORE_PAYLOAD_MAX;
Status = XenStoreSingle (Transaction, XS_DIRECTORY, DirectoryPath, Node, &Len,
- (VOID **) &TempStr);
+ TempStr);
if (Status != XENSTORE_STATUS_SUCCESS) {
+ FreePool (TempStr);
return Status;
}
@@ -1146,21 +1132,14 @@ XenStoreRead (
IN CONST XENSTORE_TRANSACTION *Transaction,
IN CONST CHAR8 *DirectoryPath,
IN CONST CHAR8 *Node,
- OUT UINT32 *LenPtr OPTIONAL,
- OUT VOID **Result
+ IN OUT UINTN *BufferSize,
+ OUT VOID *Buffer
)
{
- VOID *Value;
- XENSTORE_STATUS Status;
-
- Status = XenStoreSingle (Transaction, XS_READ, DirectoryPath, Node,
- LenPtr, &Value);
- if (Status != XENSTORE_STATUS_SUCCESS) {
- return Status;
- }
-
- *Result = Value;
- return XENSTORE_STATUS_SUCCESS;
+ ASSERT (BufferSize != NULL);
+ ASSERT (Buffer != NULL);
+ return XenStoreSingle (Transaction, XS_READ, DirectoryPath, Node,
+ BufferSize, Buffer);
}
XENSTORE_STATUS
@@ -1199,14 +1178,16 @@ XenStoreTransactionStart (
OUT XENSTORE_TRANSACTION *Transaction
)
{
- CHAR8 *IdStr;
+ CHAR8 IdStr[XENSTORE_PAYLOAD_MAX];
+ UINTN BufferSize;
XENSTORE_STATUS Status;
+ BufferSize = sizeof (IdStr);
+
Status = XenStoreSingle (XST_NIL, XS_TRANSACTION_START, "", NULL,
- NULL, (VOID **) &IdStr);
+ &BufferSize, IdStr);
if (Status == XENSTORE_STATUS_SUCCESS) {
Transaction->Id = (UINT32)AsciiStrDecimalToUintn (IdStr);
- FreePool (IdStr);
}
return Status;
@@ -1358,7 +1339,24 @@ XenBusXenStoreRead (
OUT VOID **Value
)
{
- return XenStoreRead (Transaction, This->Node, Node, NULL, Value);
+ 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;
}
XENSTORE_STATUS
@@ -1370,7 +1368,24 @@ XenBusXenStoreBackendRead (
OUT VOID **Value
)
{
- return XenStoreRead (Transaction, This->Backend, Node, NULL, Value);
+ 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;
}
XENSTORE_STATUS
diff --git a/OvmfPkg/XenBusDxe/XenStore.h b/OvmfPkg/XenBusDxe/XenStore.h
index effaad7336..13f7d132e6 100644
--- a/OvmfPkg/XenBusDxe/XenStore.h
+++ b/OvmfPkg/XenBusDxe/XenStore.h
@@ -64,29 +64,26 @@ XenStorePathExists (
);
/**
- Get the contents of a single "file". Returns the contents in *Result which
- should be freed after use. The length of the value in bytes is returned in
- *LenPtr.
+ Get the contents of a single "file". Copy the contents in Buffer if
+ provided. The length of the value in bytes is returned in *BufferSize.
@param Transaction The XenStore transaction covering this request.
@param DirectoryPath The dirname of the file to read.
@param Node The basename of the file to read.
- @param LenPtr The amount of data read.
- @param Result The returned contents from this file.
+ @param BufferSize IN: size of the buffer
+ OUT: The returned length of the reply.
+ @param Buffer The returned body of the reply.
@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.
**/
XENSTORE_STATUS
XenStoreRead (
IN CONST XENSTORE_TRANSACTION *Transaction,
IN CONST CHAR8 *DirectoryPath,
IN CONST CHAR8 *Node,
- OUT UINT32 *LenPtr OPTIONAL,
- OUT VOID **Result
+ IN OUT UINTN *BufferSize,
+ OUT VOID *Buffer
);
/**
--
Anthony PERARD
next prev parent reply other threads:[~2019-09-13 14:51 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-13 14:50 [PATCH 00/11] OvmfPkg/XenBusDxe: Fix ExitBootServices handler to avoid allocation Anthony PERARD
2019-09-13 14:50 ` [PATCH 01/11] OvmfPkg/XenBusDxe: Fix missing \n in DEBUG messages Anthony PERARD
2019-09-16 14:24 ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` [PATCH 02/11] OvmfPkg/XenBusDxe: Have XenStoreFindWatch take a pointer Anthony PERARD
2019-09-16 14:38 ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` [PATCH 03/11] OvmfPkg/XenBusDxe: Rework watch events reception Anthony PERARD
2019-09-16 14:39 ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` [PATCH 04/11] OvmfPkg/XenBusDxe: Avoid Allocate in XenStoreVSPrint Anthony PERARD
2019-09-16 14:45 ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` [PATCH 05/11] OvmfPkg/XenBusDxe: Construct paths without allocation Anthony PERARD
2019-09-16 15:39 ` [edk2-devel] " Laszlo Ersek
2019-09-16 15:43 ` Laszlo Ersek
2019-09-13 14:50 ` [PATCH 06/11] OvmfPkg/XenBusDxe: Rework XenStoreProcessMessage to avoid allocating memory Anthony PERARD
2019-09-16 15:41 ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` Anthony PERARD [this message]
2019-09-16 16:11 ` [edk2-devel] [PATCH 07/11] OvmfPkg/XenBusDxe: Use on stack buffer in internal functions Laszlo Ersek
2019-09-13 14:50 ` [PATCH 08/11] OvmfPkg/XenBus: Change XENBUS_PROTOCOL to not return allocated memory Anthony PERARD
2019-09-16 16:16 ` [edk2-devel] " Laszlo Ersek
2019-09-13 14:50 ` [PATCH 09/11] OvmfPkg/XenBusDxe: Fix NotifyExitBoot to avoid Memory Allocation Services Anthony PERARD
2019-09-16 17:36 ` [edk2-devel] " Laszlo Ersek
2019-09-16 18:36 ` Andrew Fish
2019-09-16 19:31 ` Laszlo Ersek
2019-09-16 20:50 ` Andrew Fish
2019-09-13 14:50 ` [PATCH 10/11] OvmfPkg/XenPvBlkDxe: Use XenBusIo->RegisterExitCallback Anthony PERARD
2019-09-13 14:51 ` [PATCH 11/11] OvmfPkg/XenBusDxe: Fix XenStoreWaitForEvent use during EBS Anthony PERARD
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190913145100.303433-8-anthony.perard@citrix.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox