On Wed, 19 Jul 2023 at 18:31, Gerd Hoffmann <kra...@redhat.com> 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
>
> v2:
> - add locking to IoMmuFreeBounceBuffer too, clearing bits in
> mReservedMemBitmap is not guaranteed to be atomic (Michael Brown).
>
Please put this under the --- so I don't have to remove manually it
when applying.
> Signed-off-by: Gerd Hoffmann <kra...@redhat.com>
Pushed as #4665
Thanks,
> ---
> OvmfPkg/IoMmuDxe/IoMmuBuffer.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/OvmfPkg/IoMmuDxe/IoMmuBuffer.c b/OvmfPkg/IoMmuDxe/IoMmuBuffer.c
> index c8f6cf4818e8..103003cae376 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);
>
> @@ -395,6 +398,8 @@ IoMmuFreeBounceBuffer (
> IN OUT MAP_INFO *MapInfo
> )
> {
> + EFI_TPL OldTpl;
> +
> if (MapInfo->ReservedMemBitmap == 0) {
> gBS->FreePages (MapInfo->PlainTextAddress, MapInfo->NumberOfPages);
> } else {
> @@ -407,9 +412,11 @@ IoMmuFreeBounceBuffer (
> mReservedMemBitmap,
> mReservedMemBitmap & ((UINT32)(~MapInfo->ReservedMemBitmap))
> ));
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> MapInfo->PlainTextAddress = 0;
> mReservedMemBitmap &= (UINT32)(~MapInfo->ReservedMemBitmap);
> MapInfo->ReservedMemBitmap = 0;
> + gBS->RestoreTPL (OldTpl);
> }
>
> return EFI_SUCCESS;
> --
> 2.41.0
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#107063): https://edk2.groups.io/g/devel/message/107063
Mute This Topic: https://groups.io/mt/100238846/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
