On 19/07/2023 12:33, Gerd Hoffmann wrote:
Searching for an unused bounce buffer in mReservedMemBitmap and
reserving the buffer by flipping the bit is a critical section
which must not be interrupted. Raise the TPL level to ensure
that.
Without this fix it can happen that IoMmuDxe hands out the same
bounce buffer twice, causing trouble down the road. Seen happening
in practice with VirtioNetDxe setting up the network interface (and
calling into IoMmuDxe from a polling timer callback) in parallel with
Boot Manager doing some disk I/O. An ASSERT() in VirtioNet caught
the buffer inconsistency.
Full story with lots of details and discussions is available here:
https://bugzilla.redhat.com/show_bug.cgi?id=2211060
Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
---
OvmfPkg/IoMmuDxe/IoMmuBuffer.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/OvmfPkg/IoMmuDxe/IoMmuBuffer.c b/OvmfPkg/IoMmuDxe/IoMmuBuffer.c
index c8f6cf4818e8..7f8a0368ab5d 100644
--- a/OvmfPkg/IoMmuDxe/IoMmuBuffer.c
+++ b/OvmfPkg/IoMmuDxe/IoMmuBuffer.c
@@ -367,7 +367,9 @@ IoMmuAllocateBounceBuffer (
{
EFI_STATUS Status;
UINT32 ReservedMemBitmap;
+ EFI_TPL OldTpl;
+ OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
ReservedMemBitmap = 0;
Status = InternalAllocateBuffer (
Type,
@@ -378,6 +380,7 @@ IoMmuAllocateBounceBuffer (
);
MapInfo->ReservedMemBitmap = ReservedMemBitmap;
mReservedMemBitmap |= ReservedMemBitmap;
+ gBS->RestoreTPL (OldTpl);
ASSERT (Status == EFI_SUCCESS);
It looks as though IoMmuFreeBounceBuffer() should also raise to
TPL_NOTIFY while modifying mReservedMemBitmap, since the modification
made in IoMmuFreeBounceBuffer() is not an atomic operation:
mReservedMemBitmap &= (UINT32)(~MapInfo->ReservedMemBitmap);
Thanks,
Michael
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107056): https://edk2.groups.io/g/devel/message/107056
Mute This Topic: https://groups.io/mt/100233359/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-