[PATCH 1/3] dma-buf/dma-fence_array: use kvzalloc

[Christian König]

Am 25.10.24 um 08:52 schrieb Friedrich Vock:
> On 24.10.24 22:29, Matthew Brost wrote:
>> On Thu, Oct 24, 2024 at 02:41:57PM +0200, Christian König wrote:
>>> Reports indicates that some userspace applications try to merge more 
>>> than
>>> 80k of fences into a single dma_fence_array leading to a warning from
>>
>> Really, yikes.
>
> Not really IME. Unless Christian means some reports I don't have access
> to, the cases where userspace applications tried to do that were really
> just cases where the fence count exploded exponentially because
> dma_fence_unwrap_merge failed to actually merge identical fences (see
> patch 2). At no point have I actually seen apps trying to merge 80k+
> unique fences.

While working on it I've modified our stress test tool to send the same 
1GiB SDMA copy to 100k different contexts.

Turned out it's perfectly possible to create so many fences, there is 
nothing blocking userspace to do it.

While this isn't a realistic use case the kernel should at least not 
crash or spill a warning, but either handle or reject it gracefully.

Friedrich can you confirm that patch two in this series fixes the 
problem? I would really like to get it into drm-misc-fixes before 6.12 
comes out.

Thanks,
Christian.

>
> Regards,
> Friedrich
>
>>
>>> kzalloc() that the requested size becomes to big.
>>>
>>> While that is clearly an userspace bug we should probably handle 
>>> that case
>>> gracefully in the kernel.
>>>
>>> So we can either reject requests to merge more than a reasonable 
>>> amount of
>>> fences (64k maybe?) or we can start to use kvzalloc() instead of 
>>> kzalloc().
>>> This patch here does the later.
>>>
>>
>> This patch seems reasonable to me if the above use is in fact valid.
>>
>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>> CC: stable at vger.kernel.org
>>
>> Fixes tag?
>>
>> Patch itself LGTM:
>> Reviewed-by: Matthew Brost <matthew.brost at intel.com>
>>
>>> ---
>>>   drivers/dma-buf/dma-fence-array.c | 6 +++---
>>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/dma-buf/dma-fence-array.c 
>>> b/drivers/dma-buf/dma-fence-array.c
>>> index 8a08ffde31e7..46ac42bcfac0 100644
>>> --- a/drivers/dma-buf/dma-fence-array.c
>>> +++ b/drivers/dma-buf/dma-fence-array.c
>>> @@ -119,8 +119,8 @@ static void dma_fence_array_release(struct 
>>> dma_fence *fence)
>>>       for (i = 0; i < array->num_fences; ++i)
>>>           dma_fence_put(array->fences[i]);
>>>
>>> -    kfree(array->fences);
>>> -    dma_fence_free(fence);
>>> +    kvfree(array->fences);
>>> +    kvfree_rcu(fence, rcu);
>>>   }
>>>
>>>   static void dma_fence_array_set_deadline(struct dma_fence *fence,
>>> @@ -153,7 +153,7 @@ struct dma_fence_array 
>>> *dma_fence_array_alloc(int num_fences)
>>>   {
>>>       struct dma_fence_array *array;
>>>
>>> -    return kzalloc(struct_size(array, callbacks, num_fences), 
>>> GFP_KERNEL);
>>> +    return kvzalloc(struct_size(array, callbacks, num_fences), 
>>> GFP_KERNEL);
>>>   }
>>>   EXPORT_SYMBOL(dma_fence_array_alloc);
>>>
>>> -- 
>>> 2.34.1
>>>
>