[PATCH v4] nvme/069: add a test for multipath cdev lifetime

[PATCH v4] nvme/069: add a test for multipath cdev lifetime

[John Garry]

From: John Garry <john.garry@linux.dev>

In [0], a fix was proposed for the NS and NS head (multipath) cdev
lifetime.

The issue was that fds for the nvme-generic cdev may exist after we tear
down the nvme-subsystem. Issuing an ioctl on that cdev may expose a
use-after-free.

This test recreates the method described in [0] to expose this issue
for NS head cdev.

First a fd is created by opening the nvme-generic cdev. Next we tear down
the nvme-subsystem. Finally we try to issue an ioctl on the cdev - without
the kernel fix in [0], this should trigger a KASAN warn.

[0] https://lore.kernel.org/linux-nvme/20260713104238.3034640-1-john.g.garry@oracle.com/T/#me118851584fbcbb960795d0f04e2262e5a295613

Reviewed-by: Daniel Wagner <dwagner@suse.de>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
Differences to v3:
- Add RB tag from Daniel (thanks!)
- Address comments from Shin'ichiro
 - script syntax
 - add copyright
 - wait command tweak

Difference to v2:
- remove sleep and use signal to sync

diff --git a/common/nvme b/common/nvme
index 15e9c3f..42301dc 100644
--- a/common/nvme
+++ b/common/nvme
@@ -300,6 +300,27 @@ _nvme_wait_subsys_removed() {
 	done
 }
 
