If ASSERT trigs the exception, could call stack show each caller? Thanks Liming > -----邮件原件----- > 发件人: Gerd Hoffmann <kra...@redhat.com> > 发送时间: 2024年5月29日 21:09 > 收件人: devel@edk2.groups.io; dougfl...@microsoft.com > 抄送: Liming Gao <gaolim...@byosoft.com.cn>; Ard Biesheuvel > <a...@kernel.org> > 主题: Re: [edk2-devel] [PATCH v3 00/20] NetworkPkg: CVE-2023-45236 and > CVE-2023-45237 > > On Thu, May 23, 2024 at 10:44:52PM GMT, Doug Flick via groups.io wrote: > > > > > REF:https://blog.quarkslab.com/pixiefail-nine-vulnerabilities-in-tianocores- edk-ii- > ipv6-network-stack.html > > > > This patch series patches the following CVEs: > > - CVE-2023-45236: Predictable TCP Initial Sequence Numbers > > - CVE-2023-45237: Use of a Weak PseudoRandom Number Generator > > Ok, looks like there is some more fallout from this patch series which I > havn't seen in my initial testing. It does not always happen, didn't > figure yet what exactly triggers the behavior. But in some cases there > is quite some network stack activity, apparently done by > EVT_SIGNAL_EXIT_BOOT_SERVICES event handlers ... > > With the debug patch below applied the tail of the ovmf log looks like > this: > > VirtioRngExitBoot: Context=0x7D73D798 > Hash2ServiceBindingDestroyChild - Invalid handle > MnpServiceBindingDestroyChild: Failed to uninstall the ManagedNetwork > protocol, Invalid Parameter. > Support(): UNDI3.1 found on handle 7D461118 > Support(): supported on 7D461118 > Start(): UNDI3.1 found > > snp->undi.start() 1h:8000h > InstallProtocolInterface: 7AB33A91-ACE5-4326-B572-E7EE33D39F16 > 7CE872C0 > InstallProtocolInterface: F44C00EE-1F2C-4A00-AA09-1C9F3E0800A3 > 7CE7D020 > Failed to generate random data using secure algorithm 0: Unsupported > Failed to generate random data using secure algorithm 1: Unsupported > Failed to generate random data using secure algorithm 2: Unsupported > Failed to generate random data using secure algorithm 3: Unsupported > VirtioRngGetRNG: not ready > Failed to generate random data using secure algorithm 4: Device Error > > ASSERT_EFI_ERROR (Status = Device Error) > ASSERT > /home/kraxel/projects/edk2/NetworkPkg/Library/DxeNetLib/DxeNetLib.c(965): > !(((INTN)(RETURN_STATUS)(Status)) < 0) > > The VirtioRngDxe EVT_SIGNAL_EXIT_BOOT_SERVICES handler resets the > device, to make sure it will stop any DMA. > > Once the reset is done the device can't deliver random numbers any more, > but the network code wants some. So with the debug patch an assert is > triggered, without the debug patch the system simply hangs because the > virtio-rng device wouldn't answer request sent by the driver. > > I'm wondering what the network code is actually doing here in the first > place? It apparently /installs/ protocols in the > EVT_SIGNAL_EXIT_BOOT_SERVICES handler? I don't think this is how things > are supposed to work ... > > take care, > Gerd > > ------------------------- cut here ------------------------- > diff --git a/OvmfPkg/VirtioRngDxe/VirtioRng.h > b/OvmfPkg/VirtioRngDxe/VirtioRng.h > index 2da99540a208..3519521d6ab5 100644 > --- a/OvmfPkg/VirtioRngDxe/VirtioRng.h > +++ b/OvmfPkg/VirtioRngDxe/VirtioRng.h > @@ -33,6 +33,7 @@ typedef struct { > VRING Ring; // VirtioRingInit 2 > EFI_RNG_PROTOCOL Rng; // VirtioRngInit 1 > VOID *RingMap; // VirtioRingMap > 2 > + BOOLEAN Ready; > } VIRTIO_RNG_DEV; > > #define VIRTIO_ENTROPY_SOURCE_FROM_RNG(RngPointer) \ > diff --git a/OvmfPkg/VirtioNetDxe/Events.c b/OvmfPkg/VirtioNetDxe/Events.c > index 75a9644f749c..36e3eed4617c 100644 > --- a/OvmfPkg/VirtioNetDxe/Events.c > +++ b/OvmfPkg/VirtioNetDxe/Events.c > @@ -77,7 +77,7 @@ VirtioNetExitBoot ( > // > VNET_DEV *Dev; > > - DEBUG ((DEBUG_VERBOSE, "%a: Context=0x%p\n", __func__, Context)); > + DEBUG ((DEBUG_INFO, "%a: Context=0x%p\n", __func__, Context)); > Dev = Context; > if (Dev->Snm.State == EfiSimpleNetworkInitialized) { > Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0); > diff --git a/OvmfPkg/VirtioRngDxe/VirtioRng.c > b/OvmfPkg/VirtioRngDxe/VirtioRng.c > index 069aed148af1..370c9ac8f1de 100644 > --- a/OvmfPkg/VirtioRngDxe/VirtioRng.c > +++ b/OvmfPkg/VirtioRngDxe/VirtioRng.c > @@ -156,6 +156,10 @@ VirtioRngGetRNG ( > } > > Dev = VIRTIO_ENTROPY_SOURCE_FROM_RNG (This); > + if (!Dev->Ready) { > + DEBUG ((DEBUG_INFO, "%a: not ready\n", __func__)); > + return EFI_DEVICE_ERROR; > + } > // > // Map Buffer's system physical address to device address > // > @@ -382,6 +386,7 @@ VirtioRngInit ( > // > Dev->Rng.GetInfo = VirtioRngGetInfo; > Dev->Rng.GetRNG = VirtioRngGetRNG; > + Dev->Ready = TRUE; > > return EFI_SUCCESS; > > @@ -414,8 +419,8 @@ VirtioRngUninit ( > // VIRTIO_CFG_WRITE() returns, the host will have learned to stay away from > // the old comms area. > // > + Dev->Ready = FALSE; > Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0); > - > Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap); > > VirtioRingUninit (Dev->VirtIo, &Dev->Ring); > @@ -435,7 +440,7 @@ VirtioRngExitBoot ( > { > VIRTIO_RNG_DEV *Dev; > > - DEBUG ((DEBUG_VERBOSE, "%a: Context=0x%p\n", __func__, Context)); > + DEBUG ((DEBUG_INFO, "%a: Context=0x%p\n", __func__, Context)); > // > // Reset the device. This causes the hypervisor to forget about the virtio > // ring. > @@ -444,6 +449,7 @@ VirtioRngExitBoot ( > // executing after ExitBootServices() is permitted to overwrite it. > // > Dev = Context; > + Dev->Ready = FALSE; > Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0); > } >
-=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119379): https://edk2.groups.io/g/devel/message/119379 Mute This Topic: https://groups.io/mt/106383321/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
