Skip to content

[safety] Weaken lifetime in get_xde_state() #1016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
daniel-levin wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[safety] Weaken lifetime in get_xde_state() #1016

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion xde/src/xde.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ struct UnderlayState {
shared_props: OffloadInfo,
}

fn get_xde_state() -> &'static XdeState {
fn get_xde_state<'a>() -> &'a XdeState {

@FelixMcFelix FelixMcFelix Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I agree with this in principle, but I'm not sure that it helps in the worst case? I.e., any caller can still invoke let state: &'static _ = get_xde_state(); and get a static reference. If the compiler typically picks a narrower 'a when this isn't specified, that might suffice though.

I'm otherwise not sure what the best way is to encode the idea that this pointer's validity does outlive the scope of almost every method within the module. I.e., when the module is not attached I'd expect everything else to be functionally inert.

@daniel-levin daniel-levin Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a good point... I think it may be possible to constrain the lifetime to non-static.

@iximeow iximeow Jul 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is effectively the same problem as having a reference to "library-local" data in a Rust-originated .so in userland (which is kind of an Unresolved Problem, lots of proposals for a "dynamic" lifetime etc to describe this, but none of it really has gone anywhere).

strictly speaking, returning &'static XdeState is not itself undefined behavior, but using it while xde_dip is deallocated is. so a more narrow lifetime which still does not exactly track with the initialized-ness of xde_dip could still have the problem. realistically what would be "ideal" is to have some handle to the library provided as an argument to all the xde functions which you could handle.get_xde_state() to get an appropriately-scoped lifetime and know you're not leaking a reference of xde_dip into some external code where it will get used after the module might be unloaded. as long as you never unload xde the 'static is fine, even! (ish. I think we all assume that nothing else calls functions in xde while we're going through xde_attach)

the next best thing would probably be to have an unsafe fn xde_handle() -> XdeHandle that acts as a witness that the library is loaded (where you disclaim that the library is initialized when calling it). then XdeHandle::get_xde_state() would be a more "normal" get_xde_state(&self) -> &XdeState, where the lifetime is tied to the ZST handle thingy so it can't get automagically expanded incorrectly. that ZST should probably not be copy or clone, since you wouldn't want to inadvertently leak those "witnesses" of the module around.

finally you'd want to interlock XdeHandle existing anywhere and xde_detach(). if any XdeHandle would be living on detach that's a bug anyway. that looks like it is an existing bug: if some thread is in xde_ioc_iopte_command, nothing has management_lock, so you could go detach the module, null out xde_dip, and have the system die on the null deref right?

how much of the use of xde_dip is in hot paths, would it be reasonable to have a refcount as another static here, bumped on xde_handle() and decremented on drop? then you could guard detach on that?

// Safety: The opte_dip pointer is write-once and is a valid
// pointer passed to attach(9E). The returned pointer is valid as
// it was derived from Box::into_raw() during `xde_attach`.
Expand Down