[PATCH v2 0/1] rust: sync: Make Guard::lock_ref unavailable if Lock is not Sync

[PATCH v2 0/1] rust: sync: Make Guard::lock_ref unavailable if Lock is not Sync

[Priya Bala Govindasamy]

Dear Linux Kernel Maintainers,

We are developing a tool called FerroLens to detect potential unsound 
behavior in Rust code in the Linux kernel.
The tool internally uses an LLM to detect bugs. 
We then perform manual analysis to verify these reports. 
FerroLens reported the following bug in rust/kernel/sync/lock.rs.

Guard::lock_ref() returns a reference to the Lock stored in the Guard. 
Guard is Sync if T is Sync. However, Lock is Sync only if T is Send. 
This means that a reference to Guard can be sent across threads and 
multiple threads can acquire a reference to the Lock, 
even if Lock is not Sync. 

The following scenario is possible:
If T is sync but not send, 
Thread A: Creates a Lock<T>. Because T is not Send, Lock<T> is not Sync. 
It cannot be shared with other threads. 
Thread A: Locks the mutex and gets a Guard<T>. 
Since T is Sync, code says Guard<T> is Sync. 
Thread A can now safely give a &Guard<T> to Thread B.
Thread B: calls guard.lock_ref(). This returns a &'a Lock<T>.
Thread B now has a reference to the Lock. In this case, 
even after Thread A drops the guard, Thread B still has that &Lock. 
Thread B can now call lock.lock() and get a &mut T.
This is unsound.

Here is a poc kernel module which demonstrates this problem:

use core::sync::atomic::{AtomicBool, Ordering};
use kernel::{
    bindings, c_str,
    error::from_err_ptr,
    prelude::*,
    sync::{
        lock::{mutex::MutexBackend, Backend, Guard, Lock},
        new_mutex, Arc,
    },
    types::NotThreadSafe,
};
use pin_init::stack_pin_init;

module! {
    type: LockRefPoc,
    name: "lock_ref_poc",
    authors: ["Priya Govindasamy"],
    description: "Sample module demonstrating cross-thread Guard::lock_ref access",
    license: "GPL",
}

struct LockRefPoc;

struct SyncButNotSend {
    value: u32,
    _not_send: NotThreadSafe,
}

// SAFETY: Access to `value` is synchronized by the lock in this sample, and the marker field is
// only used to prevent `Send`.
unsafe impl Sync for SyncButNotSend {}

struct HandoffState {
    lock_ref_ready: AtomicBool,
    guard_dropped: AtomicBool,
    relock_done: AtomicBool,
}

struct ThreadBContext {
    guard_addr: usize,
    handoff: Arc<HandoffState>,
    thread_id: u32,
}

fn guard_matches_lock<T, B: Backend>(guard: &Guard<'_, T, B>, lock: &Lock<T, B>) -> bool {
    core::ptr::eq(guard.lock_ref(), lock)
}

