[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.

Re: [PATCH] nvmet: protect nvmet_ports_list with nvmet_config_sem

[Christoph Hellwig]

Btw, I had some old patches here:

https://git.infradead.org/?p=users/hch/misc.git;a=shortlog;h=refs/heads/cap-analysis

to add clang lock context analysis (still called capability analysis
back then to nvmet) to annotate and verify the locking scheme.  It
would be great if someone could take on that work.