Re: [PATCH net 1/2] net: Catch invalid index in XPS mapping

[Piotr Raczynski <piotr.raczynski@intel.com>]

On Fri, Mar 17, 2023 at 01:19:40PM -0500, Nick Child wrote:
> When setting the XPS value of a TX queue, add a conditional to ensure
> that the index of the queue is less than the number of allocated TX
> queues.
> 
> Previously, this scenario went uncaught. In the best case, it resulted
> in unnecessary allocations. In the worst case, it resulted in
> out-of-bounds memory references through calls to `netdev_get_tx_queue(
> dev, index)`.
> 
> Fixes: 537c00de1c9b ("net: Add functions netif_reset_xps_queue and netif_set_xps_queue")
> Signed-off-by: Nick Child <nnac123@linux.ibm.com>
> ---
> This is a result of my own foolish mistake of giving an invalid
> index to __netif_set_xps_queue [1]. While the function adds the queue to
> the cpu's XPS queue map, the queue is never used due to a conditional
> in __get_xps_queue_idx. But there is a risk of random memory reading
> and writing that should be prevented.
> 
> 1. https://lore.kernel.org/netdev/20230224183659.2a7bfeea@kernel.org/
> 
>  net/core/dev.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index c7853192563d..cd3878043846 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2535,6 +2535,9 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
>  	struct xps_map *map, *new_map;
>  	unsigned int nr_ids;
>  
> +	if (index >= dev->num_tx_queues)
> +		return -EINVAL;
> +
>  	if (dev->num_tc) {
>  		/* Do not allow XPS on subordinate device directly */
>  		num_tc = dev->num_tc;
> -- 
> 2.31.1
> 
Reasonable check added, thanks.
Reviewed-by: Piotr Raczynski <piotr.raczynski@intel.com>