linux-kernel - Re: [RESEND PATCH] media: vb2: Fix vb2_dc_prepare do not correct sync data to device

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 for Android: free password hash cracker in your pocket

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

Message-ID: <6A2D1ECE-40A4-441C-910B-6EEB3D99D8FA@xs4all.nl>
Date:	Wed, 23 Sep 2015 10:40:56 +0200
From:	Hans Verkuil <hverkuil@...all.nl>
To:	Sakari Ailus <sakari.ailus@....fi>,
	Tiffany Lin <tiffany.lin@...iatek.com>
CC:	Pawel Osciak <pawel@...iak.com>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Kyungmin Park <kyungmin.park@...sung.com>,
	Mauro Carvalho Chehab <mchehab@....samsung.com>,
	Matthias Brugger <matthias.bgg@...il.com>,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org,
	linux-mediatek@...ts.infradead.org, robin.murphy@....com
Subject: Re: [RESEND PATCH] media: vb2: Fix vb2_dc_prepare do not correct sync data to device

Resent, hopefully without html this time.

On September 22, 2015 11:10:15 PM GMT+02:00, Sakari Ailus <sakari.ailus@....fi> wrote:
>Hi Tiffany,
>
>(Robin and Hans cc'd.)
>
>On Mon, Sep 21, 2015 at 08:26:11PM +0800, Tiffany Lin wrote:
>> vb2_dc_prepare use the number of SG entries dma_map_sg_attrs return.
>> But in dma_sync_sg_for_device, it use lengths of each SG entries
>> before dma_map_sg_attrs. dma_map_sg_attrs will concatenate
>> SGs until dma length > dma seg bundary. sgt->nents will less than
>> sgt->orig_nents. Using SG entries after dma_map_sg_attrs
>> in vb2_dc_prepare will make some SGs are not sync to device.
>> After add DMA_ATTR_SKIP_CPU_SYNC in vb2_dc_get_userptr to remove
>> sync data to device twice. Device randomly get incorrect data because
>> some SGs are not sync to device. Change to use number of SG entries
>> before dma_map_sg_attrs in vb2_dc_prepare to prevent this issue.
>> 
>> Signed-off-by: Tiffany Lin <tiffany.lin@...iatek.com>
>> ---
>>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
>b/drivers/media/v4l2-core/videobuf2-dma-contig.c
>> index 2397ceb..c5d00bd 100644
>> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
>> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
>> @@ -100,7 +100,7 @@ static void vb2_dc_prepare(void *buf_priv)
>>  	if (!sgt || buf->db_attach)
>>  		return;
>>  
>> -	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents,
>buf->dma_dir);
>> +	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->orig_nents,
>buf->dma_dir);
>>  }
>>  
>>  static void vb2_dc_finish(void *buf_priv)
>> @@ -112,7 +112,7 @@ static void vb2_dc_finish(void *buf_priv)
>>  	if (!sgt || buf->db_attach)
>>  		return;
>>  
>> -	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>> +	dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->orig_nents,
>buf->dma_dir);
>>  }
>>  
>>  /*********************************************/
>
>Acked-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
>
>Could you post a similar patch for videobuf2-dma-sg as well, please?
>This
>should probably go to stable (since when?).
>
>videobuf-dma-sg appears to be broken, too, but the fix is more changes
>than one or two lines.
>
>-- 
>Kind regards,
>
>Sakari Ailus
>e-mail: sakari.ailus@....fi	XMPP: sailus@...iisi.org.uk

Sakari, can you take a careful look at the vb2 code? If I remember correctly, the nents field receives the result of the map_sg function. I have no idea if that's correct.

BTW, don't spend too much time on vb1, nobody cares about that old framework, and vb1 drivers are rarely used on arm platforms. 

Regards,

Hans 

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/