On Fri, Jul 14, 2023 at 08:56:15AM +0200, Christian König wrote:
> Am 13.07.23 um 21:47 schrieb Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >
> > Currently dma_resv_get_fences() will leak the previously
> > allocated array if the fence iteration got restarted and
> > the krealloc_array() fails.
> >
> > Free the old array by hand, and make sure we still clear
> > the returned *fences so the caller won't end up accessing
> > freed memory. Some (but not all) of the callers of
> > dma_resv_get_fences() seem to still trawl through the
> > array even when dma_resv_get_fences() failed. And let's
> > zero out *num_fences as well for good measure.
> >
> > Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
> > Cc: Christian König <christian.koenig@xxxxxxx>
> > Cc: linux-media@xxxxxxxxxxxxxxx
> > Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> > Cc: linaro-mm-sig@xxxxxxxxxxxxxxxx
> > Fixes: d3c80698c9f5 ("dma-buf: use new iterator in dma_resv_get_fences v3")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Good catch, Reviewed-by: Christian König <christian.koenig@xxxxxxx>
>
> Should I add a CC: stable and push to drm-misc-fixes?
Sure, if you don't mind. Thanks.
>
> Thanks,
> Christian.
>
> > ---
> > drivers/dma-buf/dma-resv.c | 13 +++++++++----
> > 1 file changed, 9 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/dma-buf/dma-resv.c b/drivers/dma-buf/dma-resv.c
> > index b6f71eb00866..38b4110378de 100644
> > --- a/drivers/dma-buf/dma-resv.c
> > +++ b/drivers/dma-buf/dma-resv.c
> > @@ -571,6 +571,7 @@ int dma_resv_get_fences(struct dma_resv *obj, enum dma_resv_usage usage,
> > dma_resv_for_each_fence_unlocked(&cursor, fence) {
> >
> > if (dma_resv_iter_is_restarted(&cursor)) {
> > + struct dma_fence **new_fences;
> > unsigned int count;
> >
> > while (*num_fences)
> > @@ -579,13 +580,17 @@ int dma_resv_get_fences(struct dma_resv *obj, enum dma_resv_usage usage,
> > count = cursor.num_fences + 1;
> >
> > /* Eventually re-allocate the array */
> > - *fences = krealloc_array(*fences, count,
> > - sizeof(void *),
> > - GFP_KERNEL);
> > - if (count && !*fences) {
> > + new_fences = krealloc_array(*fences, count,
> > + sizeof(void *),
> > + GFP_KERNEL);
> > + if (count && !new_fences) {
> > + kfree(*fences);
> > + *fences = NULL;
> > + *num_fences = 0;
> > dma_resv_iter_end(&cursor);
> > return -ENOMEM;
> > }
> > + *fences = new_fences;
> > }
> >
> > (*fences)[(*num_fences)++] = dma_fence_get(fence);
--
Ville Syrjälä
Intel
