[PATCH] v4l: Fix dma buf single plane compat handling

[PATCH] v4l: Fix dma buf single plane compat handling

[Laurent Pinchart]

From: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>

Buffer length is needed for single plane as well, otherwise
is uninitialized and behaviour is undetermined.

Signed-off-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
index 8fd84a67478a..b0faa1f7e3a9 100644
--- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
+++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
@@ -482,8 +482,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 				return -EFAULT;
 			break;
 		case V4L2_MEMORY_DMABUF:
-			if (get_user(kp->m.fd, &up->m.fd))
+			if (get_user(kp->m.fd, &up->m.fd) ||
+			    get_user(kp->length, &up->length))
 				return -EFAULT;
+
 			break;
 		}
 	}
@@ -550,7 +552,8 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
 				return -EFAULT;
 			break;
 		case V4L2_MEMORY_DMABUF:
-			if (put_user(kp->m.fd, &up->m.fd))
+			if (put_user(kp->m.fd, &up->m.fd) ||
+			    put_user(kp->length, &up->length))
 				return -EFAULT;
 			break;
 		}
-- 
2.4.10

Re: [PATCH] v4l: Fix dma buf single plane compat handling

[Sakari Ailus]

Hi Laurent and Gjorgji,

On Mon, Dec 07, 2015 at 10:45:39AM +0200, Laurent Pinchart wrote:
> From: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
> 
> Buffer length is needed for single plane as well, otherwise
> is uninitialized and behaviour is undetermined.

How about:

The v4l2_buffer length field must be passed as well from user to kernel and
back, otherwise uninitialised values will be used.

> 
> Signed-off-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Shouldn't this be submitted to stable as well?

> ---
>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> index 8fd84a67478a..b0faa1f7e3a9 100644
> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> @@ -482,8 +482,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
>  				return -EFAULT;
>  			break;
>  		case V4L2_MEMORY_DMABUF:
> -			if (get_user(kp->m.fd, &up->m.fd))
> +			if (get_user(kp->m.fd, &up->m.fd) ||
> +			    get_user(kp->length, &up->length))
>  				return -EFAULT;
> +
>  			break;
>  		}
>  	}
> @@ -550,7 +552,8 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user
>  				return -EFAULT;
>  			break;
>  		case V4L2_MEMORY_DMABUF:
> -			if (put_user(kp->m.fd, &up->m.fd))
> +			if (put_user(kp->m.fd, &up->m.fd) ||
> +			    put_user(kp->length, &up->length))
>  				return -EFAULT;
>  			break;
>  		}

-- 
Kind regards,

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

Re: [PATCH] v4l: Fix dma buf single plane compat handling

[grosikopulos]

Hi Sakari,
 
> Hi Laurent and Gjorgji,
> 
> On Mon, Dec 07, 2015 at 10:45:39AM +0200, Laurent Pinchart wrote:
>> From: Gjorgji Rosikopulos grosikopulos@mm-sol.com 
>> 
>> Buffer length is needed for single plane as well, otherwise
>> is uninitialized and behaviour is undetermined.
> 
> How about:
> 
> The v4l2_buffer length field must be passed as well from user to kernel and
> back, otherwise uninitialised values will be used.

Yes that's better :)

> 
>> 
>> Signed-off-by: Gjorgji Rosikopulos grosikopulos@mm-sol.com 
>> Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com 
> 
> Acked-by: Sakari Ailus sakari.ailus@linux.intel.com 
> 
> Shouldn't this be submitted to stable as well?
> 
>> ---
>> drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c 
>> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> index 8fd84a67478a..b0faa1f7e3a9 100644
>> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>> @@ -482,8 +482,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, 
>> struct v4l2_buffer32 __user
>> 				return -EFAULT;
>> 			break;
>> 		case V4L2_MEMORY_DMABUF:
>> -			if (get_user(kp->m.fd, &up->m.fd))
>> +			if (get_user(kp->m.fd, &up->m.fd) ||
>> +			 get_user(kp->length, &up->length))
>> 				return -EFAULT;
>> +
>> 			break;
>> 		}
>> 	}
>> @@ -550,7 +552,8 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, 
>> struct v4l2_buffer32 __user
>> 				return -EFAULT;
>> 			break;
>> 		case V4L2_MEMORY_DMABUF:
>> -			if (put_user(kp->m.fd, &up->m.fd))
>> +			if (put_user(kp->m.fd, &up->m.fd) ||
>> +			 put_user(kp->length, &up->length))
>> 				return -EFAULT;
>> 			break;
>> 		}
> 
> -- 
> Kind regards,
> 
> Sakari Ailus
> e-mail: sakari.ailus@iki.fi XMPP: sailus@retiisi.org.uk 
> 

