[PATCH] nvmet/tcp: Solve coredump issue while initiator sends write CapsuleCmd

Sagi Grimberg sagi at grimberg.me
Fri Aug 21 01:57:45 EDT 2020


Hey Ziye,

> When the initiator sneds write CapsuleCmd without incapsuledata, we
		     sends                          in-capsule data

> can use the queue->connect command to handle. If we do not do that,
> we will have a coredump because queue->cmds are not initialized.

This issue is specific to the I/O queue connect where the payload
comes solicited via r2t.

I suggest to document the change log to the following:

nvmet-tcp: Fix NULL dereference when a connect data comes in h2cdata pdu

When handling commands without in-capsule data, we assign the ttag
assuming we already have the queue commands array allocated (based on
the queue size information in the connect data payload).

However if the connect itself did not send the connect data in-capsule
we have yet to allocate the queue commands, and we will assign a bogus
ttag and suffer a NULL dereference when we receive the corresponding
h2cdata pdu.

Fix this by checking if we already allocated commands before
dereferencing it when handling h2cdata, if we didn't, its for sure
a connect and we should use the preallocated connect command.

> Signed-off-by: Ziye Yang <ziye.yang at intel.com>
> ---
>   drivers/nvme/target/tcp.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
> index 9eda91162fe4..9d3eeb9338cc 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -866,7 +866,11 @@ static int nvmet_tcp_handle_h2c_data_pdu(struct nvmet_tcp_queue *queue)
>   	struct nvme_tcp_data_pdu *data = &queue->pdu.data;
>   	struct nvmet_tcp_cmd *cmd;
>   
> -	cmd = &queue->cmds[data->ttag];
> +	if (likely(queue->nr_cmds)) {
> +		cmd = &queue->cmds[data->ttag];
> +	} else {
> +		cmd = &queue->connect;
> +	}

No need for brackets on one-liner conditionals.

Also, I think it'd make sense to set the ttag to 0xffff if we hit
this condition, otherwise we are sending a bogus ttag.

Something like:
--
diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index dfcbc6522aba..3fec5119ec90 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -159,6 +159,10 @@ static void nvmet_tcp_finish_cmd(struct 
nvmet_tcp_cmd *cmd);
  static inline u16 nvmet_tcp_cmd_tag(struct nvmet_tcp_queue *queue,
                 struct nvmet_tcp_cmd *cmd)
  {
+       if (unlikely(cmd == queue->connect)) {
+               /* We didn't allocate cmds yet, send 0xffff */
+               return USHRT_MAX;
+       }
         return cmd - queue->cmds;
  }
--



More information about the Linux-nvme mailing list