Remove unnecessary (and incorrect) `&mut` cast in net-tokio
What the developer wrote
Authored by Matt Corallo
Remove unnecessary (and incorrect) `&mut` cast in net-tokio
The owned `Waker` wake method assumed it had the only reference to
the sender as the `Waker` is owned at that point, however our
`Waker`s can be `clone`d, leaving multiple references to the inner
`Sender` (held in an `Arc`).
Thus, the `&mut` cast is technically undefined behavior. However,
as this patch demonstrates, its only use is in calling an `&self`
method which derefs an internal `Arc` in tokio, so its highly
unlikely to lead to miscompilation.
Reported by Project Loupe.
What changed, and why it matters
This commit fixes a Rust unsafe-code bug in the networking glue between the Lightning Dev Kit and Tokio. The code was treating a shared sender object as if it had exclusive access, which is undefined behavior in Rust. In practice the called method only reads through an internal reference counter, so the risk of real-world harm is low, but the pattern was incorrect and could theoretically confuse the compiler into generating wrong code.
Apply the patch. As a follow-up, audit other unsafe blocks in lightning-net-tokio for similar &mut casts through shared pointers, and consider adding Miri or static-analysis checks to CI for unsafe code paths.
Security signals we found
Undefined behavior via invalid &mut uniqueness guarantee
Unsafe pointer cast in waker callback
Shared mutable state through cloned Waker/Arc
Incorrect use of Rust aliasing rules
Evidence from the diff
In lightning-net-tokio/src/lib.rs, wake_socket_waker() cast an opaque pointer to &mut mpsc::Sender<()> before calling sender.try_send(()). Because Waker can be cloned, multiple RawWakers can exist concurrently, each pointing at the same Sender held behind an Arc, so the &mut reference was not unique. The fix changes the cast to a shared & reference. try_send is an &self method, so the mutation it performs is internal to Tokio’s Arc-backed channel; the immediate caller does not need mutable access. The commit message explicitly labels the old cast as undefined behavior but downplays the chance of miscompilation.
Changed components
lightning-net-tokio/src/lib.rswake_socket_waker functionmpsc::Sender<()> waker integrationInspect captured patch +1 / −1
diff --git a/lightning-net-tokio/src/lib.rs b/lightning-net-tokio/src/lib.rs
index 2e8e568..953fed6 100644
--- a/lightning-net-tokio/src/lib.rs
+++ b/lightning-net-tokio/src/lib.rs
@@ -663,7 +663,7 @@ fn clone_socket_waker(orig_ptr: *const ()) -> task::RawWaker {
// sending thread may have already gone away due to a socket close, in which case there's nothing
// to wake up anyway.
fn wake_socket_waker(orig_ptr: *const ()) {
- let sender = unsafe { &mut *(orig_ptr as *mut mpsc::Sender<()>) };
+ let sender = unsafe { &*(orig_ptr as *mut mpsc::Sender<()>) };
let _ = sender.try_send(());
drop_socket_waker(orig_ptr);
}
Why this scored 33/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.