| Message ID | 20241024124159.4519-2-christian.koenig@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] dma-buf/dma-fence_array: use kvzalloc | expand |
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. > 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@amd.com> > CC: stable@vger.kernel.org Fixes tag? Patch itself LGTM: Reviewed-by: Matthew Brost <matthew.brost@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 >
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. 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@amd.com> >> CC: stable@vger.kernel.org > > Fixes tag? > > Patch itself LGTM: > Reviewed-by: Matthew Brost <matthew.brost@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 >>
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. Question is what limit to set... Regards, Tvrtko > Signed-off-by: Christian König <christian.koenig@amd.com> > CC: stable@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); >
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... > > Regards, > > Tvrtko > >> Signed-off-by: Christian König <christian.koenig@amd.com> >> CC: stable@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);
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 mean we could use some nice round number like 65536, but that would be totally arbitrary. Any comments on the other two patches? I need to get them upstream. Thanks, Christian. >> >> Regards, >> >> Tvrtko >> >>> Signed-off-by: Christian König <christian.koenig@amd.com> >>> CC: stable@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);
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@amd.com> >>> CC: stable@vger.kernel.org >> >> Fixes tag? >> >> Patch itself LGTM: >> Reviewed-by: Matthew Brost <matthew.brost@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 >>> >
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@amd.com> >>>> CC: stable@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); >
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. > >> 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. > >> 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@amd.com> >>>>> CC: stable@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); >>
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@amd.com> >>>>>> CC: stable@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); >>> >
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);
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. Signed-off-by: Christian König <christian.koenig@amd.com> CC: stable@vger.kernel.org --- drivers/dma-buf/dma-fence-array.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)