Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1629084 > unrolled thread

[PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'

Started byChristophe JAILLET <christophe.jaillet@wanadoo.fr>
First post2017-04-23 23:50 +0200
Last post2017-04-25 15:00 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr' Christophe JAILLET <christophe.jaillet@wanadoo.fr> - 2017-04-23 23:50 +0200
    Re: [PATCH 1/2] [media] vb2: Fix an off by one error in  'vb2_plane_vaddr' Sakari Ailus <sakari.ailus@iki.fi> - 2017-04-24 16:20 +0200
      Re: [PATCH 1/2] [media] vb2: Fix an off by one error in  'vb2_plane_vaddr' Christophe JAILLET <christophe.jaillet@wanadoo.fr> - 2017-04-24 22:10 +0200
        Re: [PATCH 1/2] [media] vb2: Fix an off by one error in  'vb2_plane_vaddr' Sakari Ailus <sakari.ailus@iki.fi> - 2017-04-24 22:30 +0200
          Re: [PATCH 1/2] [media] vb2: Fix an off by one error in  'vb2_plane_vaddr' Christophe JAILLET <christophe.jaillet@wanadoo.fr> - 2017-04-25 08:20 +0200
            Re: [PATCH 1/2] [media] vb2: Fix an off by one error in  'vb2_plane_vaddr' Dan Carpenter <dan.carpenter@oracle.com> - 2017-04-25 15:00 +0200

#1629084 — [PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'

FromChristophe JAILLET <christophe.jaillet@wanadoo.fr>
Date2017-04-23 23:50 +0200
Subject[PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'
Message-ID<tzxyh-5K2-1@gated-at.bofh.it>
We should ensure that 'plane_no' is '< vb->num_planes' as done in
'vb2_plane_cookie' just a few lines below.

Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
---
 drivers/media/v4l2-core/videobuf2-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 94afbbf92807..c0175ea7e7ad 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -868,7 +868,7 @@ EXPORT_SYMBOL_GPL(vb2_core_create_bufs);
 
 void *vb2_plane_vaddr(struct vb2_buffer *vb, unsigned int plane_no)
 {
-	if (plane_no > vb->num_planes || !vb->planes[plane_no].mem_priv)
+	if (plane_no >= vb->num_planes || !vb->planes[plane_no].mem_priv)
 		return NULL;
 
 	return call_ptr_memop(vb, vaddr, vb->planes[plane_no].mem_priv);
-- 
2.11.0

[toc] | [next] | [standalone]

#1629612 — Re: [PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'

FromSakari Ailus <sakari.ailus@iki.fi>
Date2017-04-24 16:20 +0200
SubjectRe: [PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'
Message-ID<tzN0m-83T-23@gated-at.bofh.it>
In reply to#1629084
On Sun, Apr 23, 2017 at 11:32:57PM +0200, Christophe JAILLET wrote:
> We should ensure that 'plane_no' is '< vb->num_planes' as done in
> 'vb2_plane_cookie' just a few lines below.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 94afbbf92807..c0175ea7e7ad 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -868,7 +868,7 @@ EXPORT_SYMBOL_GPL(vb2_core_create_bufs);
>  
>  void *vb2_plane_vaddr(struct vb2_buffer *vb, unsigned int plane_no)
>  {
> -	if (plane_no > vb->num_planes || !vb->planes[plane_no].mem_priv)
> +	if (plane_no >= vb->num_planes || !vb->planes[plane_no].mem_priv)
>  		return NULL;
>  
>  	return call_ptr_memop(vb, vaddr, vb->planes[plane_no].mem_priv);

Oh my. How could this happen?

This should go to stable as well.

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

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

[toc] | [prev] | [next] | [standalone]

#1629947 — Re: [PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'

FromChristophe JAILLET <christophe.jaillet@wanadoo.fr>
Date2017-04-24 22:10 +0200
SubjectRe: [PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'
Message-ID<tzSt3-33H-7@gated-at.bofh.it>
In reply to#1629612
Le 24/04/2017 à 16:16, Sakari Ailus a écrit :
> On Sun, Apr 23, 2017 at 11:32:57PM +0200, Christophe JAILLET wrote:
>> We should ensure that 'plane_no' is '< vb->num_planes' as done in
>> 'vb2_plane_cookie' just a few lines below.
>>
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>>   drivers/media/v4l2-core/videobuf2-core.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
>> index 94afbbf92807..c0175ea7e7ad 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -868,7 +868,7 @@ EXPORT_SYMBOL_GPL(vb2_core_create_bufs);
>>   
>>   void *vb2_plane_vaddr(struct vb2_buffer *vb, unsigned int plane_no)
>>   {
>> -	if (plane_no > vb->num_planes || !vb->planes[plane_no].mem_priv)
>> +	if (plane_no >= vb->num_planes || !vb->planes[plane_no].mem_priv)
>>   		return NULL;
>>   
>>   	return call_ptr_memop(vb, vaddr, vb->planes[plane_no].mem_priv);
> Oh my. How could this happen?
>
> This should go to stable as well.
Should I resubmit with "Cc: stable@vger.kernel.org" or will you add it 
yourself?

CJ

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

[toc] | [prev] | [next] | [standalone]

#1629988 — Re: [PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'

FromSakari Ailus <sakari.ailus@iki.fi>
Date2017-04-24 22:30 +0200
SubjectRe: [PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'
Message-ID<tzSMq-3ex-33@gated-at.bofh.it>
In reply to#1629947
Hi Christophe,

On Mon, Apr 24, 2017 at 10:00:24PM +0200, Christophe JAILLET wrote:
> Le 24/04/2017 à 16:16, Sakari Ailus a écrit :
> >On Sun, Apr 23, 2017 at 11:32:57PM +0200, Christophe JAILLET wrote:
> >>We should ensure that 'plane_no' is '< vb->num_planes' as done in
> >>'vb2_plane_cookie' just a few lines below.
> >>
> >>Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> >>---
> >>  drivers/media/v4l2-core/videobuf2-core.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >>diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> >>index 94afbbf92807..c0175ea7e7ad 100644
> >>--- a/drivers/media/v4l2-core/videobuf2-core.c
> >>+++ b/drivers/media/v4l2-core/videobuf2-core.c
> >>@@ -868,7 +868,7 @@ EXPORT_SYMBOL_GPL(vb2_core_create_bufs);
> >>  void *vb2_plane_vaddr(struct vb2_buffer *vb, unsigned int plane_no)
> >>  {
> >>-	if (plane_no > vb->num_planes || !vb->planes[plane_no].mem_priv)
> >>+	if (plane_no >= vb->num_planes || !vb->planes[plane_no].mem_priv)
> >>  		return NULL;
> >>  	return call_ptr_memop(vb, vaddr, vb->planes[plane_no].mem_priv);
> >Oh my. How could this happen?
> >
> >This should go to stable as well.
> Should I resubmit with "Cc: stable@vger.kernel.org" or will you add it
> yourself?

Please resend. And preferrably figure out which version is the first one
requiring the fix.

Mauro can then pick it up, and it ends up to stable through his tree. I.e.
Cc: stable ... tag is enough, no need to send an actual  e-mail there.

Thanks!

-- 
Kind regards,

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

[toc] | [prev] | [next] | [standalone]

#1630198 — Re: [PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'

FromChristophe JAILLET <christophe.jaillet@wanadoo.fr>
Date2017-04-25 08:20 +0200
SubjectRe: [PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'
Message-ID<tA1Zo-V0-1@gated-at.bofh.it>
In reply to#1629988
Le 24/04/2017 à 22:29, Sakari Ailus a écrit :
> Hi Christophe,
>
> On Mon, Apr 24, 2017 at 10:00:24PM +0200, Christophe JAILLET wrote:
>> Le 24/04/2017 à 16:16, Sakari Ailus a écrit :
>>> On Sun, Apr 23, 2017 at 11:32:57PM +0200, Christophe JAILLET wrote:
>>>> We should ensure that 'plane_no' is '< vb->num_planes' as done in
>>>> 'vb2_plane_cookie' just a few lines below.
>>>>
>>>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>>>> ---
>>>>   drivers/media/v4l2-core/videobuf2-core.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
>>>> index 94afbbf92807..c0175ea7e7ad 100644
>>>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>>>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>>>> @@ -868,7 +868,7 @@ EXPORT_SYMBOL_GPL(vb2_core_create_bufs);
>>>>   void *vb2_plane_vaddr(struct vb2_buffer *vb, unsigned int plane_no)
>>>>   {
>>>> -	if (plane_no > vb->num_planes || !vb->planes[plane_no].mem_priv)
>>>> +	if (plane_no >= vb->num_planes || !vb->planes[plane_no].mem_priv)
>>>>   		return NULL;
>>>>   	return call_ptr_memop(vb, vaddr, vb->planes[plane_no].mem_priv);
>>> Oh my. How could this happen?
>>>
>>> This should go to stable as well.
>> Should I resubmit with "Cc: stable@vger.kernel.org" or will you add it
>> yourself?
> Please resend. And preferrably figure out which version is the first one
> requiring the fix.
>
> Mauro can then pick it up, and it ends up to stable through his tree. I.e.
> Cc: stable ... tag is enough, no need to send an actual  e-mail there.
>
> Thanks!
>
Hmm, funny to see:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/media/v4l2-core/videobuf2-core.c?id=a9ae4692eda4b99f85757b15d60971ff78a0a0e2


Anyway,

3.2.88:
    still have the issue for both 'vb2_plane_vaddr' and 
'vb2_plane_cookie', but the file is in a slightly different 
directory*and the code is also slightly different*

3.4.113:
    still have the issue for both 'vb2_plane_vaddr' and 
'vb2_plane_cookie', but the file is in a slightly different directory

3.10.105, *3.12.73*:
    still have the issue for both 'vb2_plane_vaddr' and 'vb2_plane_cookie'

3.16.43 and up:
    'vb2_plane_cookie' is fixed there.

So, I guess, that the same +3.16 should be proposed here, to be 
consistent. Ok for you?


Should a:
    Fixes: e23ccc0ad9258 ("[media] v4l: add videobuf2 Video for Linux 2 
driver framework")
be also added? I've read somewhere that Fixes tags were needed for 
backport to stable.

CJ

[toc] | [prev] | [next] | [standalone]

#1630483 — Re: [PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'

FromDan Carpenter <dan.carpenter@oracle.com>
Date2017-04-25 15:00 +0200
SubjectRe: [PATCH 1/2] [media] vb2: Fix an off by one error in 'vb2_plane_vaddr'
Message-ID<tA8eu-4Pu-25@gated-at.bofh.it>
In reply to#1630198
Gar... No.  The 3.6+ from a9ae4692eda4 ("[media] vb2: fix plane index
sanity check in vb2_plane_cookie()") feels totally arbitrary to me.  No
need to be consistent.

Just do:

Cc: stable@vger.kernel.org
Fixes: e23ccc0ad925 ("[media] v4l: add videobuf2 Video for Linux 2 driver framework")

Fixes tags are always good too have btw.  You should be adding them
by default to everything even if it doesn't get backported to stable.

regards,
dan carpenter

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web