Re: [PATCH 4/4] nvme: check that EUI/GUID/UUID are globally unique

Keith Busch
2022-06-10T14:12:28Z

On Fri, Jun 10, 2022 at 12:27:24AM +0000, Alan Adamson wrote:
> +static u16 nvmet_passthru_override_id_descs(struct nvmet_req *req)
> +{
> + struct nvmet_ctrl *ctrl = req->sq->ctrl;
> + struct nvme_ns_id_desc *data, *cur;
> + u16 status = NVME_SC_SUCCESS;
> +
> + if (!(ctrl->ops->flags & NVMF_CLEAR_NS_DESCS))
> + return status;
> +
> + data = kzalloc(0x1000, GFP_KERNEL);
> + if (!data)
> + return NVME_SC_INTERNAL;
> +
> + status = nvmet_copy_from_sgl(req, 0, data, 0x1000);
> + if (status)
> + goto out_free;
> +
> + cur = data;
> + cur->nidt = NVME_NIDT_CSI;
> + cur->nidl = NVME_NIDT_CSI_LEN;
> + cur++;
> + cur->nidt = 0;

I don't think the above is correct without setting the CSI value. It's just
going to get whatever the controller happened to return at this offset, which
may be a completely differnt identifier type. I think you'd actually need to
search the descriptor list for the NIDT_CSI field and then copy just that one
into what gets returned.

And the "cur++" is just going to move the pointer past the descriptor header,
but doesn't include the descriptor's total length, so setting cur->nidt is
going to corrupt the actual descriptor. You have to add the previous's NIDL to
the cur address.

Otherwise, the rest looks fine.