[PATCH v3] nvmet: add missing lock around nvmet_ns_changed in nvmet_ns_revalidate

[Niels Dossche]

nvmet_ns_changed states via lockdep that the ns->subsys->lock must be
held. The only caller of nvmet_ns_changed which does not acquire that
lock is nvmet_ns_revalidate. nvmet_ns_revalidate has 3 callers, of which
2 do not acquire that lock: nvmet_execute_identify_cns_cs_ns and
nvmet_execute_identify_ns. The other caller
nvmet_ns_revalidate_size_store does acquire the lock. Add a parameter to
nvmet_ns_revalidate to indicate whether the lock was already taken or
not, and thus whether the function still needs to take a lock when
calling nvmet_ns_changed.

The alternative solution is to let nvmet_ns_revalidate return a bool
which indicates whether nvmet_ns_changed needs to be called and let the
callers handle the locking responsibility. This however places the
responsibility with its callers and causes more duplicate code and
potential to forget to check the return value.

Both of those identify functions are called from a common function
nvmet_execute_identify, which itself is called indirectly via the
req->execute function pointer.

This issue was found using a static type-based analyser and manually
verified.

Signed-off-by: Niels Dossche <dossche.niels at gmail.com>
---

Changes in v3:
 - improve commit description
 - do the locking locally

Changes in v2:
 - added sentence about how the issue was found.
 - added missing &

 drivers/nvme/target/admin-cmd.c | 2 +-
 drivers/nvme/target/configfs.c  | 2 +-
 drivers/nvme/target/core.c      | 9 +++++++--
 drivers/nvme/target/nvmet.h     | 2 +-
 drivers/nvme/target/zns.c       | 3 ++-
 5 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index 6fb24746de06..efa462374783 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -511,7 +511,7 @@ static void nvmet_execute_identify_ns(struct nvmet_req *req)
 		goto done;
 	}
 
-	nvmet_ns_revalidate(req->ns);
+	nvmet_ns_revalidate(req->ns, true);
 
 	/*
 	 * nuse = ncap = nsze isn't always true, but we have no way to find
diff --git a/drivers/nvme/target/configfs.c b/drivers/nvme/target/configfs.c
index 091a0ca16361..a803cd66dc4b 100644
--- a/drivers/nvme/target/configfs.c
+++ b/drivers/nvme/target/configfs.c
@@ -586,7 +586,7 @@ static ssize_t nvmet_ns_revalidate_size_store(struct config_item *item,
 		mutex_unlock(&ns->subsys->lock);
 		return -EINVAL;
 	}
-	nvmet_ns_revalidate(ns);
+	nvmet_ns_revalidate(ns, false);
 	mutex_unlock(&ns->subsys->lock);
 	return count;
 }
diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
index 5119c687de68..0ceef97e4093 100644
--- a/drivers/nvme/target/core.c
+++ b/drivers/nvme/target/core.c
@@ -531,7 +531,7 @@ static void nvmet_p2pmem_ns_add_p2p(struct nvmet_ctrl *ctrl,
 		ns->nsid);
 }
 
-void nvmet_ns_revalidate(struct nvmet_ns *ns)
+void nvmet_ns_revalidate(struct nvmet_ns *ns, bool should_acquire_lock)
 {
 	loff_t oldsize = ns->size;
 
@@ -540,8 +540,13 @@ void nvmet_ns_revalidate(struct nvmet_ns *ns)
 	else
 		nvmet_file_ns_revalidate(ns);
 
-	if (oldsize != ns->size)
+	if (oldsize != ns->size) {
+		if (should_acquire_lock)
+			mutex_lock(&ns->subsys->lock);
 		nvmet_ns_changed(ns->subsys, ns->nsid);
+		if (should_acquire_lock)
+			mutex_unlock(&ns->subsys->lock);
+	}
 }
 
 int nvmet_ns_enable(struct nvmet_ns *ns)
diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
index af193423c10b..e4f20fe95613 100644
--- a/drivers/nvme/target/nvmet.h
+++ b/drivers/nvme/target/nvmet.h
@@ -542,7 +542,7 @@ u16 nvmet_file_flush(struct nvmet_req *req);
 void nvmet_ns_changed(struct nvmet_subsys *subsys, u32 nsid);
 void nvmet_bdev_ns_revalidate(struct nvmet_ns *ns);
 int nvmet_file_ns_revalidate(struct nvmet_ns *ns);
-void nvmet_ns_revalidate(struct nvmet_ns *ns);
+void nvmet_ns_revalidate(struct nvmet_ns *ns, bool should_acquire_lock);
 u16 blk_to_nvme_status(struct nvmet_req *req, blk_status_t blk_sts);
 
 bool nvmet_bdev_zns_enable(struct nvmet_ns *ns);
diff --git a/drivers/nvme/target/zns.c b/drivers/nvme/target/zns.c
index 46bc30fe85d2..1987358bc855 100644
--- a/drivers/nvme/target/zns.c
+++ b/drivers/nvme/target/zns.c
@@ -123,7 +123,8 @@ void nvmet_execute_identify_cns_cs_ns(struct nvmet_req *req)
 		goto done;
 	}
 
-	nvmet_ns_revalidate(req->ns);
+	nvmet_ns_revalidate(req->ns, true);
+
 	zsze = (bdev_zone_sectors(req->ns->bdev) << 9) >>
 					req->ns->blksize_shift;
 	id_zns->lbafe[0].zsze = cpu_to_le64(zsze);
-- 
2.35.1