-- 

Re: [PATCH] v4l: Fix dma buf single plane compat handling

[Laurent Pinchart]

Hi Sakari,

On Tuesday 08 December 2015 17:29:16 Sakari Ailus wrote:
> On Mon, Dec 07, 2015 at 10:45:39AM +0200, Laurent Pinchart wrote:
> > From: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
> > 
> > Buffer length is needed for single plane as well, otherwise
> > is uninitialized and behaviour is undetermined.
> 
> How about:
> 
> The v4l2_buffer length field must be passed as well from user to kernel and
> back, otherwise uninitialised values will be used.
> 
> > Signed-off-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> 
> Shouldn't this be submitted to stable as well?

I'll CC stable.

> > ---
> > 
> >  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index
> > 8fd84a67478a..b0faa1f7e3a9 100644
> > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > @@ -482,8 +482,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp,
> > struct v4l2_buffer32 __user> 
> >  				return -EFAULT;
> >  			
> >  			break;
> >  		
> >  		case V4L2_MEMORY_DMABUF:
> > -			if (get_user(kp->m.fd, &up->m.fd))
> > +			if (get_user(kp->m.fd, &up->m.fd) ||
> > +			    get_user(kp->length, &up->length))
> > 
> >  				return -EFAULT;
> > 
> > +
> > 
> >  			break;
> >  		
> >  		}
> >  	
> >  	}
> > 
> > @@ -550,7 +552,8 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp,
> > struct v4l2_buffer32 __user> 
> >  				return -EFAULT;
> >  			
> >  			break;
> >  		
> >  		case V4L2_MEMORY_DMABUF:
> > -			if (put_user(kp->m.fd, &up->m.fd))
> > +			if (put_user(kp->m.fd, &up->m.fd) ||
> > +			    put_user(kp->length, &up->length))
> > 
> >  				return -EFAULT;
> >  			
> >  			break;
> >  		
> >  		}

-- 
Regards,

Laurent Pinchart

Re: [PATCH] v4l: Fix dma buf single plane compat handling

[Sakari Ailus]

On Wed, Dec 09, 2015 at 01:11:12AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Tuesday 08 December 2015 17:29:16 Sakari Ailus wrote:
> > On Mon, Dec 07, 2015 at 10:45:39AM +0200, Laurent Pinchart wrote:
> > > From: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
> > > 
> > > Buffer length is needed for single plane as well, otherwise
> > > is uninitialized and behaviour is undetermined.
> > 
> > How about:
> > 
> > The v4l2_buffer length field must be passed as well from user to kernel and
> > back, otherwise uninitialised values will be used.
> > 
> > > Signed-off-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > 
> > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > 
> > Shouldn't this be submitted to stable as well?
> 
> I'll CC stable.
> 
> > > ---
> > > 
> > >  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 +++++--
> > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index
> > > 8fd84a67478a..b0faa1f7e3a9 100644
> > > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > > @@ -482,8 +482,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp,
> > > struct v4l2_buffer32 __user> 
> > >  				return -EFAULT;
> > >  			
> > >  			break;
> > >  		
> > >  		case V4L2_MEMORY_DMABUF:
> > > -			if (get_user(kp->m.fd, &up->m.fd))
> > > +			if (get_user(kp->m.fd, &up->m.fd) ||
> > > +			    get_user(kp->length, &up->length))
> > > 
> > >  				return -EFAULT;
> > > 
> > > +

Without the extra newline, please?

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

Re: [PATCH] v4l: Fix dma buf single plane compat handling

[Laurent Pinchart]

Hi Sakari,

