[Bug Report] NVMe-oF/TCP - Slab OOB Read in `nvmet_ctrl_find_get`

[Chaitanya Kulkarni]

> Think of nvmet_copy_from_sgl as a memcpy from a different address
> space.  It works on a very different abstraction layer and thus
> doesn't even know about strings.  The callers really need to,
> and maybe we want a helper for that.  For now that patch below should
> fix the issue you reported, although for me KASAN doesn't trip up
> on the reproducer with or without the patch, so if you could test it
> on your setup that would be great:
>
> diff --git a/drivers/nvme/target/fabrics-cmd.c b/drivers/nvme/target/fabrics-cmd.c
> index 43b5bd8bb6a52d..0920fe7ce4ac99 100644
> --- a/drivers/nvme/target/fabrics-cmd.c
> +++ b/drivers/nvme/target/fabrics-cmd.c
> @@ -244,6 +244,8 @@ static void nvmet_execute_admin_connect(struct nvmet_req *req)
>   		goto out;
>   	}
>   
> +	d->subsysnqn[NVMF_NQN_FIELD_LEN] = '\0';
> +	d->hostnqn[NVMF_NQN_FIELD_LEN] = '\0';
>   	status = nvmet_alloc_ctrl(d->subsysnqn, d->hostnqn, req,
>   				  le32_to_cpu(c->kato), &ctrl);
>   	if (status)
> @@ -313,6 +315,8 @@ static void nvmet_execute_io_connect(struct nvmet_req *req)
>   		goto out;
>   	}
>   
> +	d->subsysnqn[NVMF_NQN_FIELD_LEN] = '\0';
> +	d->hostnqn[NVMF_NQN_FIELD_LEN] = '\0';
>   	ctrl = nvmet_ctrl_find_get(d->subsysnqn, d->hostnqn,
>   				   le16_to_cpu(d->cntlid), req);
>   	if (!ctrl) {

I specifically didn't send this patch when I wrote it initially, coz we are
modifying connect data and yes it will work just fine functionally but
in principal we should not touch connect data after nvmet_copy_from_sgl()
call even for the sake of debug prints or string comparison since it's
a data that we've received from host and should be in read-only mode.

please correct me if connect data should not be treated as read-only...

-ck