From: Haoze Xie GenDiskBuilder::build() still has fallible work after __blk_mq_alloc_disk(), but its error path only recovers the foreign queue data. That leaks the temporary gendisk and request_queue until later teardown. If the caller moved the last Arc> into build(), the leaked queue can retain blk-mq state after the tag set is dropped. Fix the pre-registration failure path by dropping the temporary gendisk reference with put_disk() before recovering queue_data, so disk_release() can tear down the owned queue. Also pair GenDisk::drop() with put_disk() after del_gendisk(). Once a Rust GenDisk has been added with device_add_disk(), del_gendisk() only unregisters it; the final gendisk reference still has to be dropped to complete the release path. Fixes: 3253aba3408a ("rust: block: introduce `kernel::block::mq` module") Cc: stable@kernel.org Reported-by: Yuan Tan Reported-by: Xin Liu Reviewed-by: Andreas Hindborg Signed-off-by: Haoze Xie Signed-off-by: Ren Wei --- Changes in v2: - Add the missing put_disk() after del_gendisk() in GenDisk::drop(), as suggested by Andreas Hindborg. - Keep the GenDiskBuilder::build() failure cleanup fix and fold both lifecycle fixes into one patch. - v1 Link: https://lore.kernel.org/r/b6411cc055080c984a67bfad72fd683aa84b8e13.1779596478.git.royenheart@gmail.com rust/kernel/block/mq/gen_disk.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/rust/kernel/block/mq/gen_disk.rs b/rust/kernel/block/mq/gen_disk.rs index 912cb805caf5..6ea16b943c99 100644 --- a/rust/kernel/block/mq/gen_disk.rs +++ b/rust/kernel/block/mq/gen_disk.rs @@ -149,6 +149,17 @@ pub fn build( // 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. + unsafe { bindings::put_disk(gendisk) }; + // SAFETY: `data` came from `into_foreign()` above and has not been + // converted back on this cleanup path. + drop(unsafe { T::QueueData::from_foreign(data) }); + }); + // The failure guard now owns both pieces of cleanup; the early guard + // must not run on this path anymore. + recover_data.dismiss(); let mut writer = NullTerminatedFormatter::new( // SAFETY: `gendisk` points to a valid and initialized instance. We @@ -172,7 +183,7 @@ pub fn build( }, )?; - recover_data.dismiss(); + cleanup_failure.dismiss(); // INVARIANT: `gendisk` was initialized above. // INVARIANT: `gendisk` was added to the VFS via `device_add_disk` above. @@ -214,6 +225,10 @@ fn drop(&mut self) { // initialized instance of `struct gendisk`, and it was previously added // to the VFS. unsafe { bindings::del_gendisk(self.gendisk) }; + // SAFETY: By type invariant, `self.gendisk` was added to the VFS, so + // `put_disk()` must follow `del_gendisk()` to drop the final gendisk + // reference and trigger the remaining release path. + unsafe { bindings::put_disk(self.gendisk) }; // SAFETY: `queue.queuedata` was created by `GenDiskBuilder::build` with // a call to `ForeignOwnable::into_foreign` to create `queuedata`. -- 2.47.3