[1/2] libcamera: formats: Fix planes bounds check
diff mbox series

Message ID 20240226094857.2207313-2-kieran.bingham@ideasonboard.com
State Accepted
Commit 25a8d8b8a914302f1de02af4b0f5ae70568f00ea
Headers show
Series
  • Two small fixes
Related show

Commit Message

Kieran Bingham Feb. 26, 2024, 9:48 a.m. UTC
The plane validation in the stride helper incorrectly accepts the number
of planes as a plane index. Fix the off by one issue.

Reported-by: Johan Mattsson <39247600+mjunix@users.noreply.github.com>
Fixes: e83727a194b5 ("libcamera: PixelFormatInfo: Add functions stride and frameSize")
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/formats.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Kieran Bingham Feb. 26, 2024, 9:56 a.m. UTC | #1
Quoting Kieran Bingham (2024-02-26 09:48:56)
> The plane validation in the stride helper incorrectly accepts the number
> of planes as a plane index. Fix the off by one issue.
> 
> Reported-by: Johan Mattsson <39247600+mjunix@users.noreply.github.com>

As that's a @users.noreply this bounces and I don't think I can really
keep this as is, even though that's what Github seems to provide.

I could do:

Reported-by: Johan Mattsson <mjunix@github.com>

Perhaps, or simply lose the reporter.
Opinions anyone ?

--
Kieran


