| Message ID | 20190426124753.53722-1-noralf@tronnes.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/cma-helper: Fix drm_gem_cma_free_object() | expand |
On Fri, Apr 26, 2019 at 2:48 PM Noralf Trønnes <noralf@tronnes.org> wrote: > > The logic for freeing an imported buffer with a virtual address is > broken. It will free the buffer instead of unmapping the dma buf. > Fix by reversing the if ladder and first check if the buffer is imported. > > Fixes: b9068cde51ee ("drm/cma-helper: Add DRM_GEM_CMA_VMAP_DRIVER_OPS") > Cc: stable@vger.kernel.org > Reported-by: "Li, Tingqian" <tingqian.li@intel.com> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > > This bug is present in 5.0 and it only affects tinydrm drivers that import > buffers, which is rare if anyone at all is doing it. I'll apply this to > drm-misc-next and let it trickle down through stable unless someone thinks > otherwise. I'd stuff it into drm-misc-next-fixes, since drm-misc-next is for 5.3 which is a bit much of a delay for something tagged cc: stable. -Daniel > > Noralf. > > drivers/gpu/drm/drm_gem_cma_helper.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c > index cc26625b4b33..e01ceed09e67 100644 > --- a/drivers/gpu/drm/drm_gem_cma_helper.c > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c > @@ -186,13 +186,13 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj) > > cma_obj = to_drm_gem_cma_obj(gem_obj); > > - if (cma_obj->vaddr) { > - dma_free_wc(gem_obj->dev->dev, cma_obj->base.size, > - cma_obj->vaddr, cma_obj->paddr); > - } else if (gem_obj->import_attach) { > + if (gem_obj->import_attach) { > if (cma_obj->vaddr) > dma_buf_vunmap(gem_obj->import_attach->dmabuf, cma_obj->vaddr); > drm_prime_gem_destroy(gem_obj, cma_obj->sgt); > + } else if (cma_obj->vaddr) { > + dma_free_wc(gem_obj->dev->dev, cma_obj->base.size, > + cma_obj->vaddr, cma_obj->paddr); > } > > drm_gem_object_release(gem_obj); > -- > 2.20.1 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi, Could someone please have a look at this one? Noralf. Den 26.04.2019 14.47, skrev Noralf Trønnes: > The logic for freeing an imported buffer with a virtual address is > broken. It will free the buffer instead of unmapping the dma buf. > Fix by reversing the if ladder and first check if the buffer is imported. > > Fixes: b9068cde51ee ("drm/cma-helper: Add DRM_GEM_CMA_VMAP_DRIVER_OPS") > Cc: stable@vger.kernel.org > Reported-by: "Li, Tingqian" <tingqian.li@intel.com> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > --- > > drivers/gpu/drm/drm_gem_cma_helper.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c > index cc26625b4b33..e01ceed09e67 100644 > --- a/drivers/gpu/drm/drm_gem_cma_helper.c > +++ b/drivers/gpu/drm/drm_gem_cma_helper.c > @@ -186,13 +186,13 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj) > > cma_obj = to_drm_gem_cma_obj(gem_obj); > > - if (cma_obj->vaddr) { > - dma_free_wc(gem_obj->dev->dev, cma_obj->base.size, > - cma_obj->vaddr, cma_obj->paddr); > - } else if (gem_obj->import_attach) { > + if (gem_obj->import_attach) { > if (cma_obj->vaddr) > dma_buf_vunmap(gem_obj->import_attach->dmabuf, cma_obj->vaddr); > drm_prime_gem_destroy(gem_obj, cma_obj->sgt); > + } else if (cma_obj->vaddr) { > + dma_free_wc(gem_obj->dev->dev, cma_obj->base.size, > + cma_obj->vaddr, cma_obj->paddr); > } > > drm_gem_object_release(gem_obj); >
On 5/7/19 7:04 PM, Noralf Trønnes wrote: > Hi, > > Could someone please have a look at this one? > > Noralf. > > Den 26.04.2019 14.47, skrev Noralf Trønnes: >> The logic for freeing an imported buffer with a virtual address is >> broken. It will free the buffer instead of unmapping the dma buf. >> Fix by reversing the if ladder and first check if the buffer is imported. >> >> Fixes: b9068cde51ee ("drm/cma-helper: Add DRM_GEM_CMA_VMAP_DRIVER_OPS") >> Cc: stable@vger.kernel.org >> Reported-by: "Li, Tingqian" <tingqian.li@intel.com> >> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> >> --- >> >> drivers/gpu/drm/drm_gem_cma_helper.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c >> index cc26625b4b33..e01ceed09e67 100644 >> --- a/drivers/gpu/drm/drm_gem_cma_helper.c >> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c >> @@ -186,13 +186,13 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj) >> >> cma_obj = to_drm_gem_cma_obj(gem_obj); >> >> - if (cma_obj->vaddr) { >> - dma_free_wc(gem_obj->dev->dev, cma_obj->base.size, >> - cma_obj->vaddr, cma_obj->paddr); >> - } else if (gem_obj->import_attach) { >> + if (gem_obj->import_attach) { >> if (cma_obj->vaddr) >> dma_buf_vunmap(gem_obj->import_attach->dmabuf, cma_obj->vaddr); >> drm_prime_gem_destroy(gem_obj, cma_obj->sgt); >> + } else if (cma_obj->vaddr) { >> + dma_free_wc(gem_obj->dev->dev, cma_obj->base.size, >> + cma_obj->vaddr, cma_obj->paddr); >> } >> >> drm_gem_object_release(gem_obj); >> > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Den 08.05.2019 08.33, skrev Oleksandr Andrushchenko: > On 5/7/19 7:04 PM, Noralf Trønnes wrote: >> Hi, >> >> Could someone please have a look at this one? >> >> Noralf. >> >> Den 26.04.2019 14.47, skrev Noralf Trønnes: >>> The logic for freeing an imported buffer with a virtual address is >>> broken. It will free the buffer instead of unmapping the dma buf. >>> Fix by reversing the if ladder and first check if the buffer is >>> imported. >>> >>> Fixes: b9068cde51ee ("drm/cma-helper: Add DRM_GEM_CMA_VMAP_DRIVER_OPS") >>> Cc: stable@vger.kernel.org >>> Reported-by: "Li, Tingqian" <tingqian.li@intel.com> >>> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> > Reviewed-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com> Thanks Oleksandr, applied to drm-misc-next-fixes. Noralf. >>> --- >>> >>> drivers/gpu/drm/drm_gem_cma_helper.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c >>> b/drivers/gpu/drm/drm_gem_cma_helper.c >>> index cc26625b4b33..e01ceed09e67 100644 >>> --- a/drivers/gpu/drm/drm_gem_cma_helper.c >>> +++ b/drivers/gpu/drm/drm_gem_cma_helper.c >>> @@ -186,13 +186,13 @@ void drm_gem_cma_free_object(struct >>> drm_gem_object *gem_obj) >>> cma_obj = to_drm_gem_cma_obj(gem_obj); >>> - if (cma_obj->vaddr) { >>> - dma_free_wc(gem_obj->dev->dev, cma_obj->base.size, >>> - cma_obj->vaddr, cma_obj->paddr); >>> - } else if (gem_obj->import_attach) { >>> + if (gem_obj->import_attach) { >>> if (cma_obj->vaddr) >>> dma_buf_vunmap(gem_obj->import_attach->dmabuf, >>> cma_obj->vaddr); >>> drm_prime_gem_destroy(gem_obj, cma_obj->sgt); >>> + } else if (cma_obj->vaddr) { >>> + dma_free_wc(gem_obj->dev->dev, cma_obj->base.size, >>> + cma_obj->vaddr, cma_obj->paddr); >>> } >>> drm_gem_object_release(gem_obj); >>> >> _______________________________________________ >> dri-devel mailing list >> dri-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/dri-devel >
diff --git a/drivers/gpu/drm/drm_gem_cma_helper.c b/drivers/gpu/drm/drm_gem_cma_helper.c index cc26625b4b33..e01ceed09e67 100644 --- a/drivers/gpu/drm/drm_gem_cma_helper.c +++ b/drivers/gpu/drm/drm_gem_cma_helper.c @@ -186,13 +186,13 @@ void drm_gem_cma_free_object(struct drm_gem_object *gem_obj) cma_obj = to_drm_gem_cma_obj(gem_obj); - if (cma_obj->vaddr) { - dma_free_wc(gem_obj->dev->dev, cma_obj->base.size, - cma_obj->vaddr, cma_obj->paddr); - } else if (gem_obj->import_attach) { + if (gem_obj->import_attach) { if (cma_obj->vaddr) dma_buf_vunmap(gem_obj->import_attach->dmabuf, cma_obj->vaddr); drm_prime_gem_destroy(gem_obj, cma_obj->sgt); + } else if (cma_obj->vaddr) { + dma_free_wc(gem_obj->dev->dev, cma_obj->base.size, + cma_obj->vaddr, cma_obj->paddr); } drm_gem_object_release(gem_obj);
The logic for freeing an imported buffer with a virtual address is broken. It will free the buffer instead of unmapping the dma buf. Fix by reversing the if ladder and first check if the buffer is imported. Fixes: b9068cde51ee ("drm/cma-helper: Add DRM_GEM_CMA_VMAP_DRIVER_OPS") Cc: stable@vger.kernel.org Reported-by: "Li, Tingqian" <tingqian.li@intel.com> Signed-off-by: Noralf Trønnes <noralf@tronnes.org> --- This bug is present in 5.0 and it only affects tinydrm drivers that import buffers, which is rare if anyone at all is doing it. I'll apply this to drm-misc-next and let it trickle down through stable unless someone thinks otherwise. Noralf. drivers/gpu/drm/drm_gem_cma_helper.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)