[v1] media: amlogic-c3: Fix out-of-bounds read of metering zone coordinates

[PATCH] media: amlogic-c3: Fix out-of-bounds read of metering zone coordinates

Posted by David Carlier 1 month, 1 week ago

The AWB, AE and AF coordinate loops bound themselves by
max(horiz_zones_num, vert_zones_num) + 1. Both counts are u8 values
taken verbatim from userspace, so the bound reaches 256 while the
coordinate arrays hold 18 entries (AE, AF) or 33 (AWB). An AF block
placed last in a full payload reads 474 bytes past the parameters
buffer.

Clamp the point count to the array size, as the zone weight loops
already do.

Cc: stable@vger.kernel.org
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
index ae0777a20bda..f2396e2c6640 100644
--- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
+++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
@@ -139,7 +139,8 @@ static void c3_isp_params_awb_cood(struct c3_isp_device *isp,
 	unsigned int max_point_num;
 
 	/* The number of points is one more than the number of edges */
-	max_point_num = max(cfg->horiz_zones_num, cfg->vert_zones_num) + 1;
+	max_point_num = min(max(cfg->horiz_zones_num, cfg->vert_zones_num) + 1,
+			    C3_ISP_AWB_MAX_PT_NUM);
 
 	/* Set the index address to 0 position */
 	c3_isp_write(isp, ISP_AWB_IDX_ADDR, 0);
@@ -258,7 +259,8 @@ static void c3_isp_params_ae_cood(struct c3_isp_device *isp,
 	unsigned int max_point_num;
 
 	/* The number of points is one more than the number of edges */
-	max_point_num = max(cfg->horiz_zones_num, cfg->vert_zones_num) + 1;
+	max_point_num = min(max(cfg->horiz_zones_num, cfg->vert_zones_num) + 1,
+			    C3_ISP_AE_MAX_PT_NUM);
 
 	/* Set the index address to 0 position */
 	c3_isp_write(isp, ISP_AE_IDX_ADDR, 0);
@@ -316,7 +318,8 @@ static void c3_isp_params_af_cood(struct c3_isp_device *isp,
 	unsigned int max_point_num;
 
 	/* The number of points is one more than the number of edges */
-	max_point_num = max(cfg->horiz_zones_num, cfg->vert_zones_num) + 1;
+	max_point_num = min(max(cfg->horiz_zones_num, cfg->vert_zones_num) + 1,
+			    C3_ISP_AF_MAX_PT_NUM);
 
 	/* Set the index address to 0 position */
 	c3_isp_write(isp, ISP_AF_IDX_ADDR, 0);
-- 
2.55.0

Re: [PATCH] media: amlogic-c3: Fix out-of-bounds read of metering zone coordinates

Posted by Keke Li 1 month ago

Hi David

Thanks for your patch.

On 8/22/26 05:28, David Carlier wrote:
> [You don't often get email from devnexen@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> [ EXTERNAL EMAIL ]
>
> The AWB, AE and AF coordinate loops bound themselves by
> max(horiz_zones_num, vert_zones_num) + 1. Both counts are u8 values
> taken verbatim from userspace, so the bound reaches 256 while the
> coordinate arrays hold 18 entries (AE, AF) or 33 (AWB). An AF block
> placed last in a full payload reads 474 bytes past the parameters
> buffer.
>
> Clamp the point count to the array size, as the zone weight loops
> already do.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>


Reviewed-by: Keke Li <keke.li@amlogic.com>

> ---
>   drivers/media/platform/amlogic/c3/isp/c3-isp-params.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> index ae0777a20bda..f2396e2c6640 100644
> --- a/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> +++ b/drivers/media/platform/amlogic/c3/isp/c3-isp-params.c
> @@ -139,7 +139,8 @@ static void c3_isp_params_awb_cood(struct c3_isp_device *isp,
>          unsigned int max_point_num;
>
>          /* The number of points is one more than the number of edges */
> -       max_point_num = max(cfg->horiz_zones_num, cfg->vert_zones_num) + 1;
> +       max_point_num = min(max(cfg->horiz_zones_num, cfg->vert_zones_num) + 1,
> +                           C3_ISP_AWB_MAX_PT_NUM);
>
>          /* Set the index address to 0 position */
>          c3_isp_write(isp, ISP_AWB_IDX_ADDR, 0);
> @@ -258,7 +259,8 @@ static void c3_isp_params_ae_cood(struct c3_isp_device *isp,
>          unsigned int max_point_num;
>
>          /* The number of points is one more than the number of edges */
> -       max_point_num = max(cfg->horiz_zones_num, cfg->vert_zones_num) + 1;
> +       max_point_num = min(max(cfg->horiz_zones_num, cfg->vert_zones_num) + 1,
> +                           C3_ISP_AE_MAX_PT_NUM);
>
>          /* Set the index address to 0 position */
>          c3_isp_write(isp, ISP_AE_IDX_ADDR, 0);
> @@ -316,7 +318,8 @@ static void c3_isp_params_af_cood(struct c3_isp_device *isp,
>          unsigned int max_point_num;
>
>          /* The number of points is one more than the number of edges */
> -       max_point_num = max(cfg->horiz_zones_num, cfg->vert_zones_num) + 1;
> +       max_point_num = min(max(cfg->horiz_zones_num, cfg->vert_zones_num) + 1,
> +                           C3_ISP_AF_MAX_PT_NUM);
>
>          /* Set the index address to 0 position */
>          c3_isp_write(isp, ISP_AF_IDX_ADDR, 0);
> --
> 2.55.0
>