> Fixes: e83727a194b5 ("libcamera: PixelFormatInfo: Add functions stride and frameSize")
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/formats.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> index 447e623803c7..c11fbd730c8e 100644
> --- a/src/libcamera/formats.cpp
> +++ b/src/libcamera/formats.cpp
> @@ -987,7 +987,7 @@ unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane,
>                 return 0;
>         }
>  
> -       if (plane > planes.size() || !planes[plane].bytesPerGroup) {
> +       if (plane >= planes.size() || !planes[plane].bytesPerGroup) {
>                 LOG(Formats, Warning) << "Invalid plane index, stride is zero";
>                 return 0;
>         }
> -- 
> 2.34.1
>
Laurent Pinchart Feb. 26, 2024, 10:02 a.m. UTC | #2
On Mon, Feb 26, 2024 at 09:56:49AM +0000, Kieran Bingham wrote:
> Quoting Kieran Bingham (2024-02-26 09:48:56)
> > The plane validation in the stride helper incorrectly accepts the number
> > of planes as a plane index. Fix the off by one issue.
> > 
> > Reported-by: Johan Mattsson <39247600+mjunix@users.noreply.github.com>
> 
> As that's a @users.noreply this bounces and I don't think I can really
> keep this as is, even though that's what Github seems to provide.
> 
> I could do:
> 
> Reported-by: Johan Mattsson <mjunix@github.com>
> 
> Perhaps, or simply lose the reporter.
> Opinions anyone ?

If there's no usable e-mail address, I'd just drop the reporter. The
main point of the Reported-by tag is (in my opinion) to be able to
contact the reporter to get more information. In this case the problem
is trivial, so there's no big value in reporter information.

> > Fixes: e83727a194b5 ("libcamera: PixelFormatInfo: Add functions stride and frameSize")
> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > ---
> >  src/libcamera/formats.cpp | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> > index 447e623803c7..c11fbd730c8e 100644
> > --- a/src/libcamera/formats.cpp
> > +++ b/src/libcamera/formats.cpp
> > @@ -987,7 +987,7 @@ unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane,
> >                 return 0;
> >         }
> >  
> > -       if (plane > planes.size() || !planes[plane].bytesPerGroup) {
> > +       if (plane >= planes.size() || !planes[plane].bytesPerGroup) {
> >                 LOG(Formats, Warning) << "Invalid plane index, stride is zero";
> >                 return 0;
> >         }
Laurent Pinchart Feb. 26, 2024, 10:06 a.m. UTC | #3
On Mon, Feb 26, 2024 at 12:02:07PM +0200, Laurent Pinchart wrote:
> On Mon, Feb 26, 2024 at 09:56:49AM +0000, Kieran Bingham wrote:
> > Quoting Kieran Bingham (2024-02-26 09:48:56)
> > > The plane validation in the stride helper incorrectly accepts the number
> > > of planes as a plane index. Fix the off by one issue.
> > > 
> > > Reported-by: Johan Mattsson <39247600+mjunix@users.noreply.github.com>
> > 
> > As that's a @users.noreply this bounces and I don't think I can really
> > keep this as is, even though that's what Github seems to provide.
> > 
> > I could do:
> > 
> > Reported-by: Johan Mattsson <mjunix@github.com>
> > 
> > Perhaps, or simply lose the reporter.
> > Opinions anyone ?
> 
> If there's no usable e-mail address, I'd just drop the reporter. The
> main point of the Reported-by tag is (in my opinion) to be able to
> contact the reporter to get more information. In this case the problem
> is trivial, so there's no big value in reporter information.

With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> > > Fixes: e83727a194b5 ("libcamera: PixelFormatInfo: Add functions stride and frameSize")
> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> > > ---
> > >  src/libcamera/formats.cpp | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
> > > index 447e623803c7..c11fbd730c8e 100644
> > > --- a/src/libcamera/formats.cpp
> > > +++ b/src/libcamera/formats.cpp
> > > @@ -987,7 +987,7 @@ unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane,
> > >                 return 0;
> > >         }
> > >  
> > > -       if (plane > planes.size() || !planes[plane].bytesPerGroup) {
> > > +       if (plane >= planes.size() || !planes[plane].bytesPerGroup) {
> > >                 LOG(Formats, Warning) << "Invalid plane index, stride is zero";
> > >                 return 0;
> > >         }
Umang Jain Feb. 27, 2024, 9:32 a.m. UTC | #4
Hi Kieran,

On 26/02/24 3:36 pm, Laurent Pinchart wrote:
> On Mon, Feb 26, 2024 at 12:02:07PM +0200, Laurent Pinchart wrote:
>> On Mon, Feb 26, 2024 at 09:56:49AM +0000, Kieran Bingham wrote:
>>> Quoting Kieran Bingham (2024-02-26 09:48:56)
>>>> The plane validation in the stride helper incorrectly accepts the number
>>>> of planes as a plane index. Fix the off by one issue.
>>>>
>>>> Reported-by: Johan Mattsson <39247600+mjunix@users.noreply.github.com>
>>> As that's a @users.noreply this bounces and I don't think I can really
>>> keep this as is, even though that's what Github seems to provide.
>>>
>>> I could do:
>>>
>>> Reported-by: Johan Mattsson <mjunix@github.com>
>>>
>>> Perhaps, or simply lose the reporter.
>>> Opinions anyone ?
>> If there's no usable e-mail address, I'd just drop the reporter. The
>> main point of the Reported-by tag is (in my opinion) to be able to
>> contact the reporter to get more information. In this case the problem
>> is trivial, so there's no big value in reporter information.
> With this fixed,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>

>
>>>> Fixes: e83727a194b5 ("libcamera: PixelFormatInfo: Add functions stride and frameSize")
>>>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>>> ---
>>>>   src/libcamera/formats.cpp | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
>>>> index 447e623803c7..c11fbd730c8e 100644
>>>> --- a/src/libcamera/formats.cpp
>>>> +++ b/src/libcamera/formats.cpp
>>>> @@ -987,7 +987,7 @@ unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane,
>>>>                  return 0;
>>>>          }
>>>>   
>>>> -       if (plane > planes.size() || !planes[plane].bytesPerGroup) {
>>>> +       if (plane >= planes.size() || !planes[plane].bytesPerGroup) {
>>>>                  LOG(Formats, Warning) << "Invalid plane index, stride is zero";
>>>>                  return 0;
>>>>          }

Patch
diff mbox series

diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp
index 447e623803c7..c11fbd730c8e 100644
--- a/src/libcamera/formats.cpp
+++ b/src/libcamera/formats.cpp
@@ -987,7 +987,7 @@  unsigned int PixelFormatInfo::stride(unsigned int width, unsigned int plane,
 		return 0;
 	}
 
-	if (plane > planes.size() || !planes[plane].bytesPerGroup) {
+	if (plane >= planes.size() || !planes[plane].bytesPerGroup) {
 		LOG(Formats, Warning) << "Invalid plane index, stride is zero";
 		return 0;
 	}