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

[Tvrtko Ursulin]

On 28/10/2024 10:34, Christian König wrote:
> Am 25.10.24 um 11:05 schrieb Tvrtko Ursulin:
>>
>> On 25/10/2024 09:59, Tvrtko Ursulin wrote:
>>>
>>> On 24/10/2024 13:41, 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
>>>> 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.
>>>
>>> Rejecting would potentially be safer, otherwise there is a path for 
>>> userspace to trigger a warn in kvmalloc_node (see 0829b5bcdd3b 
>>> ("drm/i915: 2 GiB of relocations ought to be enough for anybody*")) 
>>> and spam dmesg at will.
>>
>> Actually that is a WARN_ON_*ONCE* there so maybe not so critical to 
>> invent a limit. Up for discussion I suppose.
>>
>> Regards,
>>
>> Tvrtko
>>
>>>
>>> Question is what limit to set...
> 
> That's one of the reasons why I opted for kvzalloc() initially.

I didn't get that, what was the reason? To not have to invent an 
arbitrary limit?

> I mean we could use some nice round number like 65536, but that would be 
> totally arbitrary.

Yeah.. Set an arbitrary limit so a warning in __kvmalloc_node_noprof() 
is avoided? Or pass __GFP_NOWARN?

> Any comments on the other two patches? I need to get them upstream.

Will look into them shortly.

Regards,

Tvrtko


> Thanks,
> Christian.
> 
>>>
>>> Regards,
>>>
>>> Tvrtko
>>>
>>>> Signed-off-by: Christian König <christian.koenig at amd.com>
>>>> CC: stable at vger.kernel.org
>>>> ---
>>>>   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);
>