diff mbox

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

Message ID 1418303242-8513-1-git-send-email-nikhil.nd@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Nikhil Devshatwar Dec. 11, 2014, 1:07 p.m. UTC
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(-)

Comments

Sakari Ailus Dec. 11, 2014, 2:56 p.m. UTC | #1
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?
Hans Verkuil Dec. 19, 2014, 11:06 a.m. UTC | #2
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
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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);