On 07/11/2024 12:48, Christian König wrote:
Am 07.11.24 um 12:29 schrieb 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?
Well that I couldn't come up with any arbitrary limit that I had
confidence would work and not block real world use cases.
Switching to kvzalloc() just seemed the more defensive approach.
Yeah it is.
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?
Well are we sure that will never hit 65536 in a real world use case?
It's still pretty low.
Ah no, I did not express myself clearly. I did not mean 64k, but a limit
to align with INT_MAX __kvmalloc_node_noprof(). Or __GFP_NOWARN might be
better when allocation size is userspace controlled.
Regards,
Tvrtko
Any comments on the other two patches? I need to get them upstream.
Will look into them shortly.
Thanks,
Christian.
Regards,
Tvrtko
Thanks,
Christian.
Regards,
Tvrtko
Signed-off-by: Christian König <christian.koenig@xxxxxxx>
CC: stable@xxxxxxxxxxxxxxx
---
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);
