Skip to content

Conversation

@mtfriesen
Copy link
Contributor

@mtfriesen mtfriesen commented Nov 11, 2025

Description

Describe the purpose of and changes within this Pull Request.

#845 exposed a latent bug in the interface binding module: each interface copied the caller's capabilities struct into the interface object, but it left dangling pointers within that struct. If an interface was later removed, the new XdpRxQueueEnableChecksumOffload routine could dereference the dangling pointer and AV.

The expected lifetime of the caller's structure isn't clearly documented, but it certainly can't persist beyond the interface's final RemoveInterfaceComplete callback, which is the state during the CI bugcheck.

Also do the same thing for the Hooks nested pointer.
Also validate the size of the capabilities struct is large enough to contain the new/optional RxChecksumSupported field.

Resolves #922

Testing

Do any existing tests cover this change? Are new tests needed?

Builds locally, CI.

Documentation

Is there any documentation impact for this change?

N/A.

Installation

Is there any installer impact for this change?

N/A.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes a dangling pointer bug in the XDP interface binding module where interface objects copied the caller's capabilities structure but left nested pointers dangling, potentially causing an access violation when interfaces were removed.

Key changes:

  • Allocate and deep-copy the nested CapabilitiesEx structure during interface initialization
  • Free the allocated CapabilitiesEx memory during interface cleanup

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

guhetier
guhetier previously approved these changes Nov 11, 2025
Copy link
Contributor

@guhetier guhetier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also see a Hooks array in the capability struct, is its lifetime is independent from the caller structure?

@mtfriesen
Copy link
Contributor Author

I also see a Hooks array in the capability struct, is its lifetime is independent from the caller structure?

Yeah, good catch. That's not causing us immediate problems right now, but we should be consistent and protect the lifetimes and avoid dangling pointers.

@ProjectsByJackHe ProjectsByJackHe self-requested a review November 11, 2025 19:10
@mtfriesen mtfriesen merged commit 8a6540e into main Nov 11, 2025
63 checks passed
@mtfriesen mtfriesen deleted the mtfriesen/copy_capabilities_ex branch November 11, 2025 20:52
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.

CI stress tests are flaky

4 participants