Re: [PATCH v7 1/5] net/mlx5: free mlx5_st_idx_data on final dealloc

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Out-of-bounds read in UVERBS_METHOD_DMAH_ALLOC handler due to missing bounds check on user-supplied cpu_id.
--

commit efb72743b37fe4c5005fca2d75b36ef50832bc6c
Author: Zhiping Zhang <zhipingz@meta.com>

net/mlx5: free mlx5_st_idx_data on final dealloc
    
When the last reference to an ST table entry is dropped,
mlx5_st_dealloc_index() removed the entry from idx_xa but leaked the
backing mlx5_st_idx_data allocation. Repeated alloc/dealloc cycles
therefore accumulate one struct mlx5_st_idx_data per cycle.
    
Free idx_data after the xa_erase() so the lifetime of the bookkeeping
struct matches the lifetime of the ST entry it tracks.
    
Fixes: 888a7776f4fb ("net/mlx5: Add support for device steering tag")
Signed-off-by: Zhiping Zhang <zhipingz@meta.com>

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c
> index 997be91f0a135..7cedc348790db 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/st.c
> @@ -175,6 +175,7 @@ int mlx5_st_dealloc_index(struct mlx5_core_dev *dev, u16 st_index)
>  
>  	if (refcount_dec_and_test(&idx_data->usecount)) {
>  		xa_erase(&st->idx_xa, st_index);
> +		kfree(idx_data);
>  		/* We leave PCI config space as was before, no mkey will refer to it */
>  	}
>  

[Severity: Critical]
This is a pre-existing issue, but does
UVERBS_HANDLER(UVERBS_METHOD_DMAH_ALLOC) in
drivers/infiniband/core/uverbs_std_types_dmah.c lack bounds checking on the
untrusted user-supplied dmah->cpu_id?

drivers/infiniband/core/uverbs_std_types_dmah.c:UVERBS_HANDLER(UVERBS_METHOD_DMAH_ALLOC)() {
    ...
		ret = uverbs_copy_from(&dmah->cpu_id, attrs,
				       UVERBS_ATTR_ALLOC_DMAH_CPU_ID);
		if (ret)
			goto err;

		if (!cpumask_test_cpu(dmah->cpu_id, current->cpus_ptr)) {
			ret = -EPERM;
			goto err;
		}
    ...
}

The dmah->cpu_id is copied directly from an untrusted uverbs attribute
without value validation.

Since cpumask_check only emits a warning if the CPU ID is out of bounds and
does not cap the value, could test_bit compute a bit offset far outside the
bounds of current->cpus_ptr?

Can an attacker exploit this to read unmapped kernel memory (causing a kernel
panic) or infer the contents of kernel memory by observing whether the
syscall succeeds or fails with -EPERM?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260610193158.2614209-1-zhipingz@meta.com?part=1