Skip to content

Commit 7b994b0

Browse files
Christoph Hellwiggregkh
authored andcommitted
nvme-multipath: fix double initialization of ANA state
commit 5e1f689 upstream. nvme_init_identify and thus nvme_mpath_init can be called multiple times and thus must not overwrite potentially initialized or in-use fields. Split out a helper for the basic initialization when the controller is initialized and make sure the init_identify path does not blindly change in-use data structures. Fixes: 0d0b660 ("nvme: add ANA support") Reported-by: Martin Wilck <[email protected]> Signed-off-by: Christoph Hellwig <[email protected]> Reviewed-by: Keith Busch <[email protected]> Reviewed-by: Sagi Grimberg <[email protected]> Reviewed-by: Hannes Reinecke <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent e2c26dd commit 7b994b0

File tree

3 files changed

+37
-29
lines changed

3 files changed

+37
-29
lines changed

drivers/nvme/host/core.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3131,7 +3131,7 @@ int nvme_init_identify(struct nvme_ctrl *ctrl)
31313131
ctrl->hmmaxd = le16_to_cpu(id->hmmaxd);
31323132
}
31333133

3134-
ret = nvme_mpath_init(ctrl, id);
3134+
ret = nvme_mpath_init_identify(ctrl, id);
31353135
kfree(id);
31363136

31373137
if (ret < 0)
@@ -4517,6 +4517,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev,
45174517
min(default_ps_max_latency_us, (unsigned long)S32_MAX));
45184518

45194519
nvme_fault_inject_init(&ctrl->fault_inject, dev_name(ctrl->device));
4520+
nvme_mpath_init_ctrl(ctrl);
45204521

45214522
return 0;
45224523
out_free_name:

drivers/nvme/host/multipath.c

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -708,9 +708,18 @@ void nvme_mpath_remove_disk(struct nvme_ns_head *head)
708708
put_disk(head->disk);
709709
}
710710

711-
int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
711+
void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl)
712712
{
713-
int error;
713+
mutex_init(&ctrl->ana_lock);
714+
timer_setup(&ctrl->anatt_timer, nvme_anatt_timeout, 0);
715+
INIT_WORK(&ctrl->ana_work, nvme_ana_work);
716+
}
717+
718+
int nvme_mpath_init_identify(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
719+
{
720+
size_t max_transfer_size = ctrl->max_hw_sectors << SECTOR_SHIFT;
721+
size_t ana_log_size;
722+
int error = 0;
714723

715724
/* check if multipath is enabled and we have the capability */
716725
if (!multipath || !ctrl->subsys ||
@@ -722,37 +731,31 @@ int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)
722731
ctrl->nanagrpid = le32_to_cpu(id->nanagrpid);
723732
ctrl->anagrpmax = le32_to_cpu(id->anagrpmax);
724733

725-
mutex_init(&ctrl->ana_lock);
726-
timer_setup(&ctrl->anatt_timer, nvme_anatt_timeout, 0);
727-
ctrl->ana_log_size = sizeof(struct nvme_ana_rsp_hdr) +
728-
ctrl->nanagrpid * sizeof(struct nvme_ana_group_desc);
729-
ctrl->ana_log_size += ctrl->max_namespaces * sizeof(__le32);
730-
731-
if (ctrl->ana_log_size > ctrl->max_hw_sectors << SECTOR_SHIFT) {
734+
ana_log_size = sizeof(struct nvme_ana_rsp_hdr) +
735+
ctrl->nanagrpid * sizeof(struct nvme_ana_group_desc) +
736+
ctrl->max_namespaces * sizeof(__le32);
737+
if (ana_log_size > max_transfer_size) {
732738
dev_err(ctrl->device,
733-
"ANA log page size (%zd) larger than MDTS (%d).\n",
734-
ctrl->ana_log_size,
735-
ctrl->max_hw_sectors << SECTOR_SHIFT);
739+
"ANA log page size (%zd) larger than MDTS (%zd).\n",
740+
ana_log_size, max_transfer_size);
736741
dev_err(ctrl->device, "disabling ANA support.\n");
737-
return 0;
742+
goto out_uninit;
738743
}
739-
740-
INIT_WORK(&ctrl->ana_work, nvme_ana_work);
741-
kfree(ctrl->ana_log_buf);
742-
ctrl->ana_log_buf = kmalloc(ctrl->ana_log_size, GFP_KERNEL);
743-
if (!ctrl->ana_log_buf) {
744-
error = -ENOMEM;
745-
goto out;
744+
if (ana_log_size > ctrl->ana_log_size) {
745+
nvme_mpath_stop(ctrl);
746+
kfree(ctrl->ana_log_buf);
747+
ctrl->ana_log_buf = kmalloc(ctrl->ana_log_size, GFP_KERNEL);
748+
if (!ctrl->ana_log_buf)
749+
return -ENOMEM;
746750
}
747-
751+
ctrl->ana_log_size = ana_log_size;
748752
error = nvme_read_ana_log(ctrl);
749753
if (error)
750-
goto out_free_ana_log_buf;
754+
goto out_uninit;
751755
return 0;
752-
out_free_ana_log_buf:
753-
kfree(ctrl->ana_log_buf);
754-
ctrl->ana_log_buf = NULL;
755-
out:
756+
757+
out_uninit:
758+
nvme_mpath_uninit(ctrl);
756759
return error;
757760
}
758761

drivers/nvme/host/nvme.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,8 @@ void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl);
654654
int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl,struct nvme_ns_head *head);
655655
void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id);
656656
void nvme_mpath_remove_disk(struct nvme_ns_head *head);
657-
int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id);
657+
int nvme_mpath_init_identify(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id);
658+
void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl);
658659
void nvme_mpath_uninit(struct nvme_ctrl *ctrl);
659660
void nvme_mpath_stop(struct nvme_ctrl *ctrl);
660661
bool nvme_mpath_clear_current_path(struct nvme_ns *ns);
@@ -730,7 +731,10 @@ static inline void nvme_trace_bio_complete(struct request *req,
730731
blk_status_t status)
731732
{
732733
}
733-
static inline int nvme_mpath_init(struct nvme_ctrl *ctrl,
734+
static inline void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl)
735+
{
736+
}
737+
static inline int nvme_mpath_init_identify(struct nvme_ctrl *ctrl,
734738
struct nvme_id_ctrl *id)
735739
{
736740
if (ctrl->subsys->cmic & (1 << 3))

0 commit comments

Comments
 (0)