Hi Zilin,
> In the error paths of reserve_gtt_with_resource() and
> insert_gtt_with_resource(), a vma_res object allocated via
> i915_vma_resource_alloc() was incorrectly released using kfree().
>
> Since i915_vma_resource_alloc() allocates objects from a dedicated
> kmem_cache, using kfree() instead of the corresponding
> i915_vma_resource_free() causes a mismatch between allocation and
> deallocation routines, potentially leading to memory corruption.
I would not call this "improper", because the definition of
kfree allows this usage (found in mm/slub.c):
/**
* kfree - free previously allocated memory
* @object: pointer returned by kmalloc() or kmem_cache_alloc()
*
* If @object is NULL, no operation is performed.
*/
void kfree(const void *object)
{
struct folio *folio;
struct slab *slab;
struct kmem_cache *s;
void *x = (void *)object;
trace_kfree(_RET_IP_, object);
if (unlikely(ZERO_OR_NULL_PTR(object)))
return;
folio = virt_to_folio(object);
if (unlikely(!folio_test_slab(folio))) {
free_large_kmalloc(folio, (void *)object);
return;
}
slab = folio_slab(folio);
s = slab->slab_cache;
slab_free(s, slab, x, _RET_IP_);
}
EXPORT_SYMBOL(kfree);
i915_vma_resource_alloc() calls kmem_cache_zalloc, which is
defined in include/linux/slab.h:
#define kmem_cache_zalloc(_k, _flags) kmem_cache_alloc(_k,
(_flags)|__GFP_ZERO)
I understand what you are trying to do here, but I'd call this
improvement to the readability and matching alloc/free routines
rather than "fixing" it.
>
> Fix this by calling i915_vma_resource_free() to properly release the
> vma_res object in both functions.
>
> Fixes: e1a4bbb6e837d ("drm/i915: Initial introduction of vma resources")
I think you could drop the last sentence and Fixes tag.
> Signed-off-by: Zilin Guan <[email protected]>
> ---
> drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> index 7ab4c4e60264..16e72ef57bed 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> @@ -1524,7 +1524,7 @@ static int reserve_gtt_with_resource(struct i915_vma
> *vma, u64 offset)
> i915_vma_resource_init_from_vma(vma_res, vma);
> vma->resource = vma_res;
> } else {
> - kfree(vma_res);
> + i915_vma_resource_free(vma_res);
> }
> mutex_unlock(&vm->mutex);
>
> @@ -1704,7 +1704,7 @@ static int insert_gtt_with_resource(struct i915_vma
> *vma)
> i915_vma_resource_init_from_vma(vma_res, vma);
> vma->resource = vma_res;
> } else {
> - kfree(vma_res);
> + i915_vma_resource_free(vma_res);
> }
> mutex_unlock(&vm->mutex);
>
> --
> 2.34.1
>
--
Best Regards,
Krzysztof
