nvmet_subsys_attr_qid_max_store() sets subsys->max_qid and then walks subsys->ctrls to force every controller to reconnect, but it holds only nvmet_config_sem (for write). subsys->ctrls is protected by subsys->lock: nvmet_alloc_ctrl() adds an entry under subsys->lock, and nvmet_ctrl_free() removes it under subsys->lock with list_del() + kfree() (not kfree_rcu). The free path takes no nvmet_config_sem, so the two locks are independent. A controller teardown (host disconnect, keep-alive timeout or fatal error -> nvmet_sq_destroy() -> nvmet_ctrl_put() -> nvmet_ctrl_free()) can therefore list_del() an entry while qid_max_store() is iterating the list, corrupting the walk. KASAN, concurrent "echo 128 > .../subsystems//attr_qid_max" racing nvme-loop controllers connecting and disconnecting, panics in seconds: BUG: KASAN: null-ptr-deref in nvmet_subsys_attr_qid_max_store+0x10d/0x1b0 Read of size 8 at addr 0000000000000090 Call Trace: nvmet_subsys_attr_qid_max_store+0x10d/0x1b0 configfs_write_iter+0x1a7/0x250 vfs_write+0x5df/0x940 ksys_write+0xfd/0x1e0 (also seen as a general-protection fault dereferencing a LIST_POISON pointer at the same instruction.) Take subsys->lock around the list_for_each_entry() loop, matching every other subsys->ctrls accessor. ->delete_ctrl() only queues asynchronous controller teardown, so it does not re-enter subsys->lock under the loop. Fixes: 3e980f5995e0 ("nvmet: expose max queues to configfs") Cc: stable@vger.kernel.org Signed-off-by: Yu Zhang Tested-by: Yu Zhang --- drivers/nvme/target/configfs.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c index 1111111..2222222 100644 --- a/drivers/nvme/target/configfs.c +++ b/drivers/nvme/target/configfs.c @@ -1683,9 +1683,16 @@ static ssize_t nvmet_subsys_attr_qid_max_store(struct config_item *item, down_write(&nvmet_config_sem); subsys->max_qid = qid_max; - /* Force reconnect */ + /* + * Force reconnect. subsys->ctrls is protected by subsys->lock + * (nvmet_alloc_ctrl()/nvmet_ctrl_free() add and remove entries under + * it); take it here too so a concurrent controller teardown cannot + * list_del() an entry while this loop walks the list. + */ + mutex_lock(&subsys->lock); list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry) ctrl->ops->delete_ctrl(ctrl); + mutex_unlock(&subsys->lock); up_write(&nvmet_config_sem); return cnt; -- 2.43.0