* [PATCH RFC] nvme: fix NS head cdev lifetime
@ 2026-07-06 13:01 John Garry
2026-07-08 9:27 ` Christoph Hellwig
0 siblings, 1 reply; 6+ messages in thread
From: John Garry @ 2026-07-06 13:01 UTC (permalink / raw)
To: hch, kbusch, sagi, axboe; +Cc: linux-nvme, John Garry
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@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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] nvme: fix NS head cdev lifetime
2026-07-06 13:01 [PATCH RFC] nvme: fix NS head cdev lifetime John Garry
@ 2026-07-08 9:27 ` Christoph Hellwig
2026-07-08 9:57 ` John Garry
0 siblings, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2026-07-08 9:27 UTC (permalink / raw)
To: John Garry; +Cc: hch, kbusch, sagi, axboe, linux-nvme
On Mon, Jul 06, 2026 at 01:01:46PM +0000, John Garry wrote:
> 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:
Can you wire this up to blktests?
> 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@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));
> }
Can we unify this and the ns_head version by checking of which type
the device is somehow? I think driver_data would still be free to
optionally point to the ns_head for eexample.
> + 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)) {
Messed up indenttation here as continuations in an conditional/loop
should not be indented the ssame way as the body.
This could be merged a bit anywya:
if (nvme_tryget_ns_head(head) &&
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);
nvme_put_ns_head(head);
return;
}
but shouldn't we warn for the nvme_tryget_ns_head failure as well?
Or can that even happen here?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] nvme: fix NS head cdev lifetime
2026-07-08 9:27 ` Christoph Hellwig
@ 2026-07-08 9:57 ` John Garry
2026-07-08 11:26 ` John Garry
2026-07-08 16:18 ` Christoph Hellwig
0 siblings, 2 replies; 6+ messages in thread
From: John Garry @ 2026-07-08 9:57 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: kbusch, sagi, axboe, linux-nvme
On 08/07/2026 10:27, Christoph Hellwig wrote:
note: subject should be nvme-multipath for all of these. I'll repost
(with subject fixed) if anyone cares that much.
> On Mon, Jul 06, 2026 at 01:01:46PM +0000, John Garry wrote:
>> 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:
>
> Can you wire this up to blktests?
Sure, but I think that we need KASAN to detect
>
>> 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://urldefense.com/v3/__https://lore.kernel.org/linux-scsi/20260703102918.3723667-1-john.g.garry@oracle.com/T/*m67265e2906d617acd2743c0a00809246d0cfc506__;Iw!!ACWV5N9M2RV99hQ!LwK_OTvYSZQLpG5Nv_1ODqf95BdDoh-lkhsgbnFpKEnWh0bva-VU-DZdOOWPX5UR3G978T5cOhj61A$
>>
>> Signed-off-by: John Garry <john.g.garry@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));
>> }
>
> Can we unify this and the ns_head version by checking of which type
> the device is somehow? I think driver_data would still be free to
> optionally point to the ns_head for eexample.
If we put the nvne_ns_head (vs nvme_ns) in the driver data, then I can't
see a check to know is the driver data points to a nvne_ns_head or
nvme_ns. Unless we keep NULL for one, but that is not so nice.
Other options could be to
a. check the dev parent
b. put cdev fops in driver data, that's not as nice and we also need to
make the NS and NS HEAD fops public
I'll figure something out..
>
>> + 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)) {
>
> Messed up indenttation here as continuations in an conditional/loop
> should not be indented the ssame way as the body.
sure
>
> This could be merged a bit anywya:
>
> if (nvme_tryget_ns_head(head) &&
> 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);
> nvme_put_ns_head(head);
> return;
> }
>
> but shouldn't we warn for the nvme_tryget_ns_head failure as well?
> Or can that even happen here?
>
It should not happen as we have a pointer to nvme_ns_head. If the
recount was zero and we referenced ns_head, then we have bigger problems.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] nvme: fix NS head cdev lifetime
2026-07-08 9:57 ` John Garry
@ 2026-07-08 11:26 ` John Garry
2026-07-08 16:18 ` Christoph Hellwig
1 sibling, 0 replies; 6+ messages in thread
From: John Garry @ 2026-07-08 11:26 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: kbusch, sagi, axboe, linux-nvme
On 08/07/2026 10:57, John Garry wrote:
> On 08/07/2026 10:27, Christoph Hellwig wrote:
>
JFYI, this is also an issue for non-multipath, so both need to be fixed:
# ./ioctl_file /dev/ng0n1
waiting 10 seconds ....
now going to issue ioctl ....
retl=-1
# ./ioctl_file /dev/ng0n1 &
# waiting 10 seconds ....
echo "0000:00:04.0" > /sys/bus/pci/drivers/nvme/unbind
# now going to issue ioctl ....
retl=-1
[ 57.145946]
==================================================================
[ 57.147504] BUG: KASAN: slab-use-after-free in cdev_put.part.0+0x3d/0x40
[ 57.149479] Read of size 8 at addr ffff888105cee0b0 by task
ioctl_file/230
[ 57.150890]
[ 57.151251] CPU: 1 UID: 0 PID: 230 Comm: ioctl_file Not tainted
7.2.0-rc1-00002-g54ab7087cd7c #943 PREEMPT(lazy)
[ 57.151259] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 57.151263] Call Trace:
[ 57.151268] <TASK>
[ 57.151272] dump_stack_lvl+0x68/0xa0
[ 57.151294] print_report+0x10d/0x5d0
[ 57.151304] ? __virt_addr_valid+0x21d/0x3f0
[ 57.151314] ? cdev_put.part.0+0x3d/0x40
[ 57.151320] kasan_report+0x96/0xd0
[ 57.151328] ? cdev_put.part.0+0x3d/0x40
[ 57.151335] cdev_put.part.0+0x3d/0x40
[ 57.151340] __fput+0x7bc/0xa70
[ 57.151350] fput_close_sync+0xd8/0x190
[ 57.151356] ? __pfx_fput_close_sync+0x10/0x10
[ 57.151365] __x64_sys_close+0x79/0xd0
[ 57.151373] do_syscall_64+0x117/0x6b0
[ 57.151381] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 57.151389] RIP: 0033:0x7f2f5a57b727
[ 57.151395] 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 <5b> c3 0f 1f 80 00 0f
[ 57.151400] RSP: 002b:00007ffdeb622670 EFLAGS: 00000202 ORIG_RAX:
0000000000000003
[ 57.151407] RAX: ffffffffffffffda RBX: 00007f2f5a4eb780 RCX:
00007f2f5a57b727
[ 57.151411] RDX: 0000000000000000 RSI: 0000000000000000 RDI:
0000000000000003
[ 57.151414] RBP: 00007ffdeb622700 R08: 0000000000000000 R09:
0000000000000000
[ 57.151417] R10: 0000000000000000 R11: 0000000000000202 R12:
0000000000000000
[ 57.151420] R13: 00007ffdeb622830 R14: 00007f2f5a71b000 R15:
0000561d17af3d78
[ 57.151428] </TASK>
[ 57.151431]
[ 57.171652] Allocated by task 82:
[ 57.171880] kasan_save_stack+0x24/0x50
[ 57.172142] kasan_save_track+0x14/0x30
[ 57.172400] __kasan_kmalloc+0x7f/0x90
[ 57.172650] __kmalloc_cache_node_noprof+0x24b/0x5d0
[ 57.172977] nvme_alloc_ns+0xf8/0x3170
[ 57.173231] nvme_scan_ns+0x508/0x880
[ 57.173477] async_run_entry_fn+0x8c/0x350
[ 57.173762] process_scheduled_works+0xb6f/0x1a00
[ 57.174073] worker_thread+0x4ad/0xb40
[ 57.174325] kthread+0x34f/0x450
[ 57.174542] ret_from_fork+0x563/0x800
[ 57.174791] ret_from_fork_asm+0x1a/0x30
[ 57.175053]
[ 57.175165] Freed by task 230:
[ 57.175369] kasan_save_stack+0x24/0x50
[ 57.175624] kasan_save_track+0x14/0x30
[ 57.175875] kasan_save_free_info+0x3a/0x60
[ 57.176151] __kasan_slab_free+0x43/0x70
[ 57.176412] kfree+0x321/0x500
[ 57.176616] nvme_ns_chr_release+0x36/0x50
[ 57.176885] __fput+0x352/0xa70
[ 57.177096] fput_close_sync+0xd8/0x190
[ 57.177349] __x64_sys_close+0x79/0xd0
[ 57.177596] do_syscall_64+0x117/0x6b0
[ 57.177845] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 57.178180]
[ 57.178287] The buggy address belongs to the object at ffff888105cee000
[ 57.178287] which belongs to the cache kmalloc-2k of size 2048
[ 57.179071] The buggy address is located 176 bytes inside of
[ 57.179071] freed 2048-byte region [ffff888105cee000, ffff888105cee800)
[ 57.179864]
[ 57.179970] The buggy address belongs to the physical page:
[ 57.180332] page: refcount:0 mapcount:0 mapping:0000000000000000
index:0x0 pfn:0x105ce8
[ 57.180848] head: order:3 mapcount:0 entire_mapcount:0
nr_pages_mapped:0 pincount:0
[ 57.181338] flags: 0x200000000000040(head|node=0|zone=2)
[ 57.181699] page_type: f5(slab)
[ 57.181909] raw: 0200000000000040 ffff888100042f00 dead000000000100
dead000000000122
[ 57.182402] raw: 0000000000000000 0000000000080008 00000000f5000000
0000000000000000
[ 57.182893] head: 0200000000000040 ffff888100042f00
dead000000000100 dead000000000122
[ 57.183399] head: 0000000000000000 0000000000080008
00000000f5000000 0000000000000000
[ 57.183900] head: 0200000000000003 fffffffffffffe01
00000000ffffffff 00000000ffffffff
[ 57.184410] head: 0000000000000000 0000000000000000
00000000ffffffff 0000000000000000
[ 57.184906] page dumped because: kasan: bad access detected
[ 57.185271]
[ 57.185380] Memory state around the buggy address:
[ 57.185698] ffff888105cedf80: fc fc fc fc fc fc fc fc fc fc fc fc
fc fc fc fc
[ 57.186171] ffff888105cee000: fa fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 57.186631] >ffff888105cee080: fb fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 57.187201] ^
[ 57.187512] ffff888105cee100: fb fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 57.187980] ffff888105cee180: fb fb fb fb fb fb fb fb fb fb fb fb
fb fb fb fb
[ 57.188447]
==================================================================
[ 57.189823] Disabling lock debugging due to kernel taint
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] nvme: fix NS head cdev lifetime
2026-07-08 9:57 ` John Garry
2026-07-08 11:26 ` John Garry
@ 2026-07-08 16:18 ` Christoph Hellwig
2026-07-09 7:33 ` John Garry
1 sibling, 1 reply; 6+ messages in thread
From: Christoph Hellwig @ 2026-07-08 16:18 UTC (permalink / raw)
To: John Garry; +Cc: Christoph Hellwig, kbusch, sagi, axboe, linux-nvme
On Wed, Jul 08, 2026 at 10:57:02AM +0100, John Garry wrote:
> If we put the nvne_ns_head (vs nvme_ns) in the driver data, then I can't
> see a check to know is the driver data points to a nvne_ns_head or nvme_ns.
> Unless we keep NULL for one, but that is not so nice.
I think NULL just for one is fine, and I'm pretty sure we already do
something like this in the nvme code (or at least used to in the
past).
>
>> but shouldn't we warn for the nvme_tryget_ns_head failure as well?
>> Or can that even happen here?
>>
> It should not happen as we have a pointer to nvme_ns_head. If the recount
> was zero and we referenced ns_head, then we have bigger problems.
That's what I though, so we could do a plain kref_get here.
And maybe replace the annoying krefs with a plain refcount_t one day.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC] nvme: fix NS head cdev lifetime
2026-07-08 16:18 ` Christoph Hellwig
@ 2026-07-09 7:33 ` John Garry
0 siblings, 0 replies; 6+ messages in thread
From: John Garry @ 2026-07-09 7:33 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: kbusch, sagi, axboe, linux-nvme
On 08/07/2026 17:18, Christoph Hellwig wrote:
> On Wed, Jul 08, 2026 at 10:57:02AM +0100, John Garry wrote:
>> If we put the nvne_ns_head (vs nvme_ns) in the driver data, then I can't
>> see a check to know is the driver data points to a nvne_ns_head or nvme_ns.
>> Unless we keep NULL for one, but that is not so nice.
> I think NULL just for one is fine, and I'm pretty sure we already do
> something like this in the nvme code (or at least used to in the
> past).
I mentioned in a follow up mail that we have this same issue for the NS
cdev, so we need to do similar reference taking there.
So using a NULL check would look like this:
static void nvme_cdev_rel(struct device *dev)
{
ida_free(&nvme_ns_chr_minor_ida, MINOR(dev->devt));
if (dev->driver_data)
nvme_put_ns_head(container_of(dev, struct nvme_ns_head,
cdev_device));
else
nvme_put_ns(container_of(dev, struct nvme_ns, cdev_device));
}
Another idea is:
static void nvme_cdev_rel(struct device *dev)
{
ida_free(&nvme_ns_chr_minor_ida, MINOR(dev->devt));
if (dev->parent->class == &nvme_class)
nvme_put_ns(container_of(dev, struct nvme_ns, cdev_device));
else
nvme_put_ns_head(container_of(dev, struct nvme_ns_head,
cdev_device));
}
any preference?
A nicer check would be cdev->fops == nvme_ns_chr_fops, but the cdev
pointer cannot be looked up from the dev release handler and it's too
late to even ref the cdev at this point.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-09 7:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-06 13:01 [PATCH RFC] nvme: fix NS head cdev lifetime John Garry
2026-07-08 9:27 ` Christoph Hellwig
2026-07-08 9:57 ` John Garry
2026-07-08 11:26 ` John Garry
2026-07-08 16:18 ` Christoph Hellwig
2026-07-09 7:33 ` John Garry
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.