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  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:   Fri, 27 Apr 2018 17:56:00 +0100
From:   Robin Murphy <>
To:     Thomas Hellstrom <>,,
Subject: Re: [PATCH] drm/vmwgfx: Fix scatterlist unmapping

Hi Thomas,

On 25/04/18 14:21, Thomas Hellstrom wrote:
> Hi, Robin,
> Thanks for the patch. It was some time since I put together that code, 
> but I remember hitting something similar to
> Even if it's clear from the documentation that orig_nents should be used.

Hmmm, it's odd that you would see issues - it's always been something 
that CONFIG_DMA_API_DEBUG would have screamed about, and as far as I'm 
aware for x86, nents and orig_nents should always end up equal anyway. I 
would definitely be interested to see the specific fault details if it 
can be reproduced. I suppose one possibility is that there's some path 
where you inadvertently unmap something which was never mapped, but 
passing nents=0 means you manage to get away with it without the DMA API 
backend trying to interpret any bogus DMA addresses/lengths.

FWIW, the rationale is that sync_sg/unmap_sg operate on sg->page (which 
can always be translated back to a meaningful CPU address for 
cache/write buffer maintenance), not sg->dma_address (which sometimes 
cannot), therefore passing a truncated list will have the effect of just 
not syncing the tail end of the buffer, which is clearly bad.


> /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 <>
>> ---
>> 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;
>>   }
>>   /**

Powered by blists - more mailing lists