From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 09617941CB7 for ; Wed, 19 Jul 2023 16:04:33 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=NJA/zS9tq8pqebExjCHcNqsXEuc/LkzJgIkBCinfnU0=; c=relaxed/simple; d=groups.io; h=X-Received:X-Received:Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:X-Spam-Status:X-Spam-Checker-Version:Feedback-ID:X-SES-Outgoing:Precedence:List-Unsubscribe:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:X-Gm-Message-State:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1689782672; v=1; b=iPA+5PYrJO93lJbz9gMREP7we704Z/kdVfRkDdf1veZxLeARwTqJYf2sIpBAJG8C5VM5yHgZ w4t6obglxNDaKKYq0H+AgqcJ2TS9GahQ6R9S0C5Yj0/SS5hpZ+HaT9oZI/UCB1xDQWRyPlTTyVW gebczVKyEUgI70ZyOKv8nKd0= X-Received: by 127.0.0.2 with SMTP id 4BnEYY7687511xEay0FG8x8U; Wed, 19 Jul 2023 09:04:32 -0700 X-Received: from a7-18.smtp-out.eu-west-1.amazonses.com (a7-18.smtp-out.eu-west-1.amazonses.com [54.240.7.18]) by mx.groups.io with SMTP id smtpd.web11.1171.1689782670991331712 for ; Wed, 19 Jul 2023 09:04:31 -0700 Message-ID: <010201896ee54d34-2b5c712c-d799-49b0-a2eb-f0838988b313-000000@eu-west-1.amazonses.com> Date: Wed, 19 Jul 2023 16:04:28 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [edk2-devel] [PATCH 1/1] OvmfPkg/IoMmuDxe: add locking to IoMmuAllocateBounceBuffer To: devel@edk2.groups.io, kraxel@redhat.com Cc: Ard Biesheuvel , Jiewen Yao , Jordan Justen References: <20230719113317.276124-1-kraxel@redhat.com> From: "Michael Brown" In-Reply-To: <20230719113317.276124-1-kraxel@redhat.com> X-Spam-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on blyat.fensystems.co.uk Feedback-ID: 1.eu-west-1.fspj4M/5bzJ9NLRzJP0PaxRwxrpZqiDQJ1IF94CF2TA=:AmazonSES X-SES-Outgoing: 2023.07.19-54.240.7.18 Precedence: Bulk List-Unsubscribe: List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,mcb30@ipxe.org X-Gm-Message-State: AlabQKANbGQ1OyFAs5McWn3Rx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=iPA+5PYr; dmarc=none; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io 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. >=20 > 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. >=20 > Full story with lots of details and discussions is available here: > https://bugzilla.redhat.com/show_bug.cgi?id=3D2211060 >=20 > Signed-off-by: Gerd Hoffmann > --- > OvmfPkg/IoMmuDxe/IoMmuBuffer.c | 3 +++ > 1 file changed, 3 insertions(+) >=20 > diff --git a/OvmfPkg/IoMmuDxe/IoMmuBuffer.c b/OvmfPkg/IoMmuDxe/IoMmuBuffe= r.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; > =20 > + OldTpl =3D gBS->RaiseTPL (TPL_NOTIFY); > ReservedMemBitmap =3D 0; > Status =3D InternalAllocateBuffer ( > Type, > @@ -378,6 +380,7 @@ IoMmuAllocateBounceBuffer ( > ); > MapInfo->ReservedMemBitmap =3D ReservedMemBitmap; > mReservedMemBitmap |=3D ReservedMemBitmap; > + gBS->RestoreTPL (OldTpl); > =20 > ASSERT (Status =3D=3D EFI_SUCCESS); It looks as though IoMmuFreeBounceBuffer() should also raise to=20 TPL_NOTIFY while modifying mReservedMemBitmap, since the modification=20 made in IoMmuFreeBounceBuffer() is not an atomic operation: mReservedMemBitmap &=3D (UINT32)(~MapInfo->ReservedMemBitmap); Thanks, Michael -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- 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/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-