[libcamera-devel,2/3] ipa: raspberrypi: Fix possible buffer overrun in metadata parsing
diff mbox series

Message ID 20210615144211.173047-3-naush@raspberrypi.com
State Superseded
Headers show
Series
  • Raspberry Pi: Metadata parsing improvements (II)
Related show

Commit Message

Naushir Patuck June 15, 2021, 2:42 p.m. UTC
The SMIA metadata parser could possibly read one byte past the end of the
buffer as the buffer size test ran after the read operation. Fix this.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/ipa/raspberrypi/md_parser_smia.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Kieran Bingham June 18, 2021, 10 p.m. UTC | #1
Hi Naush,

On 15/06/2021 15:42, Naushir Patuck wrote:
> The SMIA metadata parser could possibly read one byte past the end of the
> buffer as the buffer size test ran after the read operation. Fix this.
> 

Ohhh subtle, I wonder if this is in the coverity scan issues....

I can't see it there ...  perhaps it doesn't know that buffer.size() is
the size of the buffer though...

But it sounds right to me.

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> ---
>  src/ipa/raspberrypi/md_parser_smia.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/md_parser_smia.cpp b/src/ipa/raspberrypi/md_parser_smia.cpp
> index 5c413f1b55cc..0a14875575a2 100644
> --- a/src/ipa/raspberrypi/md_parser_smia.cpp
> +++ b/src/ipa/raspberrypi/md_parser_smia.cpp
> @@ -71,8 +71,8 @@ MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t>
>  					return NO_LINE_START;
>  			} else {
>  				/* allow a zero line length to mean "hunt for the next line" */
> -				while (buffer[current_offset] != LINE_START &&
> -				       current_offset < buffer.size())
> +				while (current_offset < buffer.size() &&
> +				       buffer[current_offset] != LINE_START)
>  					current_offset++;
>  
>  				if (current_offset == buffer.size())
>
David Plowman June 21, 2021, 7:59 a.m. UTC | #2
Hi Naush

Ouch, I wonder who first wrote that... thanks for fixing it!

On Tue, 15 Jun 2021 at 15:42, Naushir Patuck <naush@raspberrypi.com> wrote:
>
> The SMIA metadata parser could possibly read one byte past the end of the
> buffer as the buffer size test ran after the read operation. Fix this.
>
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

Reviewed-by: David Plowman <david.plowman@raspberrypi.com>

Thanks
David

> ---
>  src/ipa/raspberrypi/md_parser_smia.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/md_parser_smia.cpp b/src/ipa/raspberrypi/md_parser_smia.cpp
> index 5c413f1b55cc..0a14875575a2 100644
> --- a/src/ipa/raspberrypi/md_parser_smia.cpp
> +++ b/src/ipa/raspberrypi/md_parser_smia.cpp
> @@ -71,8 +71,8 @@ MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t>
>                                         return NO_LINE_START;
>                         } else {
>                                 /* allow a zero line length to mean "hunt for the next line" */
> -                               while (buffer[current_offset] != LINE_START &&
> -                                      current_offset < buffer.size())
> +                               while (current_offset < buffer.size() &&
> +                                      buffer[current_offset] != LINE_START)
>                                         current_offset++;
>
>                                 if (current_offset == buffer.size())
> --
> 2.25.1
>
Laurent Pinchart June 22, 2021, 10:28 a.m. UTC | #3
Hi Naush,

Thank you for the patch.

On Tue, Jun 15, 2021 at 03:42:10PM +0100, Naushir Patuck wrote:
> The SMIA metadata parser could possibly read one byte past the end of the
> buffer as the buffer size test ran after the read operation. Fix this.
> 
> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>

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

> ---
>  src/ipa/raspberrypi/md_parser_smia.cpp | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/md_parser_smia.cpp b/src/ipa/raspberrypi/md_parser_smia.cpp
> index 5c413f1b55cc..0a14875575a2 100644
> --- a/src/ipa/raspberrypi/md_parser_smia.cpp
> +++ b/src/ipa/raspberrypi/md_parser_smia.cpp
> @@ -71,8 +71,8 @@ MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t>
>  					return NO_LINE_START;
>  			} else {
>  				/* allow a zero line length to mean "hunt for the next line" */
> -				while (buffer[current_offset] != LINE_START &&
> -				       current_offset < buffer.size())
> +				while (current_offset < buffer.size() &&
> +				       buffer[current_offset] != LINE_START)
>  					current_offset++;
>  
>  				if (current_offset == buffer.size())

Patch
diff mbox series

diff --git a/src/ipa/raspberrypi/md_parser_smia.cpp b/src/ipa/raspberrypi/md_parser_smia.cpp
index 5c413f1b55cc..0a14875575a2 100644
--- a/src/ipa/raspberrypi/md_parser_smia.cpp
+++ b/src/ipa/raspberrypi/md_parser_smia.cpp
@@ -71,8 +71,8 @@  MdParserSmia::ParseStatus MdParserSmia::findRegs(libcamera::Span<const uint8_t>
 					return NO_LINE_START;
 			} else {
 				/* allow a zero line length to mean "hunt for the next line" */
-				while (buffer[current_offset] != LINE_START &&
-				       current_offset < buffer.size())
+				while (current_offset < buffer.size() &&
+				       buffer[current_offset] != LINE_START)
 					current_offset++;
 
 				if (current_offset == buffer.size())