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

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

[Pengpeng Hou]

qed_sp_fcoe_func_start() validates fcoe_pf_params->num_cqs 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 FCoE
init ramrod. That array is fixed at SCSI_MAX_NUM_OF_CMDQS entries.

The in-tree qedf caller derives num_cqs 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_fcoe.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_fcoe.c b/drivers/net/ethernet/qlogic/qed/qed_fcoe.c
index 2ae3639d6cf7..5fbf78f6adca 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_fcoe.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_fcoe.c
@@ -126,6 +126,15 @@ qed_sp_fcoe_func_start(struct qed_hwfn *p_hwfn,
 		goto err;
 	}
 
+	if (fcoe_pf_params->num_cqs > ARRAY_SIZE(p_data->q_params.cq_cmdq_sb_num_arr)) {
+		DP_ERR(p_hwfn,
+		       "Cannot fit %u queues in %zu command queue slots. Aborting function start\n",
+		       fcoe_pf_params->num_cqs,
+		       ARRAY_SIZE(p_data->q_params.cq_cmdq_sb_num_arr));
+		rc = -EINVAL;
+		goto err;
+	}
+
 	p_data->mtu = cpu_to_le16(fcoe_pf_params->mtu);
 	tmp = cpu_to_le16(fcoe_pf_params->sq_num_pbl_pages);
 	p_data->sq_num_pages_in_pbl = tmp;
-- 
2.50.1 (Apple Git-155)

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

[Simon Horman]

On Tue, Mar 24, 2026 at 06:56:47PM +0800, Pengpeng Hou wrote:
> qed_sp_fcoe_func_start() validates fcoe_pf_params->num_cqs 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 FCoE
> init ramrod. That array is fixed at SCSI_MAX_NUM_OF_CMDQS entries.
> 
> The in-tree qedf caller derives num_cqs 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_fcoe.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_fcoe.c b/drivers/net/ethernet/qlogic/qed/qed_fcoe.c
> index 2ae3639d6cf7..5fbf78f6adca 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_fcoe.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_fcoe.c
> @@ -126,6 +126,15 @@ qed_sp_fcoe_func_start(struct qed_hwfn *p_hwfn,
>  		goto err;
>  	}
>  

Hi,

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

Is that sufficient to avoid the OOB access you describe?

> +	if (fcoe_pf_params->num_cqs > ARRAY_SIZE(p_data->q_params.cq_cmdq_sb_num_arr)) {
> +		DP_ERR(p_hwfn,
> +		       "Cannot fit %u queues in %zu command queue slots. Aborting function start\n",
> +		       fcoe_pf_params->num_cqs,
> +		       ARRAY_SIZE(p_data->q_params.cq_cmdq_sb_num_arr));
> +		rc = -EINVAL;
> +		goto err;
> +	}
> +
>  	p_data->mtu = cpu_to_le16(fcoe_pf_params->mtu);
>  	tmp = cpu_to_le16(fcoe_pf_params->sq_num_pbl_pages);
>  	p_data->sq_num_pages_in_pbl = tmp;
> -- 
> 2.50.1 (Apple Git-155)
> 

Re: [PATCH] qed: fcoe: 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_cqs does not exceed the FCoE CQ
resources reported by the hardware:

    fcoe_pf_params->num_cqs <= p_hwfn->hw_info.feat_num[QED_FCOE_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 qedf caller derives num_cqs from the device-reported CQ count
and the online CPU count:

    dev_info.num_cqs = FEAT_NUM(hwfn, QED_FCOE_CQ)
    num_queues = min(dev_info.num_cqs, num_online_cpus())
    fcoe_pf_params->num_cqs = num_queues

So the existing resource check and the ramrod array capacity are not the
same bound. If the hardware reports more than 64 FCoE CQs, current-tree
callers can still pass a num_cqs 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