Skip to content

NvmExpressDriverBindingStart: early return leaks Private and protocol opens#12425

Open
spbrogan wants to merge 1 commit intotianocore:masterfrom
spbrogan:fix_tc_12399
Open

NvmExpressDriverBindingStart: early return leaks Private and protocol opens#12425
spbrogan wants to merge 1 commit intotianocore:masterfrom
spbrogan:fix_tc_12399

Conversation

@spbrogan
Copy link
Copy Markdown
Member

@spbrogan spbrogan commented Apr 9, 2026

After allocating Private via AllocateZeroPool and successfully opening both gEfiDevicePathProtocolGuid and gEfiPciIoProtocolGuid BY_DRIVER, the function attempts PciIo->Attributes(EfiPciIoAttributeOperationGet) If this call fails, the code executes return Status instead of goto Exit.

Trigger path:

NvmExpressDriverBindingStart is called.
OpenProtocol for DevicePath succeeds (opens BY_DRIVER). OpenProtocol for PciIo succeeds (opens BY_DRIVER). AllocateZeroPool for Private succeeds.
PciIo->Attributes(Get) returns an error.
return Status bypasses the Exit: label.

Consequence:
Memory leak of NVME_CONTROLLER_PRIVATE_DATA. Two protocols remain opened BY_DRIVER on the controller handle, preventing other drivers from binding.

  • Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

review

Integration Instructions

n/a

@mikebeaton
Copy link
Copy Markdown
Member

Changes match intended cleanup.

After allocating Private via AllocateZeroPool and successfully opening
both gEfiDevicePathProtocolGuid and gEfiPciIoProtocolGuid BY_DRIVER,
the function attempts PciIo->Attributes(EfiPciIoAttributeOperationGet)
If this call fails, the code executes return Status instead of goto Exit.

Trigger path:

NvmExpressDriverBindingStart is called.
OpenProtocol for DevicePath succeeds (opens BY_DRIVER).
OpenProtocol for PciIo succeeds (opens BY_DRIVER).
AllocateZeroPool for Private succeeds.
PciIo->Attributes(Get) returns an error.
return Status bypasses the Exit: label.

Consequence:
Memory leak of NVME_CONTROLLER_PRIVATE_DATA. Two protocols remain opened
BY_DRIVER on the controller handle, preventing other drivers from binding.

Signed-off-by: Sean Brogan <sebrogan@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: NvmeDxe:: Incomplete cleanup on failure case resulting in memory leak and protocol tracking

2 participants