Re: [igt-dev] [PATCH i-g-t] lib/igt_fb: fix the plane_size array for yuv formats

[Mark Yacoub <markyacoub@chromium.org>]

On Mon, Nov 29, 2021 at 4:00 PM Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
>
> clear_yuv_buffer() checks the size of the plane_size[] to
> make sure its greater than the number of planes to avoid
> overflows.
>
> The plane_size[] is fixed to two currently.
>
> However some of the formats like YV12 indeed have more than
> 2 planes in the format_desc[] hence this incorrectly failing
> this check.
>
> Increase the size of the plane_size[] to match the correct
> max number of planes.
>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
>  lib/igt_fb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 92a0170..e42302d 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1007,7 +1007,7 @@ static void memset64(uint64_t *s, uint64_t c, size_t n)
>  static void clear_yuv_buffer(struct igt_fb *fb)
>  {
>         bool full_range = fb->color_range == IGT_COLOR_YCBCR_FULL_RANGE;
> -       size_t plane_size[2];
> +       size_t plane_size[3];
I haven't looked closely at this code before but i took a look now and
2 things come to mind:

1. can we use lookup_drm_format(fb->drm_format)->num_planes for the
size of the plane_size array, this way it's expandable as needed (we
would need to add asserts to check for size at each switch-case.
2. Starting Line 1029, we clear up the memory up to 2 planes. if we
would use 3 planes, where are we using the 3rd one to clear up the
memory as well?

>         void *ptr;
>
>         igt_assert(igt_format_is_yuv(fb->drm_format));
> --
> 2.7.4
>