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(-)
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 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
dri-devel@lists.freedesktop.org
-
Daniel Vetter -
Noralf Trønnes -
Oleksandr Andrushchenko