Commit 5e1f6899 authored by Christoph Hellwig's avatar Christoph Hellwig
Browse files

nvme-multipath: fix double initialization of ANA state 



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: 0d0b660f ("nvme: add ANA support")
Reported-by: default avatarMartin Wilck <mwilck@suse.com>
Signed-off-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarKeith Busch <kbusch@kernel.org>
Reviewed-by: default avatarSagi Grimberg <sagi@grimberg.me>
Reviewed-by: default avatarHannes Reinecke <hare@suse.de>
parent efed9a33

drivers/nvme/host/core.c

+2 −1
Original line number Diff line number Diff line
@@ -2901,7 +2901,7 @@ static int nvme_init_identify(struct nvme_ctrl *ctrl) 
		ctrl->hmmaxd = le16_to_cpu(id->hmmaxd);

	}



	ret = nvme_mpath_init(ctrl, id);

	ret = nvme_mpath_init_identify(ctrl, id);

	if (ret < 0)

		goto out_free;
@@ -4364,6 +4364,7 @@ int nvme_init_ctrl(struct nvme_ctrl *ctrl, struct device *dev, 
		min(default_ps_max_latency_us, (unsigned long)S32_MAX));



	nvme_fault_inject_init(&ctrl->fault_inject, dev_name(ctrl->device));

	nvme_mpath_init_ctrl(ctrl);



	return 0;

out_free_name:

drivers/nvme/host/multipath.c

+29 −26
Original line number Diff line number Diff line
@@ -781,9 +781,18 @@ void nvme_mpath_remove_disk(struct nvme_ns_head *head) 
	put_disk(head->disk);

}



int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)

void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl)

{

	int error;

	mutex_init(&ctrl->ana_lock);

	timer_setup(&ctrl->anatt_timer, nvme_anatt_timeout, 0);

	INIT_WORK(&ctrl->ana_work, nvme_ana_work);

}



int nvme_mpath_init_identify(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id)

{

	size_t max_transfer_size = ctrl->max_hw_sectors << SECTOR_SHIFT;

	size_t ana_log_size;

	int error = 0;



	/* check if multipath is enabled and we have the capability */

	if (!multipath || !ctrl->subsys ||
@@ -795,37 +804,31 @@ int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id) 
	ctrl->nanagrpid = le32_to_cpu(id->nanagrpid);

	ctrl->anagrpmax = le32_to_cpu(id->anagrpmax);



	mutex_init(&ctrl->ana_lock);

	timer_setup(&ctrl->anatt_timer, nvme_anatt_timeout, 0);

	ctrl->ana_log_size = sizeof(struct nvme_ana_rsp_hdr) +

		ctrl->nanagrpid * sizeof(struct nvme_ana_group_desc);

	ctrl->ana_log_size += ctrl->max_namespaces * sizeof(__le32);



	if (ctrl->ana_log_size > ctrl->max_hw_sectors << SECTOR_SHIFT) {

	ana_log_size = sizeof(struct nvme_ana_rsp_hdr) +

		ctrl->nanagrpid * sizeof(struct nvme_ana_group_desc) +

		ctrl->max_namespaces * sizeof(__le32);

	if (ana_log_size > max_transfer_size) {

		dev_err(ctrl->device,

			"ANA log page size (%zd) larger than MDTS (%d).\n",

			ctrl->ana_log_size,

			ctrl->max_hw_sectors << SECTOR_SHIFT);

			"ANA log page size (%zd) larger than MDTS (%zd).\n",

			ana_log_size, max_transfer_size);

		dev_err(ctrl->device, "disabling ANA support.\n");

		return 0;

		goto out_uninit;

	}



	INIT_WORK(&ctrl->ana_work, nvme_ana_work);

	if (ana_log_size > ctrl->ana_log_size) {

		nvme_mpath_stop(ctrl);

		kfree(ctrl->ana_log_buf);

		ctrl->ana_log_buf = kmalloc(ctrl->ana_log_size, GFP_KERNEL);

	if (!ctrl->ana_log_buf) {

		error = -ENOMEM;

		goto out;

		if (!ctrl->ana_log_buf)

			return -ENOMEM;

	}



	ctrl->ana_log_size = ana_log_size;

	error = nvme_read_ana_log(ctrl);

	if (error)

		goto out_free_ana_log_buf;

		goto out_uninit;

	return 0;

out_free_ana_log_buf:

	kfree(ctrl->ana_log_buf);

	ctrl->ana_log_buf = NULL;

out:



out_uninit:

	nvme_mpath_uninit(ctrl);

	return error;

}

drivers/nvme/host/nvme.h

+6 −2
Original line number Diff line number Diff line
@@ -712,7 +712,8 @@ void nvme_kick_requeue_lists(struct nvme_ctrl *ctrl); 
int nvme_mpath_alloc_disk(struct nvme_ctrl *ctrl,struct nvme_ns_head *head);

void nvme_mpath_add_disk(struct nvme_ns *ns, struct nvme_id_ns *id);

void nvme_mpath_remove_disk(struct nvme_ns_head *head);

int nvme_mpath_init(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id);

int nvme_mpath_init_identify(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id);

void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl);

void nvme_mpath_uninit(struct nvme_ctrl *ctrl);

void nvme_mpath_stop(struct nvme_ctrl *ctrl);

bool nvme_mpath_clear_current_path(struct nvme_ns *ns);
@@ -780,7 +781,10 @@ static inline void nvme_mpath_check_last_path(struct nvme_ns *ns) 
static inline void nvme_trace_bio_complete(struct request *req)

{

}

static inline int nvme_mpath_init(struct nvme_ctrl *ctrl,

static inline void nvme_mpath_init_ctrl(struct nvme_ctrl *ctrl)

{

}

static inline int nvme_mpath_init_identify(struct nvme_ctrl *ctrl,

		struct nvme_id_ctrl *id)

{

	if (ctrl->subsys->cmic & NVME_CTRL_CMIC_ANA)