The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Adarsh Das <adarshdas950@gmail.com>
To: adarshdas950@gmail.com
Cc: a.hindborg@kernel.org, acourbot@nvidia.com, aliceryhl@google.com,
	axboe@kernel.dk, bjorn3_gh@protonmail.com, boqun@kernel.org,
	dakr@kernel.org, daniel.almeida@collabora.com, gary@garyguo.net,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	lossin@kernel.org, ojeda@kernel.org,
	rust-for-linux@vger.kernel.org, tamird@kernel.org,
	tmgross@umich.edu, work@onurozkan.dev
Subject: [PATCH v2] rust: block: set GenDisk block_device_operations.owner to THIS_MODULE
Date: Thu,  6 Aug 2026 14:06:55 +0530	[thread overview]
Message-ID: <20260806083655.23161-1-adarshdas950@gmail.com> (raw)
In-Reply-To: <20260805192020.107601-1-adarshdas950@gmail.com>

GenDiskBuilder left block_device_operations.owner NULL. Pass the driver's
ThisModule into GenDiskBuilder::build(), heap-allocate the operations
table, and keep it alive until the gendisk is released via free_disk.
Update rnull as the in-tree caller.

v2:
- Free fops in free_disk instead of GenDisk::drop to fix use-after-free
  when the device stays open after removal. (Sashiko)
- Install the cleanup guard before fops allocation to avoid leaking gendisk
  on -ENOMEM. (Sashiko)
- Link to v1: https://lore.kernel.org/all/20260805192020.107601-1-adarshdas950@gmail.com/

Signed-off-by: Adarsh Das <adarshdas950@gmail.com>
---
 drivers/block/rnull/configfs.rs  |  1 +
 drivers/block/rnull/rnull.rs     |  3 +-
 rust/kernel/block/mq.rs          |  9 ++--
 rust/kernel/block/mq/gen_disk.rs | 82 ++++++++++++++++++++++----------
 4 files changed, 66 insertions(+), 29 deletions(-)

