Re: [PATCH iwl-next v4 11/15] ice: introduce handling of virtchnl LARGE VF opcodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] ice: u8 overflow of num_ring_rx and num_ring_tx in ice_vc_map_q_vector_msg()
--

commit cafaf07076ec22c624fc3bc56c70b0e708de5ea9
Author: Brett Creeley <brett.creeley@intel.com>

ice: introduce handling of virtchnl LARGE VF opcodes

This commit adds virtchnl message handlers for VIRTCHNL_OP_ENABLE_QUEUES_V2,
VIRTCHNL_OP_DISABLE_QUEUES_V2, and VIRTCHNL_OP_MAP_QUEUE_VECTOR to support VFs
with more than 16 queue pairs, and allowlists the VIRTCHNL_VF_LARGE_NUM_QPAIRS
opcodes.

> diff --git a/drivers/net/ethernet/intel/ice/virt/queues.c b/drivers/net/ethernet/intel/ice/virt/queues.c
> index 7f2b459dbc836..f87ccf5d370bb 100644
> --- a/drivers/net/ethernet/intel/ice/virt/queues.c
> +++ b/drivers/net/ethernet/intel/ice/virt/queues.c
> @@ -1046,3 +1046,342 @@ int ice_vc_request_qs_msg(struct ice_vf *vf, u8 *msg)

[ ... ]

> +int ice_vc_map_q_vector_msg(struct ice_vf *vf, u8 *msg, u16 msglen)
> +{
> +	enum virtchnl_status_code v_ret = VIRTCHNL_STATUS_ERR_PARAM;
> +	struct virtchnl_queue_vector_maps *qv_maps;
> +	struct ice_vsi *vsi;
> +
> +	qv_maps = (struct virtchnl_queue_vector_maps *)msg;
> +
> +	if (!ice_vc_validate_qv_maps(vf, qv_maps, msglen))
> +		goto error_param;
> +
> +	vsi = ice_get_vf_vsi(vf);
> +	if (!vsi)
> +		goto error_param;
> +
> +	for (int i = 0; i < qv_maps->num_qv_maps; i++) {
> +		struct virtchnl_queue_vector *qv_map = &qv_maps->qv_maps[i];
> +		struct ice_q_vector *q_vector;
> +		u16 vector_id;
> +		int vsi_q_id;
> +
> +		vsi_q_id = qv_map->queue_id;
> +		vector_id = qv_map->vector_id;
> +
> +		q_vector = vf->vf_ops->get_q_vector(vsi, vector_id);
> +		if (!q_vector)
> +			goto error_param;
> +
> +		if (!ice_vc_isvalid_q_id(vsi, vsi_q_id))
> +			goto error_param;
> +
> +		/* keep SW state in sync with HW, as ice_cfg_interrupt() does */
> +		if (qv_map->queue_type == VIRTCHNL_QUEUE_TYPE_RX) {
> +			struct ice_rx_ring *ring = vsi->rx_rings[vsi_q_id];
> +
> +			if (ring->q_vector && ring->q_vector->num_ring_rx)
> +				ring->q_vector->num_ring_rx--;
> +			ring->q_vector = q_vector;
> +			q_vector->num_ring_rx++;
> +			q_vector->rx.itr_idx = qv_map->itr_idx;
> +			ice_cfg_rxq_interrupt(vsi, vsi_q_id,
> +					      q_vector->vf_reg_idx,
> +					      qv_map->itr_idx);
> +		} else if (qv_map->queue_type == VIRTCHNL_QUEUE_TYPE_TX) {
> +			struct ice_tx_ring *ring = vsi->tx_rings[vsi_q_id];
> +
> +			if (ring->q_vector && ring->q_vector->num_ring_tx)
> +				ring->q_vector->num_ring_tx--;
> +			ring->q_vector = q_vector;
> +			q_vector->num_ring_tx++;

[Severity: Medium]
Can q_vector->num_ring_rx and q_vector->num_ring_tx overflow in
ice_vc_map_q_vector_msg() in drivers/net/ethernet/intel/ice/virt/queues.c?

In drivers/net/ethernet/intel/ice/ice.h, num_ring_rx and num_ring_tx are
defined as u8 (max 255):

struct ice_q_vector {
	...
	u8 num_ring_rx;			/* total number of Rx rings in vector */
	u8 num_ring_tx;			/* total number of Tx rings in vector */

With VIRTCHNL_VF_LARGE_NUM_QPAIRS enabled, a VF can be configured with up to
ICE_MAX_QS_PER_VF (256) queue pairs. If the VF sends a
VIRTCHNL_OP_MAP_QUEUE_VECTOR message mapping all 256 Rx or Tx queues to the
same interrupt vector, incrementing q_vector->num_ring_rx or
q_vector->num_ring_tx 256 times wraps the u8 counter from 255 to 0.

If any of those queues are later remapped to another vector, the check:

	if (ring->q_vector && ring->q_vector->num_ring_rx)
		ring->q_vector->num_ring_rx--;

will see 0 and skip the decrement, leaving the per-vector ring accounting out
of sync. Should num_ring_rx and num_ring_tx in struct ice_q_vector be widened
to u16?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103141646.303620-1-przemyslaw.kitszel@intel.com?part=11