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
next prev parent 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.