+_nvme_find_subsys_nvme_generic() {
+	local subsyspath _subsysnqn subsyspathbase nvmegenericbase devicelink
+	local subsysnqn="$def_subsysnqn"
+
+	for subsyspath in /sys/class/nvme-subsystem/*; do
+		_subsysnqn=$(cat "${subsyspath}/subsysnqn" 2> /dev/null)
+		if [ "$subsysnqn" == "$_subsysnqn" ]; then
+			subsyspathbase="$(basename "$subsyspath")"
+			for nvmegeneric in /sys/class/nvme-generic/*; do
+				nvmegenericbase="$(basename "$nvmegeneric")"
+				devicelink="$(readlink "$nvmegeneric/device")"
+				if [[ "$devicelink" =~ $subsyspathbase ]]; then
+					echo "$nvmegenericbase"
+					break
+				fi
+			done
+			break
+		fi
+	done
+}
+
 _nvme_connect_subsys() {
 	local subsysnqn="$def_subsysnqn"
 	local hostnqn="$def_hostnqn"
diff --git a/src/.gitignore b/src/.gitignore
index 988d785..af9ebf6 100644
--- a/src/.gitignore
+++ b/src/.gitignore
@@ -14,3 +14,4 @@
 /nvme-passthrough-meta
 /ioctl-lbmd-query
 /nvme-passthru-admin-uring
+/nvme-delay-ioctl
diff --git a/src/Makefile b/src/Makefile
index d75daf1..4779ffd 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -18,6 +18,7 @@ C_TARGETS := \
 	loop_change_fd \
 	loop_get_status_null \
 	mount_clear_sock \
+	nvme-delay-ioctl \
 	nvme-passthrough-meta \
 	ioctl-lbmd-query \
 	nbdsetsize \
diff --git a/src/nvme-delay-ioctl.c b/src/nvme-delay-ioctl.c
new file mode 100644
index 0000000..2cce95a
--- /dev/null
+++ b/src/nvme-delay-ioctl.c
@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: GPL-3.0+
+// Copyright (C) 2026 John Garry
+
+/*
+ * Simple test to issue an ioctl some time after opening a cdev.
+ * This is to test lifetime of cdev, and whether we can handle
+ * removing the cdev while we have active fds.
+ */
+#define _GNU_SOURCE
+#include <dirent.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <signal.h>
+
+#include <inttypes.h>
+#include <sys/ioctl.h>
+#include <linux/types.h>
+
+#ifndef _LINUX_NVME_IOCTL_H
+#define _LINUX_NVME_IOCTL_H
+#define NVME_IOCTL_ID		_IO('N', 0x40)
+#endif /* _UAPI_LINUX_NVME_IOCTL_H */
+
+int main(int argc, char **argv)
+{
+	int fd, fd1;
+	int count;
+
+	if (argc < 2) {
+		fprintf(stderr, "usage: %s /dev/ngXnX", argv[0]);
+		return EINVAL;
+	}
+
+	fd = open(argv[1], O_RDONLY);
+	if (fd < 0)
+		return fd;
+
+	/*
+	 * Steps:
+	 * a. Signal to parent that we have opened the file so that it may
+	 *    start the teardown.
+	 * b. Try to open device until disallowed/gone.
+	 * c. Sleep to allow nvme-subsystem be torn down.
+	 * d. Issue the ioctl on original fd.
+	 */
+	kill(getppid(), SIGUSR2);
+	count = 0;
+	for (;;) {
+		fd1 = open(argv[1], O_RDONLY);
+		usleep(500000);
+		if (fd1 < 0)
+			break;
+		close(fd1);
+		count++;
+		if (count > 10) {
+			fprintf(stderr, "%s still present", argv[0]);
+			return EINVAL;
+		}
+	}
+
+	ioctl(fd, NVME_IOCTL_ID);
+	close(fd);
+
+	return 0;
+}
diff --git a/tests/nvme/069 b/tests/nvme/069
new file mode 100755
index 0000000..61f796d
--- /dev/null
+++ b/tests/nvme/069
@@ -0,0 +1,61 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-3.0+
+# Copyright (C) 2026 John Garry
+#
+# Test NVMe multipath nvme-generic cdev lifetime behaves as expected.
+
+. tests/nvme/rc
+
+DESCRIPTION="NVMe multipath cdev lifetime test"
+CHECK_DMESG=1
+QUICK=1
+
+requires() {
+	_nvme_requires
+	_have_loop
+	_have_module_param_value nvme_core multipath Y
+	_require_nvme_trtype_is_fabrics
+	_have_kernel_options KASAN
+}
+
+set_conditions() {
+	_set_nvme_trtype "$@"
+}
+
+test() {
+	echo "Running ${TEST_NAME}"
+
+	local nvmedev
+	local loops=0
+	local quit=0
+	local nvmegeneric
+
+	_setup_nvmet
+	_nvmet_target_setup
+	_nvme_connect_subsys
+
+	nvmedev=$(_find_nvme_dev "${def_subsysnqn}")
+	nvmegeneric=$(_nvme_find_subsys_nvme_generic)
+
+	trap 'quit=1' USR2
+	# nvme-delay-ioctl will open the nvme-generic cdev, wait for teardown,
+	# and then issue an ioctl.
+	# We tear down the nvme-subsystem in the foreground so we can catch
+	# if the ioctl triggers a use-after-free KASAN warn.
+	src/nvme-delay-ioctl "/dev/$nvmegeneric" &
+
+	while [ "$quit" -ne 1 ]; do
+		sleep 0.1
+		((loops++))
+		if [[ "$loops" == "100" ]]; then
+			echo "no signal"
+			break
+		fi
+	done
+
+	_nvme_disconnect_ctrl "${nvmedev}"
+	_nvmet_target_cleanup
+	wait "$(jobs -p)"
+
+	echo "Test complete"
+}
diff --git a/tests/nvme/069.out b/tests/nvme/069.out
new file mode 100644
index 0000000..cd2e1fd
--- /dev/null
+++ b/tests/nvme/069.out
@@ -0,0 +1,2 @@
+Running nvme/069
+Test complete
-- 
2.43.7

Re: [PATCH v4] nvme/069: add a test for multipath cdev lifetime

[Nilay Shroff]

On 7/16/26 1:59 PM, John Garry wrote:
> +#include <inttypes.h>
> +#include <sys/ioctl.h>
> +#include <linux/types.h>
> +
> +#ifndef _LINUX_NVME_IOCTL_H
> +#define _LINUX_NVME_IOCTL_H
> +#define NVME_IOCTL_ID		_IO('N', 0x40)
> +#endif /* _UAPI_LINUX_NVME_IOCTL_H */
> +
> +int main(int argc, char **argv)
> +{
> +	int fd, fd1;
> +	int count;
> +
> +	if (argc < 2) {
> +		fprintf(stderr, "usage: %s /dev/ngXnX", argv[0]);
> +		return EINVAL;
> +	}
> +
> +	fd = open(argv[1], O_RDONLY);
> +	if (fd < 0)
> +		return fd;
> +
> +	/*
> +	 * Steps:
> +	 * a. Signal to parent that we have opened the file so that it may
> +	 *    start the teardown.
> +	 * b. Try to open device until disallowed/gone.
> +	 * c. Sleep to allow nvme-subsystem be torn down.
> +	 * d. Issue the ioctl on original fd.
> +	 */
> +	kill(getppid(), SIGUSR2);
> +	count = 0;
> +	for (;;) {
> +		fd1 = open(argv[1], O_RDONLY);
> +		usleep(500000);
> +		if (fd1 < 0)
> +			break;

Does it make sense to check for errno set to ENODEV or ENOENT
before breaking out here?

Thanks,
--Nilay

Re: [PATCH v4] nvme/069: add a test for multipath cdev lifetime

[John Garry]

On 16/07/2026 16:16, Nilay Shroff wrote:
>> +	kill(getppid(), SIGUSR2);
>> +	count = 0;
>> +	for (;;) {
>> +		fd1 = open(argv[1], O_RDONLY);
>> +		usleep(500000);
>> +		if (fd1 < 0)
>> +			break;
> 
> Does it make sense to check for errno set to ENODEV or ENOENT
> before breaking out here?

ehh, if the errno is not ENODEV or ENOENT, then how to handle? Would it 
to fail the test due to unexpected errno?

Re: [PATCH v4] nvme/069: add a test for multipath cdev lifetime

[Nilay Shroff]

On 7/16/26 10:06 PM, John Garry wrote:
> On 16/07/2026 16:16, Nilay Shroff wrote:
>>> +    kill(getppid(), SIGUSR2);
>>> +    count = 0;
>>> +    for (;;) {
>>> +        fd1 = open(argv[1], O_RDONLY);
>>> +        usleep(500000);
>>> +        if (fd1 < 0)
>>> +            break;
>>
>> Does it make sense to check for errno set to ENODEV or ENOENT
>> before breaking out here?
> 
> ehh, if the errno is not ENODEV or ENOENT, then how to handle? Would it to fail the test due to unexpected errno?

I think yes, something like below:

for (;;) {
         fd1 = open(argv[1], O_RDONLY);
         if (fd1 < 0) {
                 if (errno == EINTR)
                         continue;

                 if (errno == ENOENT || errno == ENODEV)
                         break;

                 perror("open");
                 return EINVAL;
         }

         close(fd1);
         usleep(500000);

         if (++count > 10) {
                 fprintf(stderr, "%s still present\n", argv[0]);
                 return EINVAL;
         }
}

Thanks,
--Nilay

Re: [PATCH v4] nvme/069: add a test for multipath cdev lifetime

[John Garry]

On 17/07/2026 08:00, Nilay Shroff wrote:
> On 7/16/26 10: 06 PM, John Garry wrote: > On 16/07/2026 16: 16, Nilay Shroff
> wrote: >>> +    kill(getppid(), SIGUSR2); >>> +    count = 0; >>> +    for (;;)
> { >>> +        fd1 = open(argv[1], O_RDONLY); >>>
> 
> 
> On 7/16/26 10:06 PM, John Garry wrote:
>> On 16/07/2026 16:16, Nilay Shroff wrote:
>>>> +    kill(getppid(), SIGUSR2);
>>>> +    count = 0;
>>>> +    for (;;) {
>>>> +        fd1 = open(argv[1], O_RDONLY);
>>>> +        usleep(500000);
>>>> +        if (fd1 < 0)
>>>> +            break;
>>>
>>> Does it make sense to check for errno set to ENODEV or ENOENT
>>> before breaking out here?
>> 
>> ehh, if the errno is not ENODEV or ENOENT, then how to handle? Would it to fail the test due to unexpected errno?
> 
> I think yes, something like below:
> 
> for (;;) {
>            fd1 = open(argv[1], O_RDONLY);
>            if (fd1 < 0) {
>                    if (errno == EINTR)
>                            continue;
> 
>                    if (errno == ENOENT || errno == ENODEV)
>                            break;
> 
>                    perror("open");

if fd1 < 0, how is it open? Or maybe this message is too vague...

>                    return EINVAL;

So are ENOENT, ENODEV, and EINTR the only expected error codes? Others 
are just unsupported and we fail the test (if and when we get them), right?

>            }
> 
>            close(fd1);
>            usleep(500000);
> 
>            if (++count > 10) {
>                    fprintf(stderr, "%s still present\n", argv[0]);
>                    return EINVAL;
>            }
> }
> 
> Thanks,
> --Nilay
> 

Re: [PATCH v4] nvme/069: add a test for multipath cdev lifetime

[Nilay Shroff]

On 7/17/26 2:36 PM, John Garry wrote:
> On 17/07/2026 08:00, Nilay Shroff wrote:
>> On 7/16/26 10: 06 PM, John Garry wrote: > On 16/07/2026 16: 16, Nilay Shroff
>> wrote: >>> +    kill(getppid(), SIGUSR2); >>> +    count = 0; >>> +    for (;;)
>> { >>> +        fd1 = open(argv[1], O_RDONLY); >>>
>>
>>
>> On 7/16/26 10:06 PM, John Garry wrote:
>>> On 16/07/2026 16:16, Nilay Shroff wrote:
>>>>> +    kill(getppid(), SIGUSR2);
>>>>> +    count = 0;
>>>>> +    for (;;) {
>>>>> +        fd1 = open(argv[1], O_RDONLY);
>>>>> +        usleep(500000);
>>>>> +        if (fd1 < 0)
>>>>> +            break;
>>>>
>>>> Does it make sense to check for errno set to ENODEV or ENOENT
>>>> before breaking out here?
>>>
>>> ehh, if the errno is not ENODEV or ENOENT, then how to handle? Would it to fail the test due to unexpected errno?
>>
>> I think yes, something like below:
>>
>> for (;;) {
>>            fd1 = open(argv[1], O_RDONLY);
>>            if (fd1 < 0) {
>>                    if (errno == EINTR)
>>                            continue;
>>
>>                    if (errno == ENOENT || errno == ENODEV)
>>                            break;
>>
>>                    perror("open");
> 
> if fd1 < 0, how is it open? Or maybe this message is too vague...
> 
Yes, if fd1 < 0 then it implies errno is non-zero and device is not
opened. Later we evaluate the error code and take necessary
action.

>>                    return EINVAL;
> 
> So are ENOENT, ENODEV, and EINTR the only expected error codes? Others are just unsupported and we fail the test (if and when we get them), right?

Yes. My thinking is that, apart from EINTR, any failure from open() is an error.
For this particular test, we're specifically waiting until the device node
disappears, so ENOENT or ENODEV indicate the expected condition and we can
break out of the loop and issue the ioctl() on the original fd. Any other
errno would be unexpected, so I'd treat it as a test failure by logging the
error and exiting.

Thanks,
--Nilay