Skip to content

Commit 2957771

Browse files
royenheartaxboe
authored andcommitted
rust: block: fix GenDisk cleanup paths
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<TagSet<T>> 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: 3253aba ("rust: block: introduce `kernel::block::mq` module") Cc: stable@kernel.org Reported-by: Yuan Tan <yuantan098@gmail.com> Reported-by: Xin Liu <bird@lzu.edu.cn> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org> Signed-off-by: Haoze Xie <royenheart@gmail.com> Signed-off-by: Ren Wei <n05ec@lzu.edu.cn> Link: https://patch.msgid.link/b70aff9a920cc42110fe5cf454c3099561863519.1780063368.git.royenheart@gmail.com Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent 11ff85d commit 2957771

1 file changed

Lines changed: 19 additions & 1 deletion

File tree

rust/kernel/block/mq/gen_disk.rs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,19 @@ impl GenDiskBuilder {
150150
// SAFETY: `gendisk` is a valid pointer as we initialized it above
151151
unsafe { (*gendisk).fops = &TABLE };
152152

153+
let cleanup_failure = ScopeGuard::new_with_data((gendisk, data), |(gendisk, data)| {
154+
// SAFETY: `gendisk` came from `__blk_mq_alloc_disk()` above and
155+
// has not been added to the VFS on this cleanup path.
156+
unsafe { bindings::put_disk(gendisk) };
157+
// SAFETY: `data` came from `into_foreign()` above and has not been
158+
// converted back on this cleanup path.
159+
drop(unsafe { T::QueueData::from_foreign(data) });
160+
});
161+
162+
// The failure guard now owns both pieces of cleanup; the early guard
163+
// must not run on this path anymore.
164+
recover_data.dismiss();
165+
153166
let mut writer = NullTerminatedFormatter::new(
154167
// SAFETY: `gendisk` points to a valid and initialized instance. We
155168
// have exclusive access, since the disk is not added to the VFS
@@ -172,7 +185,7 @@ impl GenDiskBuilder {
172185
},
173186
)?;
174187

175-
recover_data.dismiss();
188+
cleanup_failure.dismiss();
176189

177190
// INVARIANT: `gendisk` was initialized above.
178191
// INVARIANT: `gendisk` was added to the VFS via `device_add_disk` above.
@@ -215,6 +228,11 @@ impl<T: Operations> Drop for GenDisk<T> {
215228
// to the VFS.
216229
unsafe { bindings::del_gendisk(self.gendisk) };
217230

231+
// SAFETY: By type invariant, `self.gendisk` was added to the VFS, so
232+
// `put_disk()` must follow `del_gendisk()` to drop the final gendisk
233+
// reference and trigger the remaining release path.
234+
unsafe { bindings::put_disk(self.gendisk) };
235+
218236
// SAFETY: `queue.queuedata` was created by `GenDiskBuilder::build` with
219237
// a call to `ForeignOwnable::into_foreign` to create `queuedata`.
220238
// `ForeignOwnable::from_foreign` is only called here.

0 commit comments

Comments
 (0)