diff --git a/drivers/block/rnull/configfs.rs b/drivers/block/rnull/configfs.rs
index 7c2eb5c0b722..bba30d590f68 100644
--- a/drivers/block/rnull/configfs.rs
+++ b/drivers/block/rnull/configfs.rs
@@ -147,6 +147,7 @@ fn store(this: &DeviceConfig, page: &[u8]) -> Result {
 
         if !guard.powered && power_op {
             guard.disk = Some(NullBlkDevice::new(
+                &THIS_MODULE,
                 &guard.name,
                 guard.block_size,
                 guard.rotational,
diff --git a/drivers/block/rnull/rnull.rs b/drivers/block/rnull/rnull.rs
index 0ca8715febe8..4265a133cbf0 100644
--- a/drivers/block/rnull/rnull.rs
+++ b/drivers/block/rnull/rnull.rs
@@ -46,6 +46,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
 
 impl NullBlkDevice {
     fn new(
+        this_module: &'static ThisModule,
         name: &CStr,
         block_size: u32,
         rotational: bool,
@@ -61,7 +62,7 @@ fn new(
             .logical_block_size(block_size)?
             .physical_block_size(block_size)?
             .rotational(rotational)
-            .build(fmt!("{}", name.to_str()?), tagset, queue_data)
+            .build(this_module, fmt!("{}", name.to_str()?), tagset, queue_data)
     }
 }
 
diff --git a/rust/kernel/block/mq.rs b/rust/kernel/block/mq.rs
index 1fd0d54dd549..33561e0f67af 100644
--- a/rust/kernel/block/mq.rs
+++ b/rust/kernel/block/mq.rs
@@ -8,8 +8,8 @@
 //! - Implement [`Operations`] for a type `T`.
 //! - Create a [`TagSet<T>`].
 //! - Create a [`GenDisk<T>`], via the [`GenDiskBuilder`].
-//! - Add the disk to the system by calling [`GenDiskBuilder::build`] passing in
-//!   the `TagSet` reference.
+//! - Add the disk to the system by calling [`GenDiskBuilder::build`], passing in
+//!   the driver's [`ThisModule`], the disk name, the `TagSet`, and queue data.
 //!
 //! The types available in this module that have direct C counterparts are:
 //!
@@ -86,9 +86,12 @@
 //!
 //! let tagset: Arc<TagSet<MyBlkDevice>> =
 //!     Arc::pin_init(TagSet::new(1, 256, 1), flags::GFP_KERNEL)?;
+//! # // SAFETY: Dummy `ThisModule` for doctest compilation only.
+//! # static THIS_MODULE: ThisModule =
+//! #     unsafe { ThisModule::from_ptr(core::ptr::null_mut()) };
 //! let mut disk = gen_disk::GenDiskBuilder::new()
 //!     .capacity_sectors(4096)
-//!     .build(fmt!("myblk"), tagset, ())?;
+//!     .build(&THIS_MODULE, fmt!("myblk"), tagset, ())?;
 //!
 //! # Ok::<(), kernel::error::Error>(())
 //! ```
diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs
index fc97dd873974..a51027e8c1c1 100644
--- a/rust/kernel/block/mq/gen_disk.rs
+++ b/rust/kernel/block/mq/gen_disk.rs
@@ -17,6 +17,23 @@
     types::{ForeignOwnable, ScopeGuard},
 };
 
+/// # Safety
+///
+/// `disk` must be valid.
+unsafe extern "C" fn free_fops(disk: *mut bindings::gendisk) {
+    // SAFETY: `disk` is valid.
+    let fops = unsafe { (*disk).fops };
+    if fops.is_null() {
+        return;
+    }
+
+    // SAFETY: `disk` is valid; `fops` came from `KBox::into_raw` in `build`.
+    unsafe {
+        (*disk).fops = core::ptr::null_mut();
+        drop(KBox::from_raw(fops.cast_mut()));
+    }
+}
+
 /// A builder for [`GenDisk`].
 ///
 /// Use this struct to configure and add new [`GenDisk`] to the VFS.
@@ -95,8 +112,12 @@ pub fn capacity_sectors(mut self, capacity: u64) -> Self {
     }
 
     /// Build a new `GenDisk` and add it to the VFS.
+    ///
+    /// `this_module` must be the [`ThisModule`] for the kernel module registering
+    /// the disk.
     pub fn build<T: Operations>(
         self,
+        this_module: &'static ThisModule,
         name: fmt::Arguments<'_>,
         tagset: Arc<TagSet<T>>,
         queue_data: T::QueueData,
@@ -125,32 +146,16 @@ pub fn build<T: Operations>(
             )
         })?;
 
-        const TABLE: bindings::block_device_operations = bindings::block_device_operations {
-            submit_bio: None,
-            open: None,
-            release: None,
-            ioctl: None,
-            compat_ioctl: None,
-            check_events: None,
-            unlock_native_capacity: None,
-            getgeo: None,
-            set_read_only: None,
-            swap_slot_free_notify: None,
-            report_zones: None,
-            devnode: None,
-            alternative_gpt_sector: None,
-            get_unique_id: None,
-            // TODO: Set to `THIS_MODULE`.
-            owner: core::ptr::null_mut(),
-            pr_ops: core::ptr::null_mut(),
-            free_disk: None,
-            poll_bio: None,
-        };
-
-        // SAFETY: `gendisk` is a valid pointer as we initialized it above
-        unsafe { (*gendisk).fops = &TABLE };
-
         let cleanup_failure = ScopeGuard::new_with_data((gendisk, data), |(gendisk, data)| {
+            // SAFETY: `gendisk` came from `__blk_mq_alloc_disk()` above and
+            // has not been added to the VFS on this cleanup path.
+            let fops = unsafe { (*gendisk).fops };
+            if !fops.is_null() {
+                // SAFETY: `gendisk` came from `__blk_mq_alloc_disk()` above.
+                unsafe { (*gendisk).fops = core::ptr::null_mut() };
+                // SAFETY: `fops` came from `KBox::into_raw` below on this path.
+                drop(unsafe { KBox::from_raw(fops.cast_mut()) });
+            }
             // SAFETY: `gendisk` came from `__blk_mq_alloc_disk()` above and
             // has not been added to the VFS on this cleanup path.
             unsafe { bindings::put_disk(gendisk) };
@@ -159,6 +164,33 @@ pub fn build<T: Operations>(
             drop(unsafe { T::QueueData::from_foreign(data) });
         });
 
+        let fops = KBox::new(
+            bindings::block_device_operations {
+                submit_bio: None,
+                open: None,
+                release: None,
+                ioctl: None,
+                compat_ioctl: None,
+                check_events: None,
+                unlock_native_capacity: None,
+                getgeo: None,
+                set_read_only: None,
+                swap_slot_free_notify: None,
+                report_zones: None,
+                devnode: None,
+                alternative_gpt_sector: None,
+                get_unique_id: None,
+                owner: this_module.as_ptr(),
+                pr_ops: core::ptr::null_mut(),
+                free_disk: Some(free_fops),
+                poll_bio: None,
+            },
+            GFP_KERNEL,
+        )?;
+
+        // SAFETY: `gendisk` is a valid pointer as we initialized it above.
+        unsafe { (*gendisk).fops = KBox::into_raw(fops).cast() };
+
         // The failure guard now owns both pieces of cleanup; the early guard
         // must not run on this path anymore.
         recover_data.dismiss();
-- 
2.55.0


  reply	other threads:[~2026-08-06  8:37 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-05 19:20 [PATCH] rust: block: set GenDisk block_device_operations.owner to THIS_MODULE Adarsh Das
2026-08-06  8:36 ` Adarsh Das [this message]
2026-08-06  9:08   ` [PATCH v2] " Andreas Hindborg
2026-08-06 10:40   ` [PATCH v3] " Adarsh Das

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260806083655.23161-1-adarshdas950@gmail.com \
    --to=adarshdas950@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=acourbot@nvidia.com \
    --cc=aliceryhl@google.com \
    --cc=axboe@kernel.dk \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun@kernel.org \
    --cc=dakr@kernel.org \
    --cc=daniel.almeida@collabora.com \
    --cc=gary@garyguo.net \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=tamird@kernel.org \
    --cc=tmgross@umich.edu \
    --cc=work@onurozkan.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox