linux-kernel - Re: [PATCH] drm/vmwgfx: Fix scatterlist unmapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 25 Apr 2018 15:21:19 +0200
From:   Thomas Hellstrom <thellstrom@...are.com>
To:     Robin Murphy <robin.murphy@....com>,
        linux-graphics-maintainer@...are.com, syeh@...are.com
Cc:     dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm/vmwgfx: Fix scatterlist unmapping

Hi, Robin,

Thanks for the patch. It was some time since I put together that code, 
but I remember hitting something similar to

https://www.linuxquestions.org/questions/linux-kernel-70/%27nents%27-argument-of-dma_unmap_sg-4175621964/

Even if it's clear from the documentation that orig_nents should be used.

/Thomas

On 04/13/2018 05:14 PM, Robin Murphy wrote:
> dma_unmap_sg() should be called with the same number of entries
> originally passed to dma_map_sg(), not the number it returned, which may
> be fewer. Admittedly this driver probably never runs on non-coherent
> architectures where getting that wrong could lead to data loss, but it's
> always good to be correct, and it's trivially easy to fix by just
> restoring the SG table state before the call instead of afterwards.
>
> Signed-off-by: Robin Murphy <robin.murphy@....com>
> ---
>
> Found by inspection while poking around TTM users.
>
>   drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
> index 21111fd091f9..971223d39469 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_buffer.c
> @@ -369,9 +369,9 @@ static void vmw_ttm_unmap_from_dma(struct vmw_ttm_tt *vmw_tt)
>   {
>   	struct device *dev = vmw_tt->dev_priv->dev->dev;
>   
> +	vmw_tt->sgt.nents = vmw_tt->sgt.orig_nents;
>   	dma_unmap_sg(dev, vmw_tt->sgt.sgl, vmw_tt->sgt.nents,
>   		DMA_BIDIRECTIONAL);
> -	vmw_tt->sgt.nents = vmw_tt->sgt.orig_nents;
>   }
>   
>   /**