[media] videobuf-dma-contig: fix vm_iomap_memory() call

[media] videobuf-dma-contig: fix vm_iomap_memory() call

[Ma Haijun]

Hi all,

This is a trivial fix, but I think the patch itself has problem too. 
The function requires a phys_addr_t, but we feed it with a dma_handle_t.
AFAIK, this implicit conversion does not always work.
Can I use virt_to_phys(mem->vaddr) to get the physical address instead?
(mem->vaddr and mem->dma_handle are from dma_alloc_coherent)

Regards

Ma Haijun

Ma Haijun (1):
  [media] videobuf-dma-contig: fix incorrect argument to
    vm_iomap_memory() call

 drivers/media/v4l2-core/videobuf-dma-contig.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
1.8.3.2

Re: [media] videobuf-dma-contig: fix vm_iomap_memory() call

[Hans Verkuil]

On 03/27/2014 12:07 PM, Ma Haijun wrote:
> Hi all,
> 
> This is a trivial fix, but I think the patch itself has problem too. 
> The function requires a phys_addr_t, but we feed it with a dma_handle_t.
> AFAIK, this implicit conversion does not always work.
> Can I use virt_to_phys(mem->vaddr) to get the physical address instead?
> (mem->vaddr and mem->dma_handle are from dma_alloc_coherent)

Does this actually fail? With what driver and on what hardware?

I ask because I am very reluctant to make any changes to videobuf. It is
slowly being replaced by the vastly superior videobuf2 framework. Existing
drivers in the kernel still using the old videobuf seem to work just fine
(or at least as fine as videobuf allows you to be).

Regards,

	Hans

> 
> Regards
> 
> Ma Haijun
> 
> Ma Haijun (1):
>   [media] videobuf-dma-contig: fix incorrect argument to
>     vm_iomap_memory() call
> 
>  drivers/media/v4l2-core/videobuf-dma-contig.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

RE: [media] videobuf-dma-contig: fix vm_iomap_memory() call

[Ma Haijun]

Hi,

> -----Original Message-----
> 
> On 03/27/2014 12:07 PM, Ma Haijun wrote:
> > Hi all,
> >
> > This is a trivial fix, but I think the patch itself has problem too.
> > The function requires a phys_addr_t, but we feed it with a dma_handle_t.
> > AFAIK, this implicit conversion does not always work.
> > Can I use virt_to_phys(mem->vaddr) to get the physical address instead?
> > (mem->vaddr and mem->dma_handle are from dma_alloc_coherent)
> 
> Does this actually fail? With what driver and on what hardware?

I notice it when I am reading the code,
so I do not know if any driver is broken.

> 
> I ask because I am very reluctant to make any changes to videobuf. It is
> slowly being replaced by the vastly superior videobuf2 framework. Existing
> drivers in the kernel still using the old videobuf seem to work just fine
> (or at least as fine as videobuf allows you to be).

Sorry that the cover letter is a bit misleading, hope you does not skip the
patch due to this.

The actual problem is that userland virtual address is erroneously passed to
vm_iomap_memory, while it expects physical address.
And that is what the patch try to address.

Seems this bug can be exploited to map and access physical address.
So it is also a security problem, I think we should ether fix it
or remove the code if not used any more.

Regards,

	Haijun


> 
> Regards,
> 
> 	Hans
> 
> >
> > Regards
> >
> > Ma Haijun
> >
> > Ma Haijun (1):
> >   [media] videobuf-dma-contig: fix incorrect argument to
> >     vm_iomap_memory() call
> >
> >  drivers/media/v4l2-core/videobuf-dma-contig.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >