Re: [edk2-devel] [PATCH v3 00/20] NetworkPkg: CVE-2023-45236 and CVE-2023-45237

["Gerd Hoffmann" <kraxel@redhat.com>]

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 (#119343): https://edk2.groups.io/g/devel/message/119343
Mute This Topic: https://groups.io/mt/106276830/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-