unsafe extern "C" fn thread_b_main(data: *mut kernel::ffi::c_void) -> i32 {
    // SAFETY: `data` comes from `Arc::into_raw` in `spawn_thread_b`, and the thread takes back
    // ownership of exactly one reference.
    let ctx = unsafe { Arc::from_raw(data.cast::<ThreadBContext>()) };

    let guard: &Guard<'_, SyncButNotSend, MutexBackend> =
        // SAFETY: Thread A passes the address of a live guard to thread B.
        unsafe { &*(ctx.guard_addr as *const Guard<'_, SyncButNotSend, MutexBackend>) };

    let leaked_lock = guard.lock_ref();
    pr_info!(
        "thread B obtained &Lock<SyncButNotSend, _> through &Guard\n",
    );
    ctx.handoff.lock_ref_ready.store(true, Ordering::Release);

    while !ctx.handoff.guard_dropped.load(Ordering::Acquire) {
        unsafe { bindings::msleep(20) };
    }

    let mut relocked = leaked_lock.lock();
    relocked.value += 1;
    pr_info!(
        "thread B locked after thread A dropped the original guard; value={}\n",
        relocked.value
    );
    ctx.handoff.relock_done.store(true, Ordering::Release);
    0
}

fn spawn_thread_b(guard_addr: usize, handoff: Arc<HandoffState>, thread_id: u32) -> Result {
    let ctx = Arc::new(
        ThreadBContext {
            guard_addr,
            handoff,
            thread_id,
        },
        GFP_KERNEL,
    )?;
    let data = Arc::into_raw(ctx) as *mut kernel::ffi::c_void;

    let task = match from_err_ptr(unsafe {
        bindings::kthread_create_on_node(
            Some(thread_b_main),
            data,
            bindings::NUMA_NO_NODE,
            c_str!("lock_ref_thread_b/%u").as_char_ptr(),
            thread_id,
        )
    }) {
        Ok(task) => task,
        Err(err) => {
            // SAFETY: Thread creation failed, so the raw reference was not handed off.
            unsafe { drop(Arc::from_raw(data.cast::<ThreadBContext>())) };
            return Err(err);
        }
    };

    // SAFETY: `task` is a valid sleeping kthread returned by `kthread_create_on_node`.
    unsafe { bindings::wake_up_process(task) };
    Ok(())
}

unsafe extern "C" fn thread_a_main(_data: *mut kernel::ffi::c_void) -> i32 {
    let handoff = match Arc::new(
        HandoffState {
            lock_ref_ready: AtomicBool::new(false),
            guard_dropped: AtomicBool::new(false),
            relock_done: AtomicBool::new(false),
        },
        GFP_KERNEL,
    ) {
        Ok(h) => h,
        Err(_) => return ENOMEM.to_errno(),
    };

    stack_pin_init!(let lock = new_mutex!(
        SyncButNotSend {
            value: 100,
            _not_send: NotThreadSafe,
        },
        "lock_ref_poc.thread_a_lock"
    ));

    let guard = lock.lock();
    if !guard_matches_lock(&guard, &lock) {
        pr_err!("thread A: guard.lock_ref() did not return the original lock\n");
        return EINVAL.to_errno();
    }

    if let Err(err) = spawn_thread_b(
        (&guard as *const Guard<'_, SyncButNotSend, MutexBackend>) as usize,
        handoff.clone(),
        2,
    ) {
        return err.to_errno();
    }

    while !handoff.lock_ref_ready.load(Ordering::Acquire) {
        unsafe { bindings::msleep(20) };
    }

    drop(guard);
    pr_info!("thread A dropped the original guard after thread B captured &Lock\n");
    handoff.guard_dropped.store(true, Ordering::Release);

    while !handoff.relock_done.load(Ordering::Acquire) {
        unsafe { bindings::msleep(20) };
    }

    0
}

fn spawn_thread_a() -> Result {
    let task = match from_err_ptr(unsafe {
        bindings::kthread_create_on_node(
            Some(thread_a_main),
            core::ptr::null_mut(),
            bindings::NUMA_NO_NODE,
            c_str!("lock_ref_thread_a").as_char_ptr(),
        )
    }) {
        Ok(task) => task,
        Err(err) => return Err(err),
    };

    // SAFETY: `task` is a valid sleeping kthread returned by `kthread_create_on_node`.
    unsafe { bindings::wake_up_process(task) };
    Ok(())
}

impl kernel::Module for LockRefPoc {
    fn init(_module: &'static ThisModule) -> Result<Self> {
        
        spawn_thread_a()?;

        Ok(Self)
    }
}

impl Drop for LockRefPoc {
    fn drop(&mut self) {
        pr_info!("lock_ref_poc unloaded\n");
    }
}


Makefile:

obj-m += lock_ref_poc.o

KDIR := /lib/modules/$(shell uname -r)/build

PWD := $(shell pwd)

all:
	$(MAKE) -C $(KDIR) M=$(PWD) LLVM=1 modules

clean:
	$(MAKE) -C $(KDIR) M=$(PWD) clean


Output:
[  360.591802] lock_ref_poc: thread B obtained &Lock<SyncButNotSend, _> through &Guard
[  360.617600] lock_ref_poc: thread A dropped the original guard after thread B captured &Lock
[  360.641550] lock_ref_poc: thread B locked after thread A dropped the original guard; value=101
[  362.580557] lock_ref_poc: lock_ref_poc unloaded


Priya Bala Govindasamy (1):
  Make Guard::lock_ref unavailable if Lock is not Sync

 rust/kernel/sync/lock.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

-- 
2.34.1

[PATCH v2 1/1] Make Guard::lock_ref unavailable if Lock is not Sync

[Priya Bala Govindasamy]

Guard::lock_ref() returns a reference to the Lock stored in the Guard. 
Guard is Sync if T is Sync. However, Lock is Sync only if T is Send. 
This means that a reference to Guard can be sent across threads and 
multiple threads can acquire a reference to the Lock, 
even if Lock is not Sync. 

The following scenario is possible:
If T is sync but not send, 
Thread A: Creates a Lock<T>. Because T is not Send, Lock<T> is not Sync. 
It cannot be shared with other threads. 
Locks the mutex and gets a Guard<T>. 
Since T is Sync, code says Guard<T> is Sync. 
Thread A can now safely give a &Guard<T> to Thread B.
Thread B: calls guard.lock_ref(). This returns a &'a Lock<T>.
Thread B now has a reference to the Lock. In this case, 
even after Thread A drops the guard, Thread B still has that &Lock. 
Thread B can now call lock.lock() and get a &mut T.
This is unsound.

Fix this issue by adding a trait bound to Guard::lock_ref so that 
this method is available only when Lock<T, B> implements Sync. 
This ensures that multiple threads cannot acquire a reference 
to the Lock if it is not Sync.

Fixes: 8f65291dae0e ("rust: sync: Add accessor for the lock behind a given guard")
Cc: stable@vger.kernel.org
Reported-by: Dylan Zueck<dzueck@uci.edu>
Reported-by: Yuan Tan<ytan089@ucr.edu>
Signed-off-by: Priya Bala Govindasamy<pgovind2@uci.edu>
Assisted-by: ChatGPT:gpt-5.4
---
changes in v2:
- Add a trait bound only to the Guard::lock_ref() method. Changes to the lock lifetime are not necessary.

Link: https://lore.kernel.org/all/20260605022400.31489-1-kentertan12138@outlook.com/#t

 rust/kernel/sync/lock.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
index 10b6b5e9b024..91317d0d81b9 100644
--- a/rust/kernel/sync/lock.rs
+++ b/rust/kernel/sync/lock.rs
@@ -234,7 +234,10 @@ impl<'a, T: ?Sized, B: Backend> Guard<'a, T, B> {
     /// // `g` originates from `l`.
     /// assert_held(&g, &l);
     /// ```
-    pub fn lock_ref(&self) -> &'a Lock<T, B> {
+    pub fn lock_ref(&self) -> &'a Lock<T, B>
+    where
+        Lock<T, B>: Sync,
+    {
         self.lock
     }
 
-- 
2.34.1

Re: [PATCH v2 1/1] Make Guard::lock_ref unavailable if Lock is not Sync

[Gary Guo]

On Tue Jun 30, 2026 at 12:52 AM BST, Priya Bala Govindasamy wrote:
> Guard::lock_ref() returns a reference to the Lock stored in the Guard. 
> Guard is Sync if T is Sync. However, Lock is Sync only if T is Send. 

Please quote code with ``

> This means that a reference to Guard can be sent across threads and 
> multiple threads can acquire a reference to the Lock, 
> even if Lock is not Sync. 
>
> The following scenario is possible:
> If T is sync but not send, 
> Thread A: Creates a Lock<T>. Because T is not Send, Lock<T> is not Sync. 
> It cannot be shared with other threads. 
> Locks the mutex and gets a Guard<T>. 
> Since T is Sync, code says Guard<T> is Sync. 
> Thread A can now safely give a &Guard<T> to Thread B.
> Thread B: calls guard.lock_ref(). This returns a &'a Lock<T>.
> Thread B now has a reference to the Lock. In this case, 
> even after Thread A drops the guard, Thread B still has that &Lock. 
> Thread B can now call lock.lock() and get a &mut T.
> This is unsound.

This is way too verbose. Please be concise. Something like

    For `l: Lock<T>`, this means that `send_across_thread(&l.lock()).lock_ref()`
    is able to send `&Lock<T>` across threads despite `T: !Sync`.

should be more readable.

>
> Fix this issue by adding a trait bound to Guard::lock_ref so that 
> this method is available only when Lock<T, B> implements Sync. 
> This ensures that multiple threads cannot acquire a reference 
> to the Lock if it is not Sync.

Again, this is repeating the same thing with two different style. Typical LLM
style verbosity. Just remove the second sentence and keep the first.

>
> Fixes: 8f65291dae0e ("rust: sync: Add accessor for the lock behind a given guard")
> Cc: stable@vger.kernel.org
> Reported-by: Dylan Zueck<dzueck@uci.edu>
> Reported-by: Yuan Tan<ytan089@ucr.edu>
> Signed-off-by: Priya Bala Govindasamy<pgovind2@uci.edu>
> Assisted-by: ChatGPT:gpt-5.4

Acked-by: Gary Guo <gary@garyguo.net>

> ---
> changes in v2:
> - Add a trait bound only to the Guard::lock_ref() method. Changes to the lock lifetime are not necessary.
>
> Link: https://lore.kernel.org/all/20260605022400.31489-1-kentertan12138@outlook.com/#t
>
>  rust/kernel/sync/lock.rs | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/sync/lock.rs b/rust/kernel/sync/lock.rs
> index 10b6b5e9b024..91317d0d81b9 100644
> --- a/rust/kernel/sync/lock.rs
> +++ b/rust/kernel/sync/lock.rs
> @@ -234,7 +234,10 @@ impl<'a, T: ?Sized, B: Backend> Guard<'a, T, B> {
>      /// // `g` originates from `l`.
>      /// assert_held(&g, &l);
>      /// ```
> -    pub fn lock_ref(&self) -> &'a Lock<T, B> {
> +    pub fn lock_ref(&self) -> &'a Lock<T, B>
> +    where
> +        Lock<T, B>: Sync,
> +    {
>          self.lock
>      }
>