On Tue, May 29, 2018 at 04:59:05PM +0000, Verkamp, Daniel wrote:
> > + } else if (ctrl->nr_changed_ns == NVME_MAX_CHANGED_NAMESPACES) {
> > + ctrl->changed_ns_list[0] = cpu_to_le32(0xffffffff);
> > + }
>
> Unless I'm missing it happening somewhere else, the list-full case that sets element 0 to 0xffffffff should also explicitly zero out the rest of the list to satisfy the "remainder of the list shall be zero-filled" wording in the spec, since the other changed_ns_list entries will be filled with non-zero NSIDs when we get here.
True. We actually zero out unused elements already, but that doesn't
catch the ctrl->nr_changed_ns == NVME_MAX_CHANGED_NAMESPACES special
case. This relative patch should fix it:
diff --git a/drivers/nvme/target/admin-cmd.c b/drivers/nvme/target/admin-cmd.c
index d7b6293e830b..7b69c348d608 100644
--- a/drivers/nvme/target/admin-cmd.c
+++ b/drivers/nvme/target/admin-cmd.c
@@ -136,7 +136,10 @@ static void nvmet_execute_get_log_changed_ns(struct nvmet_req *req)
goto out;
mutex_lock(&ctrl->lock);
- len = ctrl->nr_changed_ns * sizeof(__le32);
+ if (ctrl->nr_changed_ns == NVME_MAX_CHANGED_NAMESPACES)
+ len = sizeof(__le32);
+ else
+ len = ctrl->nr_changed_ns * sizeof(__le32);
status = nvmet_copy_to_sgl(req, 0, ctrl->changed_ns_list, len);
if (!status)
status = nvmet_zero_sgl(req, len, req->data_len - len);
