[2511] NetworkPkg: Ip4Dxe/Ip6Dxe: Set SB notify to NULL after event close#1662
Conversation
Ip4StartAutoConfig() and Ip6ConfigStartStatefulAutoConfig() close the DHCP service binding notify event when the service child is successfully created, but do not NULL the instance field afterward. On re-entry, the stale handle passes the non-NULL condition and is closed a second time, which results in a page fault with memory protections enabled. This change sets the event field to NULL immediately after CloseEvent() so that subsequent calls skip attempting to clsoe the event again. Co-authored-by: Wenbo Hou <wenbhou@microsoft.com> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
|
Its good that you are finding and fixing these. How are the interfaces being exercised? Was it a shell test app or something else that is verifying everything? Asking because something that could exercise all the network interfaces would be useful for testing. |
Unfortunately, it is through manual testing of different scenarios on different hardware. Given the emerging trend of these issues in NetworkPkg, some automated testing likely would find more. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/202511 #1662 +/- ##
================================================
Coverage ? 0.55%
================================================
Files ? 165
Lines ? 72366
Branches ? 1789
================================================
Hits ? 402
Misses ? 71957
Partials ? 7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Ip4StartAutoConfig() and Ip6ConfigStartStatefulAutoConfig() close the DHCP service binding notify event when the service child is successfully created, but do not NULL the instance field afterward.
On re-entry, the stale handle passes the non-NULL condition and is closed a second time, which results in a page fault with memory protections enabled.
This change sets the event field to NULL immediately after CloseEvent() so that subsequent calls skip attempting to close the event again.
Code design issue in original implementation. Not a regression.
How This Was Tested
PXE boot with a specific Kensington USB HUB and memory protections enabled.
Repro snippets from debug log:
Integration Instructions
N/A