[PATCH iwl-net v2] idpf: fix RSS LUT memcpy size

[PATCH iwl-net v2] idpf: fix RSS LUT memcpy size

[Larysa Zaremba]

Based on the following feedback from Sashiko (received for iXD phase 1
patchset, but valid for the net tree):

 "Is the bounds check xn_params.recv_mem.iov_len < lut_buf_size sufficient?
  Since lut_buf_size only represents the size of the array elements, should
  this check instead verify that the payload is at least
  sizeof(struct virtchnl2_rss_lut) + lut_buf_size?

  [...]

  Does memcpy copy the correct amount of data here? rss_lut_size stores the
  number of 32-bit entries, not the size in bytes. Should it use
  lut_buf_size or rss_data->rss_lut_size * sizeof(u32) instead?"

After inspecting the code, it was concluded that RSS memcpy size is in fact
4 times smaller than it has to be, since a single array entry in a u32, and
rss_data->rss_lut_size is clearly used as an array size. Required Rx buffer
size is also too small, but this is a common issue in the idpf code.

Use a full buffer size (lut_buf_size) instead of the array length
(rss_data->rss_lut_size) when doing memcpy of RSS lookup table.
While at it, increase required Rx buffer size to a whole flex-array
containing structure instead of just the array.

Link: https://sashiko.dev/#/patchset/20260323174052.5355-1-larysa.zaremba%40intel.com?part=8
Fixes: 95af467d9a4e ("idpf: configure resources for RX queues")
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Signed-off-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
v1 -> v2: replace manual array size calculation with flex_array_size()

v1: https://lore.kernel.org/netdev/20260429074232.180528-1-larysa.zaremba@intel.com/

 drivers/net/ethernet/intel/idpf/idpf_virtchnl.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
index be66f9b2e101..0fc7c68447f8 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
@@ -2915,8 +2915,9 @@ int idpf_send_get_set_rss_lut_msg(struct idpf_adapter *adapter,
 	if (reply_sz < sizeof(struct virtchnl2_rss_lut))
 		return -EIO;
 
-	lut_buf_size = le16_to_cpu(recv_rl->lut_entries) * sizeof(u32);
-	if (reply_sz < lut_buf_size)
+	lut_buf_size = flex_array_size(recv_rl, lut,
+				       le16_to_cpu(recv_rl->lut_entries));
+	if (reply_sz < lut_buf_size + sizeof(struct virtchnl2_rss_lut))
 		return -EIO;
 
 	/* size didn't change, we can reuse existing lut buf */
@@ -2933,7 +2934,7 @@ int idpf_send_get_set_rss_lut_msg(struct idpf_adapter *adapter,
 	}
 
 do_memcpy:
-	memcpy(rss_data->rss_lut, recv_rl->lut, rss_data->rss_lut_size);
+	memcpy(rss_data->rss_lut, recv_rl->lut, lut_buf_size);
 
 	return 0;
 }
-- 
2.47.0

Re: [PATCH iwl-net v2] idpf: fix RSS LUT memcpy size

[Simon Horman]

From: 'Simon Horman' <horms@kernel.org>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev
---
idpf: fix RSS LUT memcpy size

This commit increases the memcpy size when retrieving the RSS lookup table
to account for the full buffer size rather than just the number of entries.
It also adjusts the required Rx buffer size check to include the structure
size.

Link: https://sashiko.dev/#/patchset/20260323174052.5355-1-larysa.zaremba%40intel.com?part=8

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> index be66f9b2e101c..0fc7c68447f83 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_virtchnl.c
> @@ -2915,8 +2915,9 @@ int idpf_send_get_set_rss_lut_msg(struct idpf_adapter *adapter,
>  	if (reply_sz < sizeof(struct virtchnl2_rss_lut))
>  		return -EIO;
>  
> -	lut_buf_size = le16_to_cpu(recv_rl->lut_entries) * sizeof(u32);
> -	if (reply_sz < lut_buf_size)
> +	lut_buf_size = flex_array_size(recv_rl, lut,
> +				       le16_to_cpu(recv_rl->lut_entries));
> +	if (reply_sz < lut_buf_size + sizeof(struct virtchnl2_rss_lut))
>  		return -EIO;

Could this cause an out-of-bounds read if the message size exceeds the
allocated buffer?

Looking at the allocation of recv_rl earlier in this function, it has a
fixed size:

    recv_rl = kzalloc(IDPF_CTLQ_MAX_BUF_LEN, GFP_KERNEL);

If the hardware-controlled recv_rl->lut_entries yields a calculated structure
size larger than IDPF_CTLQ_MAX_BUF_LEN, reply_sz may also exceed
IDPF_CTLQ_MAX_BUF_LEN.

While this check ensures reply_sz is large enough for lut_buf_size, does it
ensure we don't read past the end of the IDPF_CTLQ_MAX_BUF_LEN allocation
during the memcpy?

[ ... ]
> @@ -2933,7 +2934,7 @@ int idpf_send_get_set_rss_lut_msg(struct idpf_adapter *adapter,
>  	}
>  
>  do_memcpy:
> -	memcpy(rss_data->rss_lut, recv_rl->lut, rss_data->rss_lut_size);
> +	memcpy(rss_data->rss_lut, recv_rl->lut, lut_buf_size);

Does this copy skip necessary byte-order conversions?

The hardware response recv_rl->lut contains __le32 (little-endian) values,
but the destination rss_data->rss_lut is an array of host-order u32
integers.

The set path correctly applies cpu_to_le32() to elements before sending
them:

    rl->lut[i] = rxhash_ena ? cpu_to_le32(rss_data->rss_lut[i]) : 0;

On big-endian architectures, copying little-endian bytes directly with
memcpy() will result in byte-swapped queue indices. Does the driver need to
iterate over the entries and convert each using le32_to_cpu() instead?