[PATCH] qed: iscsi: limit command queue array fill to the ramrod array size

[PATCH] qed: iscsi: limit command queue array fill to the ramrod array size

[Pengpeng Hou]

qed_sp_iscsi_func_start() validates p_params->num_queues against the
number of command queues exposed by the hardware resource table, but it
then uses that count to fill q_params.cq_cmdq_sb_num_arr[] in the SCSI
init ramrod. That array is fixed at SCSI_MAX_NUM_OF_CMDQS entries.

The in-tree qedi caller derives num_queues from the device's reported
num_cqs and the online CPU count, so current-tree callers can request
more queues than fit in the fixed ramrod array on sufficiently large
systems even though the existing hardware-resource check passes.

Reject queue counts that exceed the command queue array capacity before
filling the ramrod.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/net/ethernet/qlogic/qed/qed_iscsi.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
index e16808291338..62cf85f9a042 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
@@ -177,6 +177,15 @@ qed_sp_iscsi_func_start(struct qed_hwfn *p_hwfn,
 		return -EINVAL;
 	}
 
+	if (p_params->num_queues > ARRAY_SIZE(p_queue->cq_cmdq_sb_num_arr)) {
+		DP_ERR(p_hwfn,
+		       "Cannot fit %u queues in %zu command queue slots. Aborting function start\n",
+		       p_params->num_queues,
+		       ARRAY_SIZE(p_queue->cq_cmdq_sb_num_arr));
+		qed_sp_destroy_request(p_hwfn, p_ent);
+		return -EINVAL;
+	}
+
 	val = p_params->half_way_close_timeout;
 	p_init->half_way_close_timeout = cpu_to_le16(val);
 	p_init->num_sq_pages_in_ring = p_params->num_sq_pages_in_ring;
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] qed: iscsi: limit command queue array fill to the ramrod array size

[Simon Horman]

On Tue, Mar 24, 2026 at 06:56:29PM +0800, Pengpeng Hou wrote:
> qed_sp_iscsi_func_start() validates p_params->num_queues against the
> number of command queues exposed by the hardware resource table, but it
> then uses that count to fill q_params.cq_cmdq_sb_num_arr[] in the SCSI
> init ramrod. That array is fixed at SCSI_MAX_NUM_OF_CMDQS entries.
> 
> The in-tree qedi caller derives num_queues from the device's reported
> num_cqs and the online CPU count, so current-tree callers can request
> more queues than fit in the fixed ramrod array on sufficiently large
> systems even though the existing hardware-resource check passes.
> 
> Reject queue counts that exceed the command queue array capacity before
> filling the ramrod.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
>  drivers/net/ethernet/qlogic/qed/qed_iscsi.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
> index e16808291338..62cf85f9a042 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_iscsi.c
> @@ -177,6 +177,15 @@ qed_sp_iscsi_func_start(struct qed_hwfn *p_hwfn,
>  		return -EINVAL;
>  	}

Hi,

As per my comment to your similar patch for fcoe.

The code immediately above checks fcoe_pf_params->num_cqs
against p_hwfn->hw_info.feat_num[QED_ISCSI_CQ], which I assume
is derived from hardware.

Is that sufficient to avoid the OOB access you describe?

>  
> +	if (p_params->num_queues > ARRAY_SIZE(p_queue->cq_cmdq_sb_num_arr)) {
> +		DP_ERR(p_hwfn,
> +		       "Cannot fit %u queues in %zu command queue slots. Aborting function start\n",
> +		       p_params->num_queues,
> +		       ARRAY_SIZE(p_queue->cq_cmdq_sb_num_arr));
> +		qed_sp_destroy_request(p_hwfn, p_ent);
> +		return -EINVAL;
> +	}
> +
>  	val = p_params->half_way_close_timeout;
>  	p_init->half_way_close_timeout = cpu_to_le16(val);
>  	p_init->num_sq_pages_in_ring = p_params->num_sq_pages_in_ring;
> -- 
> 2.50.1 (Apple Git-155)
> 

Re: [PATCH] qed: iscsi: limit command queue array fill to the ramrod array size

[Pengpeng Hou]

Hi,

No, I do not think the existing feat_num-based check is sufficient here.

That check only verifies that num_queues does not exceed the iSCSI CQ
resources reported by the hardware:

    p_params->num_queues <= p_hwfn->hw_info.feat_num[QED_ISCSI_CQ]

But the ramrod field being filled afterwards is a fixed-size array:

    cq_cmdq_sb_num_arr[SCSI_MAX_NUM_OF_CMDQS]

and SCSI_MAX_NUM_OF_CMDQS is 64 in the current tree.

The in-tree qedi caller derives num_queues from the device-reported CQ
count and the online CPU count:

    dev_info.num_cqs = FEAT_NUM(hwfn, QED_ISCSI_CQ)
    num_queues = min(dev_info.num_cqs, num_online_cpus())

So the existing resource check and the ramrod array capacity are not the
same bound. If the hardware reports more than 64 iSCSI CQs, current-tree
callers can still pass a num_queues value that satisfies the feat_num
check but exceeds ARRAY_SIZE(cq_cmdq_sb_num_arr).

I can resend with this clarified in the commit message.

Best regards,
Pengpeng