NetworkPkg/Mtftp4Dxe: Prevent invalid memory dereference on MTFTP poll

[makubacki]

Description

Sequence of events:

1. Mtftp4Driver.c: In Mtftp4DriverBindingStart(), Mtftp4CreateService() is called to create the periodic timer event for notification function Mtftp4OnTimerTick() at TPL_CALLBACK.
2. Mtftp4Driver.c: Mtftp4DriverBindingStart() starts the timer by calling gBS->SetTimer (MtftpSb->Timer, TimerPeriodic, TICKS_PER_SECOND)
3. Eventually, Mtftp4Start() is called to interact with a MTFTPv4 server (from a function like EfiMtftp4ReadFile() in EFI_MTFTP4_PROTOCOL)
4. Toward the end of this function the condition Token->Event != NULL returns false, so execution continues to a loop that continuously calls Poll() in EFI_MTFTP4_PROTOCOL while Token->Status == EFI_NOT_READY

In the above sequence, Mtftp4Start() was invoked at TPL_APPLICATION.

Now, the MTFTP protocol functions are being used as needed to transmit and receive data files.

The functions that depend on processing incoming/outgoing data packets all eventually call Mtftp4Start(). These functions include:

• EfiMtftp4ReadDirectory()
• EfiMtftp4ReadFile()
• EfiMtftp4WriteFile()

---

Meanwhile, an unexpected device disconnect can cause teardown of the driver stack to begin. When this happens:

• Mtftp4Driver.c: Mtftp4DriverBindingStop() (at TPL_CALLBACK potentially interrupting an active loop in Mtftp4Start()). Eventually, it enumerates the children on the service binding handle and calls NetDestroyLinkList() with Mtftp4DestroyChildEntryInHandleBuffer() as the callback on each child handle.
• Mtftp4Driver.c: Mtftp4DestroyChildEntryInHandleBuffer() calls ServiceBinding->DestroyChild() which ends up in Mtftp4ServiceBindingDestroyChild()
• Mtftp4Driver.c: Mtftp4ServiceBindingDestroyChild() begins removing protocols from the child handle, eventually uninstalling the gEfiMtftp4ProtocolGuid protocol.

At this point the protocol is uninstalled from the protocol database, but MTFTP4_PROTOCOL Instance itself is still valid until Mtftp4CleanOperation() is called next.

• Mtftp4Driver.c: Mtftp4ServiceBindingDestroyChild() then makes a TPL raise to TPL_CALLBACK (no-op if TPL is already TPL_CALLBACK) and cleans up the MTFTP session:

  Mtftp4CleanOperation (Instance, EFI_DEVICE_ERROR); UdpIoFreeIo (Instance->UnicastPort);
  
  RemoveEntryList (&Instance->Link); MtftpSb->ChildrenNum--;
  
  gBS->RestoreTPL (OldTpl); ```

Note: The timer is stopped in Mtftp4CleanService() which is called in Mtftp4DriverBindingStop() after Mtftp4ServiceBindingDestroyChild() is called and gEfiMtftp4ServiceBindingProtocolGuid is uninstalled.

It is possible that during Mtftp4ServiceBindingDestroyChild() execution, the Poll() function is being repeatedly invoked in the wait loop in Mtftp4Start() (depending on whether a MTFTP operation is in progress at the time of disconnect).

  //
  // Return immediately for asynchronous operation or poll the
  // instance for synchronous operation.
  //
  while (Token->Status == EFI_NOT_READY) {
    This->Poll (This);
  }

It is also possible that it is being invoked at a TPL less than TPL_CALLBACK, such as TPL_APPLICATION as allowed by UEFI Specification calling restrictions on MTFTP4 operations which is TPL_CALLBACK or lower.

So, it is possible that This->Poll() could be interrupted by the disconnect flow and when execution resumes, the instance data EfiMtftp4Poll is operating on is invalid.

---

This change raises the TPL to TPL_CALLBACK around invocation of This->Poll() in Mtftp4Start(), preventing interruption by the teardown flow in Mtftp4ServiceBindingDestroyChild().

---

• Impacts functionality?
• Impacts security?
• Breaking change?
• Includes tests?
• Includes documentation?

How This Was Tested

• PXE server setup on a separate device (TFTP + DHCP)
• On DUT, set PXE boot as the first boot option
• Boot
• Wait for PXE boot over IPv4 to start
• Unplug a USB hub during Network Bootstrap Program (NBP) file download

---

Traced TPLs in the USB hub disconnect during NBP flow shown above. Relevant TPLs:

Mtftp4DriverBindingStart: CurrentTpl=0x4 (TPL_APPLICATION)
Mtftp4ServiceBindingCreateChild: CurrentTpl=0x4 (TPL_APPLICATION)
EfiMtftp4Configure: CurrentTpl=0x4 (TPL_APPLICATION)
EfiMtftp4ReadFile: CurrentTpl=0x4 (TPL_APPLICATION)
EfiMtftp4Poll: CurrentTpl=0x4 (TPL_APPLICATION)
EfiMtftp4Poll: CurrentTpl=0x4 (TPL_APPLICATION)

<-- USB HUB (NIC) Disconnect -->

Mtftp4ServiceBindingDestroyChild: CurrentTpl=0x8 (TPL_CALLBACK)
Mtftp4DriverBindingStop: CurrentTpl=0x8 (TPL_CALLBACK)

Integration Instructions

• N/A

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (release/202502@c29fe9c). Learn more about missing BASE report.

Additional details and impacted files

@@               Coverage Diff                @@
##             release/202502   #1623   +/-   ##
================================================
  Coverage                  ?   0.55%           
================================================
  Files                     ?     165           
  Lines                     ?   71954           
  Branches                  ?    1751           
================================================
  Hits                      ?     402           
  Misses                    ?   71545           
  Partials                  ?       7           

Flag	Coverage Δ
NetworkPkg	0.55% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.