[PATCH] nvmet: protect nvmet_ports_list with nvmet_config_sem

[PATCH] nvmet: protect nvmet_ports_list with nvmet_config_sem

[Qing Zhang]

>From 554233f4165842a0a104896c30ed4d2e89c11931 Mon Sep 17 00:00:00 2001
From: Qing Zhang <diasyzhang@tencent.com>
Date: Fri, 23 Oct 2026 19:29:28 +0800
Subject: [PATCH] nvmet: protect nvmet_ports_list with nvmet_config_sem

Readers of nvmet_ports_list (e.g. nvmet_subsys_disc_changed()) hold
nvmet_config_sem and assert it via lockdep.  The writers --
nvmet_ports_make() and nvmet_port_release() -- do not take the lock;
they rely on the parent directory's VFS inode lock for serialization,
which is an implementation detail of configfs and not a guaranteed
contract.  Align the writers with the lock the readers already require.

Also move list_add() in nvmet_ports_make() to after the port is fully
initialized: the old position published the port on the global list
before INIT_LIST_HEAD(&port->subsystems), so a concurrent reader
could traverse uninitialized list heads.

flush_workqueue() in nvmet_port_release() stays outside the semaphore
to avoid a lock dependency on nvmet_wq.

Fixes: b662a078576e ("nvmet: enable Discovery Controller AENs")
Signed-off-by: Qing Zhang <diasyzhang@tencent.com>
---
 drivers/nvme/target/configfs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index b03b5d1c2dc3..75cb9e63025f 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -2009,7 +2009,9 @@ static void nvmet_port_release(struct config_item *item)
 
 	/* Let inflight controllers teardown complete */
 	flush_workqueue(nvmet_wq);
+	down_write(&nvmet_config_sem);
 	list_del(&port->global_entry);
+	up_write(&nvmet_config_sem);
 
 	key_put(port->keyring);
 	kfree(port);
@@ -2070,8 +2072,6 @@ static struct config_group *nvmet_ports_make(struct config_group *group,
 			port->ana_state[i] = NVME_ANA_INACCESSIBLE;
 	}
 
-	list_add(&port->global_entry, &nvmet_ports_list);
-
 	INIT_LIST_HEAD(&port->entry);
 	INIT_LIST_HEAD(&port->subsystems);
 	INIT_LIST_HEAD(&port->referrals);
@@ -2105,6 +2105,10 @@ static struct config_group *nvmet_ports_make(struct config_group *group,
 	configfs_add_default_group(&port->ana_default_group.group,
 			&port->ana_groups_group);
 
+	down_write(&nvmet_config_sem);
+	list_add(&port->global_entry, &nvmet_ports_list);
+	up_write(&nvmet_config_sem);
+
 	return &port->group;
 }
 
-- 
2.43.7

Re: [PATCH] nvmet: protect nvmet_ports_list with nvmet_config_sem

[Maurizio Lombardi]

On Fri Oct 23, 2026 at 1:36 PM CEST, Qing Zhang wrote:
>>From 554233f4165842a0a104896c30ed4d2e89c11931 Mon Sep 17 00:00:00 2001
> From: Qing Zhang <diasyzhang@tencent.com>
> Date: Fri, 23 Oct 2026 19:29:28 +0800
> Subject: [PATCH] nvmet: protect nvmet_ports_list with nvmet_config_sem
>
> Readers of nvmet_ports_list (e.g. nvmet_subsys_disc_changed()) hold
> nvmet_config_sem and assert it via lockdep.  The writers --
> nvmet_ports_make() and nvmet_port_release() -- do not take the lock;
> they rely on the parent directory's VFS inode lock for serialization,
> which is an implementation detail of configfs and not a guaranteed
> contract.  Align the writers with the lock the readers already require.
>
> Also move list_add() in nvmet_ports_make() to after the port is fully
> initialized: the old position published the port on the global list
> before INIT_LIST_HEAD(&port->subsystems), so a concurrent reader
> could traverse uninitialized list heads.
>
> flush_workqueue() in nvmet_port_release() stays outside the semaphore
> to avoid a lock dependency on nvmet_wq.
>
> Fixes: b662a078576e ("nvmet: enable Discovery Controller AENs")
> Signed-off-by: Qing Zhang <diasyzhang@tencent.com>
> ---
>  drivers/nvme/target/configfs.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
> index b03b5d1c2dc3..75cb9e63025f 100644
> --- a/drivers/nvme/target/configfs.c
> +++ b/drivers/nvme/target/configfs.c
> @@ -2009,7 +2009,9 @@ static void nvmet_port_release(struct config_item *item)
>  
>  	/* Let inflight controllers teardown complete */
>  	flush_workqueue(nvmet_wq);
> +	down_write(&nvmet_config_sem);
>  	list_del(&port->global_entry);
> +	up_write(&nvmet_config_sem);
>  
>  	key_put(port->keyring);
>  	kfree(port);
> @@ -2070,8 +2072,6 @@ static struct config_group *nvmet_ports_make(struct config_group *group,
>  			port->ana_state[i] = NVME_ANA_INACCESSIBLE;
>  	}
>  
> -	list_add(&port->global_entry, &nvmet_ports_list);
> -
>  	INIT_LIST_HEAD(&port->entry);
>  	INIT_LIST_HEAD(&port->subsystems);
>  	INIT_LIST_HEAD(&port->referrals);
> @@ -2105,6 +2105,10 @@ static struct config_group *nvmet_ports_make(struct config_group *group,
>  	configfs_add_default_group(&port->ana_default_group.group,
>  			&port->ana_groups_group);
>  
> +	down_write(&nvmet_config_sem);
> +	list_add(&port->global_entry, &nvmet_ports_list);
> +	up_write(&nvmet_config_sem);
> +
>  	return &port->group;
>  }
>  

Yes, I was already planning to submit the same fix as part of this
larger patch: https://lore.kernel.org/linux-nvme/DLOGQG9GZC3X.1UZ2MY7S3T5N2@arkamax.eu/
but I didn't have time to finish the tests and submit it.

Reviewed-by: Maurizio Lombardi <mlombard@redhat.com>

Re: [PATCH] nvmet: protect nvmet_ports_list with nvmet_config_sem

[Keith Busch]

On Fri, Oct 23, 2026 at 07:36:28PM +0800, Qing Zhang wrote:
> >From 554233f4165842a0a104896c30ed4d2e89c11931 Mon Sep 17 00:00:00 2001
> From: Qing Zhang <diasyzhang@tencent.com>
> Date: Fri, 23 Oct 2026 19:29:28 +0800
> Subject: [PATCH] nvmet: protect nvmet_ports_list with nvmet_config_sem
> 
> Readers of nvmet_ports_list (e.g. nvmet_subsys_disc_changed()) hold
> nvmet_config_sem and assert it via lockdep.  The writers --
> nvmet_ports_make() and nvmet_port_release() -- do not take the lock;
> they rely on the parent directory's VFS inode lock for serialization,
> which is an implementation detail of configfs and not a guaranteed
> contract.  Align the writers with the lock the readers already require.
> 
> Also move list_add() in nvmet_ports_make() to after the port is fully
> initialized: the old position published the port on the global list
> before INIT_LIST_HEAD(&port->subsystems), so a concurrent reader
> could traverse uninitialized list heads.
> 
> flush_workqueue() in nvmet_port_release() stays outside the semaphore
> to avoid a lock dependency on nvmet_wq.

Thanks, applied to nvme-7.4.