summaryrefslogtreecommitdiffstats
path: root/OvmfPkg
diff options
context:
space:
mode:
authorSami Mujawar <sami.mujawar@arm.com>2024-04-15 09:46:29 +0100
committermergify[bot] <37929162+mergify[bot]@users.noreply.github.com>2024-07-31 14:09:34 +0000
commitb342070ce63075e7691cbd086ef8567cb02c372e (patch)
tree02e379524dbdd1c712540e4a02952f2370f2e507 /OvmfPkg
parent02f7ecbbb2da356585f5c4df4a5e7aa64a6b985d (diff)
downloadedk2-b342070ce63075e7691cbd086ef8567cb02c372e.tar.gz
edk2-b342070ce63075e7691cbd086ef8567cb02c372e.tar.bz2
edk2-b342070ce63075e7691cbd086ef8567cb02c372e.zip
OvmfPkg: Use heap memory for virtio-blk request
The storage space for virtio-blk request header being shared with the host was from the stack as the request structure was a local function variable. A bug in the VMM can corrupt the stack space, and such issues can be very hard to debug. Note: This is only an issue with a normal guest VM (non-CCA). A CCA guest VM would perform bounce buffering for sharing the data and therefore not have this issue. Instead of using the stack for sharing the data with the host, memory can be allocated from the heap pool. However, pool allocations are not any safer in terms of pages being shared between different allocations, and so mapping a pool allocation for DMA may expose it to potential corruption by the VMM in exactly the same way. The only difference is the potential impact on program behaviour, which is much higher with the stack. Additionally, for guest-side corruption heap allocations can take advantage by turning on heap guard to help find the bug. Therefore, minor improvement can be achieved by allocating memory for the virtio-blk request header from the heap for sharing with the host. Cc: Ard Biesheuvel <ardb+tianocore@kernel.org> Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Diffstat (limited to 'OvmfPkg')
-rw-r--r--OvmfPkg/VirtioBlkDxe/VirtioBlk.c30
1 files changed, 20 insertions, 10 deletions
diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
index 74ed52f9da..f881cde4c5 100644
--- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
+++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
@@ -13,6 +13,7 @@
Copyright (C) 2012, Red Hat, Inc.
Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2017, AMD Inc, All rights reserved.<BR>
+ Copyright (c) 2024, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -240,7 +241,7 @@ SynchronousRequest (
)
{
UINT32 BlockSize;
- volatile VIRTIO_BLK_REQ Request;
+ volatile VIRTIO_BLK_REQ *Request;
volatile UINT8 *HostStatus;
VOID *HostStatusBuffer;
DESC_INDICES Indices;
@@ -273,15 +274,20 @@ SynchronousRequest (
//
ASSERT (BufferSize % BlockSize == 0);
+ Request = AllocateZeroPool (sizeof (*Request));
+ if (Request == NULL) {
+ return EFI_DEVICE_ERROR;
+ }
+
//
// Prepare virtio-blk request header, setting zero size for flush.
// IO Priority is homogeneously 0.
//
- Request.Type = RequestIsWrite ?
- (BufferSize == 0 ? VIRTIO_BLK_T_FLUSH : VIRTIO_BLK_T_OUT) :
- VIRTIO_BLK_T_IN;
- Request.IoPrio = 0;
- Request.Sector = MultU64x32 (Lba, BlockSize / 512);
+ Request->Type = RequestIsWrite ?
+ (BufferSize == 0 ? VIRTIO_BLK_T_FLUSH : VIRTIO_BLK_T_OUT) :
+ VIRTIO_BLK_T_IN;
+ Request->IoPrio = 0;
+ Request->Sector = MultU64x32 (Lba, BlockSize / 512);
//
// Host status is bi-directional (we preset with a value and expect the
@@ -294,7 +300,8 @@ SynchronousRequest (
&HostStatusBuffer
);
if (EFI_ERROR (Status)) {
- return EFI_DEVICE_ERROR;
+ Status = EFI_DEVICE_ERROR;
+ goto FreeBlkRequest;
}
HostStatus = HostStatusBuffer;
@@ -306,8 +313,8 @@ SynchronousRequest (
Status = VirtioMapAllBytesInSharedBuffer (
Dev->VirtIo,
VirtioOperationBusMasterRead,
- (VOID *)&Request,
- sizeof Request,
+ (VOID *)Request,
+ sizeof (*Request),
&RequestDeviceAddress,
&RequestMapping
);
@@ -372,7 +379,7 @@ SynchronousRequest (
VirtioAppendDesc (
&Dev->Ring,
RequestDeviceAddress,
- sizeof Request,
+ sizeof (*Request),
VRING_DESC_F_NEXT,
&Indices
);
@@ -454,6 +461,9 @@ FreeHostStatusBuffer:
HostStatusBuffer
);
+FreeBlkRequest:
+ FreePool ((VOID *)Request);
+
return Status;
}