[PATCH] nvme: validate maxcmd before use it

[PATCH] nvme: validate maxcmd before use it

[Guixin Liu]

According to the NVMe specification, if the 'maximum outstanding
commands' (maxcmd) field is not utilized, it must be set to 0h.
When a host connects to a target that does not employ the maxcmd
field, the queue size (sqsize) gets set to an excessively large
value due to the calculation (u16)(0 - 1) = 65535.
As a result, such a misconfiguration leads to the failure of the
connection establishment.

Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/nvme/host/fc.c   | 3 ++-
 drivers/nvme/host/rdma.c | 3 ++-
 drivers/nvme/host/tcp.c  | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/host/fc.c b/drivers/nvme/host/fc.c
index fb22976..8ddbeef 100644
--- a/drivers/nvme/host/fc.c
+++ b/drivers/nvme/host/fc.c
@@ -3156,7 +3156,8 @@ static void nvme_fc_map_queues(struct blk_mq_tag_set *set)
 		goto out_stop_keep_alive;
 	}
 
-	if (opts->queue_size > ctrl->ctrl.maxcmd) {
+	if (ctrl->ctrl.maxcmd &&
+	    opts->queue_size > ctrl->ctrl.maxcmd) {
 		/* warn if maxcmd is lower than queue_size */
 		dev_warn(ctrl->ctrl.device,
 			"queue_size %zu > ctrl maxcmd %u, reducing "
diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
index 81e2621..597df7c 100644
--- a/drivers/nvme/host/rdma.c
+++ b/drivers/nvme/host/rdma.c
@@ -1037,7 +1037,8 @@ static int nvme_rdma_setup_ctrl(struct nvme_rdma_ctrl *ctrl, bool new)
 		ctrl->ctrl.sqsize = NVME_RDMA_MAX_QUEUE_SIZE - 1;
 	}
 
-	if (ctrl->ctrl.sqsize + 1 > ctrl->ctrl.maxcmd) {
+	if (ctrl->ctrl.maxcmd &&
+	    ctrl->ctrl.sqsize + 1 > ctrl->ctrl.maxcmd) {
 		dev_warn(ctrl->ctrl.device,
 			"sqsize %u > ctrl maxcmd %u, clamping down\n",
 			ctrl->ctrl.sqsize + 1, ctrl->ctrl.maxcmd);
diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c
index 08805f0..12b00ad 100644
--- a/drivers/nvme/host/tcp.c
+++ b/drivers/nvme/host/tcp.c
@@ -2197,7 +2197,8 @@ static int nvme_tcp_setup_ctrl(struct nvme_ctrl *ctrl, bool new)
 			"queue_size %zu > ctrl sqsize %u, clamping down\n",
 			opts->queue_size, ctrl->sqsize + 1);
 
-	if (ctrl->sqsize + 1 > ctrl->maxcmd) {
+	if (ctrl->maxcmd &&
+	    ctrl->sqsize + 1 > ctrl->maxcmd) {
 		dev_warn(ctrl->device,
 			"sqsize %u > ctrl maxcmd %u, clamping down\n",
 			ctrl->sqsize + 1, ctrl->maxcmd);
-- 
1.8.3.1

Re: [PATCH] nvme: validate maxcmd before use it

[Hannes Reinecke]

On 12/15/23 09:43, Guixin Liu wrote:
> According to the NVMe specification, if the 'maximum outstanding
> commands' (maxcmd) field is not utilized, it must be set to 0h.
> When a host connects to a target that does not employ the maxcmd
> field, the queue size (sqsize) gets set to an excessively large
> value due to the calculation (u16)(0 - 1) = 65535.
> As a result, such a misconfiguration leads to the failure of the
> connection establishment.
> 
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---
>   drivers/nvme/host/fc.c   | 3 ++-
>   drivers/nvme/host/rdma.c | 3 ++-
>   drivers/nvme/host/tcp.c  | 3 ++-
>   3 files changed, 6 insertions(+), 3 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes

Re: [PATCH] nvme: validate maxcmd before use it

[Christoph Hellwig]

On Fri, Dec 15, 2023 at 04:43:28PM +0800, Guixin Liu wrote:
> According to the NVMe specification, if the 'maximum outstanding
> commands' (maxcmd) field is not utilized, it must be set to 0h.
> When a host connects to a target that does not employ the maxcmd
> field, the queue size (sqsize) gets set to an excessively large
> value due to the calculation (u16)(0 - 1) = 65535.
> As a result, such a misconfiguration leads to the failure of the
> connection establishment.

MAXCMD is mandatory for Fabrics.

Re: [PATCH] nvme: validate maxcmd before use it

[Sagi Grimberg]

>> According to the NVMe specification, if the 'maximum outstanding
>> commands' (maxcmd) field is not utilized, it must be set to 0h.
>> When a host connects to a target that does not employ the maxcmd
>> field, the queue size (sqsize) gets set to an excessively large
>> value due to the calculation (u16)(0 - 1) = 65535.
>> As a result, such a misconfiguration leads to the failure of the
>> connection establishment.
> 
> MAXCMD is mandatory for Fabrics.
> 

Indeed.