Re: [PATCH 6/7] rust: workqueue: add Work::cancel_work_sync()

[John Hubbard <jhubbard@nvidia.com>]

On 8/4/26 12:52 PM, Danilo Krummrich wrote:
> Add a method to cancel a work item and wait for it to finish if it is
> currently running.
> 
> This will also be used by ScopedWork's destructor to synchronously
> cancel work before dropping borrowed data.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  rust/kernel/workqueue/mod.rs | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/rust/kernel/workqueue/mod.rs b/rust/kernel/workqueue/mod.rs
> index 5de88c59b2e5..2b87f935712a 100644
> --- a/rust/kernel/workqueue/mod.rs
> +++ b/rust/kernel/workqueue/mod.rs
> @@ -585,6 +585,14 @@ pub unsafe fn raw_get(ptr: *const Self) -> *mut bindings::work_struct {
>          // the compiler does not complain that the `work` field is unused.
>          unsafe { Opaque::cast_into(core::ptr::addr_of!((*ptr).work)) }
>      }
> +
> +    /// Cancels the work item and waits for it to finish if it is running.
> +    ///
> +    /// Returns `true` if the work was pending, `false` otherwise.
> +    pub fn cancel_work_sync(&self) -> bool {
> +        // SAFETY: We have a reference to a valid, initialized Work, so the pointer is valid.
> +        unsafe { bindings::cancel_work_sync(Self::raw_get(self)) }
> +    }

This seems to expose a way for driver writers to leak work items,
doesn't it?

Previously, work items were either run, or failed to enqueue, and
both of those paths restored the Arc via Arc::from_raw().

But now with this new cancel_work_sync(), Arc::from_raw() never gets
called. So for example:

#[pin_data]
struct MyStruct {
    #[pin]
    work: Work<MyStruct>,
}

impl_has_work! {
    impl HasWork<Self> for MyStruct { self.work }
}

impl WorkItem for MyStruct {
    type Pointer = Arc<MyStruct>;

    fn run(_this: Arc<MyStruct>) {}
}

let obj = Arc::pin_init(
    pin_init!(MyStruct {
        work <- new_work!("MyStruct::work"),
    }),
    GFP_KERNEL,
)?;

// refcount 1 -> 2, and the workqueue owns the second one
let _ = workqueue::system_dfl().enqueue(obj.clone());

// takes it off the worklist, so run() never reclaims that reference
obj.work.cancel_work_sync();

// refcount 2 -> 1. MyStruct is never dropped.
drop(obj);

thanks,
-- 
John Hubbard