[PATCH RFC] nvme: fix NS head cdev lifetime
John Garry
john.g.garry at oracle.com
Mon Jul 6 06:01:46 PDT 2026
Sashiko bot reported a potential problem for the NS head cdev lifetime in
the libmultipath refactoring in [0].
Currently the NS head .open and .release file_operations methods take a
reference to the nvme_ns_head to ensure that this structure does not
disappear while we have files open.
When we teardown the NS head, we call nvme_cdev_del() -> cdev_device_del()
-> cdev_del(). However after cdev_del() returns, cdevs already open will
remain and their fops will still be callable. As such, we can still
reference the cdev after the nvme_ns_head reference count drops to 0 (and
is freed).
This can be shown with an application which delays between opening the cdev
and issuing the ioctl while the NS head is being torn down:
# ./ioctl_file /dev/ng1n1 &
# waiting 10 seconds ....
# ./ini_nvme_teardown.sh
[ 21.221718] nvme nvme1: Removing ctrl: NQN "nvme-test-target"
[ 21.274609] nvme nvme2: Removing ctrl: NQN "nvme-test-target"
# now going to issue ioctl ....
[ 26.549285] ==================================================================
[ 26.550841] BUG: KASAN: slab-use-after-free in cdev_put.part.0+0x3d/0x40
[ 26.552352] Read of size 8 at addr ffff88811e7fa170 by task ioctl_file/237
[ 26.553805]
[ 26.554227] CPU: 3 UID: 0 PID: 237 Comm: ioctl_file Not tainted 7.2.0-rc1-00004-g6852a10e32d4 #921 PREEMPT(lazy)
[ 26.554236] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 26.554241] Call Trace:
[ 26.554245] <TASK>
[ 26.554248] dump_stack_lvl+0x68/0xa0
[ 26.554266] print_report+0x10d/0x5d0
[ 26.554276] ? __virt_addr_valid+0x21d/0x3f0
[ 26.554287] ? cdev_put.part.0+0x3d/0x40
[ 26.554292] kasan_report+0x96/0xd0
[ 26.554300] ? cdev_put.part.0+0x3d/0x40
[ 26.554307] cdev_put.part.0+0x3d/0x40
[ 26.554313] __fput+0x7bc/0xa70
[ 26.554322] fput_close_sync+0xd8/0x190
[ 26.554328] ? __pfx_fput_close_sync+0x10/0x10
[ 26.554337] __x64_sys_close+0x79/0xd0
[ 26.554344] do_syscall_64+0x117/0x6b0
[ 26.554351] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 26.554358] RIP: 0033:0x7f938c067727
[ 26.554364] Code: 48 89 fa 4c 89 df e8 28 ad 00 00 8b 93 08 03 00 00 59 5e 48 83 f8 fc 74 1a 5b c3 0f 1f 84 00 00 00 00 00 48 8b 44 24 10 0f 05 <5bf
[ 26.554369] RSP: 002b:00007fff49f05980 EFLAGS: 00000202 ORIG_RAX: 0000000000000003
[ 26.554376] RAX: ffffffffffffffda RBX: 00007f938bfd7780 RCX: 00007f938c067727
[ 26.554380] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
[ 26.554383] RBP: 00007fff49f05a10 R08: 0000000000000000 R09: 0000000000000000
[ 26.554386] R10: 0000000000000000 R11: 0000000000000202 R12: 0000000000000000
[ 26.554389] R13: 00007fff49f05b40 R14: 00007f938c207000 R15: 000055c148471d78
[ 26.554397] </TASK>
[ 26.554399]
[ 26.575612] Allocated by task 100:
[ 26.575871] kasan_save_stack+0x24/0x50
[ 26.576157] kasan_save_track+0x14/0x30
[ 26.576418] __kasan_kmalloc+0x7f/0x90
[ 26.576668] __kmalloc_noprof+0x281/0x6c0
[ 26.576938] nvme_alloc_ns+0x7f7/0x3170
[ 26.577206] nvme_scan_ns+0x508/0x880
[ 26.577449] async_run_entry_fn+0x8c/0x350
[ 26.577723] process_scheduled_works+0xb6f/0x1a00
[ 26.578034] worker_thread+0x4ad/0xb40
[ 26.578283] kthread+0x34f/0x450
[ 26.578501] ret_from_fork+0x563/0x800
[ 26.578752] ret_from_fork_asm+0x1a/0x30
[ 26.579012]
[ 26.579124] Freed by task 237:
[ 26.579335] kasan_save_stack+0x24/0x50
[ 26.579596] kasan_save_track+0x14/0x30
[ 26.579855] kasan_save_free_info+0x3a/0x60
[ 26.580131] __kasan_slab_free+0x43/0x70
[ 26.580388] kfree+0x321/0x500
[ 26.580591] nvme_ns_head_chr_release+0x39/0x50
[ 26.580883] __fput+0x352/0xa70
[ 26.581095] fput_close_sync+0xd8/0x190
[ 26.581350] __x64_sys_close+0x79/0xd0
[ 26.581595] do_syscall_64+0x117/0x6b0
[ 26.581842] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 26.582176]
[ 26.582286] Last potentially related work creation:
[ 26.582593] kasan_save_stack+0x24/0x50
[ 26.582841] kasan_record_aux_stack+0x89/0xa0
[ 26.583210] insert_work+0x22/0x170
[ 26.583442] __queue_work+0x7b1/0xfa0
[ 26.583682] queue_work_on+0x77/0x80
[ 26.583921] kblockd_schedule_work+0x18/0x20
[ 26.584207] nvme_mpath_put_disk+0x42/0xa0
[ 26.584632] nvme_free_ns_head+0x1c/0x160
[ 26.584904] nvme_ns_head_chr_release+0x39/0x50
[ 26.585208] __fput+0x352/0xa70
[ 26.585420] fput_close_sync+0xd8/0x190
[ 26.585677] __x64_sys_close+0x79/0xd0
[ 26.585924] do_syscall_64+0x117/0x6b0
[ 26.586173] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 26.586505]
[ 26.586616] Second to last potentially related work creation:
[ 26.586991] kasan_save_stack+0x24/0x50
[ 26.587246] kasan_record_aux_stack+0x89/0xa0
[ 26.587538] insert_work+0x22/0x170
[ 26.587770] __queue_work+0x7b1/0xfa0
[ 26.588010] queue_work_on+0x77/0x80
[ 26.588247] kblockd_schedule_work+0x18/0x20
[ 26.588529] nvme_remove_head+0x3d/0xb0
[ 26.588787] nvme_ns_remove+0x4b2/0x930
[ 26.589040] nvme_remove_namespaces+0x29c/0x410
[ 26.589340] nvme_do_delete_ctrl+0xf3/0x190
[ 26.589611] nvme_delete_ctrl_sync+0x71/0x90
[ 26.589889] nvme_sysfs_delete+0x91/0xb0
[ 26.590151] kernfs_fop_write_iter+0x2fb/0x4a0
[ 26.590452] vfs_write+0x929/0xfc0
[ 26.590688] ksys_write+0xf2/0x1d0
[ 26.590923] do_syscall_64+0x117/0x6b0
[ 26.591171] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 26.591498]
[ 26.591605] The buggy address belongs to the object at ffff88811e7fa000
[ 26.591605] which belongs to the cache kmalloc-4k of size 4096
[ 26.592409] The buggy address is located 368 bytes inside of
[ 26.592409] freed 4096-byte region [ffff88811e7fa000, ffff88811e7fb000)
[ 26.593205]
[ 26.593321] The buggy address belongs to the physical page:
[ 26.593701] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x11e7f8
[ 26.594250] head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
[ 26.594743] flags: 0x200000000000040(head|node=0|zone=2)
[ 26.595093] page_type: f5(slab)
[ 26.595312] raw: 0200000000000040 ffff888100043040 dead000000000122 0000000000000000
[ 26.595810] raw: 0000000000000000 0000000000040004 00000000f5000000 0000000000000000
[ 26.596309] head: 0200000000000040 ffff888100043040 dead000000000122 0000000000000000
[ 26.596806] head: 0000000000000000 0000000000040004 00000000f5000000 0000000000000000
[ 26.597313] head: 0200000000000003 fffffffffffffe01 00000000ffffffff 00000000ffffffff
[ 26.597813] head: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
[ 26.598317] page dumped because: kasan: bad access detected
[ 26.598676]
[ 26.598784] Memory state around the buggy address:
[ 26.599093] ffff88811e7fa000: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 26.599559] ffff88811e7fa080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 26.600023] >ffff88811e7fa100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 26.600485] ^
[ 26.600921] ffff88811e7fa180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 26.601390] ffff88811e7fa200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[ 26.601855] ==================================================================
[ 26.602374] Disabling lock debugging due to kernel taint
When all fds for the cdev disappear, the cdev removal path puts a
reference to the parent object, which is the nvme_ns_head.cdev_device - see
cdev_default_release() -> kobject_put(parent). We can use the lifetime
of the cdev_device to resolve this lifetime issue.
Fix the lifetime for the cdev by making adding the cdev add take a
reference to the NS head and drop that reference in the
nvme_ns_head.cdev_device release function.
[0] https://lore.kernel.org/linux-scsi/20260703102918.3723667-1-john.g.garry@oracle.com/T/#m67265e2906d617acd2743c0a00809246d0cfc506
Signed-off-by: John Garry <john.g.garry at oracle.com>
---
Setting as RFC as I am not sure if this is the best solution.
diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 453c1f0b2dd09..c7578b7ed6b76 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -3887,7 +3887,7 @@ static int nvme_subsys_check_duplicate_ids(struct nvme_subsystem *subsys,
return 0;
}
-static void nvme_cdev_rel(struct device *dev)
+void nvme_cdev_rel(struct device *dev)
{
ida_free(&nvme_ns_chr_minor_ida, MINOR(dev->devt));
}
@@ -3900,7 +3900,8 @@ void nvme_cdev_del(struct cdev *cdev, struct device *cdev_device)
int nvme_cdev_add(const char *name, struct cdev *cdev,
struct device *cdev_device,
- const struct file_operations *fops, struct module *owner)
+ const struct file_operations *fops, struct module *owner,
+ void (*release)(struct device *cdev_dev))
{
int minor, ret;
@@ -3915,7 +3916,7 @@ int nvme_cdev_add(const char *name, struct cdev *cdev,
}
cdev_device->devt = MKDEV(MAJOR(nvme_ns_chr_devt), minor);
cdev_device->class = &nvme_ns_chr_class;
- cdev_device->release = nvme_cdev_rel;
+ cdev_device->release = release;
device_initialize(cdev_device);
cdev_init(cdev, fops);
cdev->owner = owner;
@@ -3956,7 +3957,8 @@ static void nvme_add_ns_cdev(struct nvme_ns *ns)
ns->head->instance);
if (nvme_cdev_add(name, &ns->cdev, &ns->cdev_device,
- &nvme_ns_chr_fops, ns->ctrl->ops->module)) {
+ &nvme_ns_chr_fops, ns->ctrl->ops->module,
+ nvme_cdev_rel)) {
dev_err(ns->ctrl->device, "Unable to create the %s device\n",
name);
return;
diff --git a/drivers/nvme/host/multipath.c b/drivers/nvme/host/multipath.c
index 9b9a657fa330f..016b6b0128c71 100644
--- a/drivers/nvme/host/multipath.c
+++ b/drivers/nvme/host/multipath.c
@@ -616,29 +616,23 @@ static inline struct nvme_ns_head *cdev_to_ns_head(struct cdev *cdev)
return container_of(cdev, struct nvme_ns_head, cdev);
}
-static int nvme_ns_head_chr_open(struct inode *inode, struct file *file)
-{
- if (!nvme_tryget_ns_head(cdev_to_ns_head(inode->i_cdev)))
- return -ENXIO;
- return 0;
-}
-
-static int nvme_ns_head_chr_release(struct inode *inode, struct file *file)
-{
- nvme_put_ns_head(cdev_to_ns_head(inode->i_cdev));
- return 0;
-}
-
static const struct file_operations nvme_ns_head_chr_fops = {
.owner = THIS_MODULE,
- .open = nvme_ns_head_chr_open,
- .release = nvme_ns_head_chr_release,
.unlocked_ioctl = nvme_ns_head_chr_ioctl,
.compat_ioctl = compat_ptr_ioctl,
.uring_cmd = nvme_ns_head_chr_uring_cmd,
.uring_cmd_iopoll = nvme_ns_chr_uring_cmd_iopoll,
};
+static void nvme_cdev_ns_head_rel(struct device *dev)
+{
+ struct nvme_ns_head *head = container_of(dev, struct nvme_ns_head,
+ cdev_device);
+
+ nvme_cdev_rel(dev);
+ nvme_put_ns_head(head);
+}
+
static void nvme_add_ns_head_cdev(struct nvme_ns_head *head)
{
char name[32];
@@ -647,11 +641,15 @@ static void nvme_add_ns_head_cdev(struct nvme_ns_head *head)
snprintf(name, sizeof(name), "ng%dn%d", head->subsys->instance,
head->instance);
- if (nvme_cdev_add(name, &head->cdev, &head->cdev_device,
- &nvme_ns_head_chr_fops, THIS_MODULE)) {
- dev_err(disk_to_dev(head->disk),
- "Unable to create the %s device\n", name);
- return;
+ if (nvme_tryget_ns_head(head)) {
+ if (nvme_cdev_add(name, &head->cdev, &head->cdev_device,
+ &nvme_ns_head_chr_fops, THIS_MODULE,
+ nvme_cdev_ns_head_rel)) {
+ dev_err(disk_to_dev(head->disk),
+ "Unable to create the %s device\n", name);
+ nvme_put_ns_head(head);
+ return;
+ }
}
set_bit(NVME_NSHEAD_CDEV_LIVE, &head->flags);
}
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 824651cc898db..60d2d35926743 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -999,8 +999,10 @@ bool nvme_tryget_ns_head(struct nvme_ns_head *head);
void nvme_put_ns_head(struct nvme_ns_head *head);
int nvme_cdev_add(const char *name, struct cdev *cdev,
struct device *cdev_device,
- const struct file_operations *fops, struct module *owner);
+ const struct file_operations *fops, struct module *owner,
+ void (*release)(struct device *cdev_device));
void nvme_cdev_del(struct cdev *cdev, struct device *cdev_device);
+void nvme_cdev_rel(struct device *dev);
int nvme_ioctl(struct block_device *bdev, blk_mode_t mode,
unsigned int cmd, unsigned long arg);
long nvme_ns_chr_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
--
2.43.7
More information about the Linux-nvme
mailing list