[PATCH] [media] videobuf-dma-contig: NULL check for vb2_plane_cookie

[PATCH] [media] videobuf-dma-contig: NULL check for vb2_plane_cookie

[Nikhil Devshatwar]

vb2_plane_cookie can return NULL if the plane no is greater than
total no of planes or when mem_ops are absent.

Add NULL check to avoid NULL pointer crash in the kernel.

Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
---
 include/media/videobuf2-dma-contig.h |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/media/videobuf2-dma-contig.h b/include/media/videobuf2-dma-contig.h
index 8197f87..5efc56e 100644
--- a/include/media/videobuf2-dma-contig.h
+++ b/include/media/videobuf2-dma-contig.h
@@ -21,7 +21,10 @@ vb2_dma_contig_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no)
 {
 	dma_addr_t *addr = vb2_plane_cookie(vb, plane_no);
 
-	return *addr;
+	if (addr == NULL)
+		return addr;
+	else
+		return *addr;
 }
 
 void *vb2_dma_contig_init_ctx(struct device *dev);
-- 
1.7.9.5

Re: [PATCH] [media] videobuf-dma-contig: NULL check for vb2_plane_cookie

[Sakari Ailus]

Hi Nikhil,

On Thu, Dec 11, 2014 at 06:37:22PM +0530, Nikhil Devshatwar wrote:
> vb2_plane_cookie can return NULL if the plane no is greater than
> total no of planes or when mem_ops are absent.
> 
> Add NULL check to avoid NULL pointer crash in the kernel.
> 
> Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
> ---
>  include/media/videobuf2-dma-contig.h |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/media/videobuf2-dma-contig.h b/include/media/videobuf2-dma-contig.h
> index 8197f87..5efc56e 100644
> --- a/include/media/videobuf2-dma-contig.h
> +++ b/include/media/videobuf2-dma-contig.h
> @@ -21,7 +21,10 @@ vb2_dma_contig_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no)
>  {
>  	dma_addr_t *addr = vb2_plane_cookie(vb, plane_no);
>  
> -	return *addr;
> +	if (addr == NULL)
> +		return addr;
> +	else
> +		return *addr;
>  }
>  
>  void *vb2_dma_contig_init_ctx(struct device *dev);

Should this happen? Wouldn't it be a bug somewhere, quite possibly the driver?

-- 
Kind regards,

Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [PATCH] [media] videobuf-dma-contig: NULL check for vb2_plane_cookie

[Hans Verkuil]

Hi Nikhil,

On 12/11/2014 03:56 PM, Sakari Ailus wrote:
> Hi Nikhil,
> 
> On Thu, Dec 11, 2014 at 06:37:22PM +0530, Nikhil Devshatwar wrote:
>> vb2_plane_cookie can return NULL if the plane no is greater than
>> total no of planes or when mem_ops are absent.
>>
>> Add NULL check to avoid NULL pointer crash in the kernel.
>>
>> Signed-off-by: Nikhil Devshatwar <nikhil.nd@ti.com>
>> ---
>>  include/media/videobuf2-dma-contig.h |    5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/media/videobuf2-dma-contig.h b/include/media/videobuf2-dma-contig.h
>> index 8197f87..5efc56e 100644
>> --- a/include/media/videobuf2-dma-contig.h
>> +++ b/include/media/videobuf2-dma-contig.h
>> @@ -21,7 +21,10 @@ vb2_dma_contig_plane_dma_addr(struct vb2_buffer *vb, unsigned int plane_no)
>>  {
>>  	dma_addr_t *addr = vb2_plane_cookie(vb, plane_no);
>>  
>> -	return *addr;
>> +	if (addr == NULL)
>> +		return addr;
>> +	else
>> +		return *addr;

How about:

	return addr ? *addr : NULL;

Much better.

>>  }
>>  
>>  void *vb2_dma_contig_init_ctx(struct device *dev);
> 
> Should this happen? Wouldn't it be a bug somewhere, quite possibly the driver?
> 

I agree with Sakari: could this ever happen in practice unless it is a driver bug?

If you can provide an example, then that would help.

Regards,

	Hans