On Wednesday 09 December 2015 13:07:40 Sakari Ailus wrote:
> On Wed, Dec 09, 2015 at 01:11:12AM +0200, Laurent Pinchart wrote:
> > On Tuesday 08 December 2015 17:29:16 Sakari Ailus wrote:
> > > On Mon, Dec 07, 2015 at 10:45:39AM +0200, Laurent Pinchart wrote:
> > > > From: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
> > > > 
> > > > Buffer length is needed for single plane as well, otherwise
> > > > is uninitialized and behaviour is undetermined.
> > > 
> > > How about:
> > > 
> > > The v4l2_buffer length field must be passed as well from user to kernel
> > > and back, otherwise uninitialised values will be used.
> > > 
> > > > Signed-off-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > 
> > > Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> > > 
> > > Shouldn't this be submitted to stable as well?
> > 
> > I'll CC stable.
> > 
> > > > ---
> > > > 
> > > >  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 +++++--
> > > >  1 file changed, 5 insertions(+), 2 deletions(-)
> > > > 
> > > > diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > > > b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index
> > > > 8fd84a67478a..b0faa1f7e3a9 100644
> > > > --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > > > +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
> > > > @@ -482,8 +482,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer
> > > > *kp, struct v4l2_buffer32 __user
> > > >  				return -EFAULT;
> > > >  			break;
> > > >  		
> > > >  		case V4L2_MEMORY_DMABUF:
> > > > -			if (get_user(kp->m.fd, &up->m.fd))
> > > > +			if (get_user(kp->m.fd, &up->m.fd) ||
> > > > +			    get_user(kp->length, &up->length))
> > > >  				return -EFAULT;
> > > > +
> 
> Without the extra newline, please?

Sure, I'll fix that in the pull request.

-- 
Regards,

Laurent Pinchart

Re: [PATCH] v4l: Fix dma buf single plane compat handling

[Hans Verkuil]

Hi all,

While going through patchwork I found this patch that does the same as this
one from Tiffany:

https://patchwork.linuxtv.org/patch/32631/

I haven't seen a second version of this patch from Laurent with the requested
changes fixed, so unless Laurent says otherwise I'd like to merge Tiffany's
version.

Laurent, is that OK for you?

Regards,

	Hans

On 12/13/2015 09:40 PM, Laurent Pinchart wrote:
> Hi Sakari,
> 
> On Wednesday 09 December 2015 13:07:40 Sakari Ailus wrote:
>> On Wed, Dec 09, 2015 at 01:11:12AM +0200, Laurent Pinchart wrote:
>>> On Tuesday 08 December 2015 17:29:16 Sakari Ailus wrote:
>>>> On Mon, Dec 07, 2015 at 10:45:39AM +0200, Laurent Pinchart wrote:
>>>>> From: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
>>>>>
>>>>> Buffer length is needed for single plane as well, otherwise
>>>>> is uninitialized and behaviour is undetermined.
>>>>
>>>> How about:
>>>>
>>>> The v4l2_buffer length field must be passed as well from user to kernel
>>>> and back, otherwise uninitialised values will be used.
>>>>
>>>>> Signed-off-by: Gjorgji Rosikopulos <grosikopulos@mm-sol.com>
>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>>
>>>> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
>>>>
>>>> Shouldn't this be submitted to stable as well?
>>>
>>> I'll CC stable.
>>>
>>>>> ---
>>>>>
>>>>>  drivers/media/v4l2-core/v4l2-compat-ioctl32.c | 7 +++++--
>>>>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>>>>> b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c index
>>>>> 8fd84a67478a..b0faa1f7e3a9 100644
>>>>> --- a/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>>>>> +++ b/drivers/media/v4l2-core/v4l2-compat-ioctl32.c
>>>>> @@ -482,8 +482,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer
>>>>> *kp, struct v4l2_buffer32 __user
>>>>>  				return -EFAULT;
>>>>>  			break;
>>>>>  		
>>>>>  		case V4L2_MEMORY_DMABUF:
>>>>> -			if (get_user(kp->m.fd, &up->m.fd))
>>>>> +			if (get_user(kp->m.fd, &up->m.fd) ||
>>>>> +			    get_user(kp->length, &up->length))
>>>>>  				return -EFAULT;
>>>>> +
>>
>> Without the extra newline, please?
> 
> Sure, I'll fix that in the pull request.
> 

Re: [PATCH] v4l: Fix dma buf single plane compat handling

[Laurent Pinchart]

Hi Hans,

On Monday 18 January 2016 12:21:38 Hans Verkuil wrote:
> Hi all,
> 
> While going through patchwork I found this patch that does the same as this
> one from Tiffany:
> 
> https://patchwork.linuxtv.org/patch/32631/
> 
> I haven't seen a second version of this patch from Laurent with the
> requested changes fixed,

As it was too late for v4.5 I was waiting for more comments before sending v2.

> so unless Laurent says otherwise I'd like to merge
> Tiffany's version.

I've now reviewed Tiffany's patch.

> Laurent, is that OK for you?

When patches conflict I usually try to merge the first one that was submitted. 
In this specific case I can make an exception.

-- 
Regards,

Laurent Pinchart