Re: [PATCH] of: unittest: Fix out of bounds array access when id < 0

[Frank Rowand]

On 12/21/21 4:28 AM, Yin Xiujiang wrote:
> In of_unittest_untrack_overlay if id is less than 0 then
> overlay_id_bits will be out of bounds. And it is also mentioned
> in bugzilla as a bug report:
> https://bugzilla.kernel.org/show_bug.cgi?id=214867
> 
> Fix this bug by tiggering WARN_ON()

This patch is just papering over the underlying problems.

The tracking of overlay ids in unittest.c depends on some
expectations of the expected range of values of id to be
tracked.  The resulting implementation is a bit fragile.
Let me take a look at whether I can create an alternate
implementation of id tracking.

-Frank

> 
> Reported-by: Erhard F. <erhard_f@xxxxxxxxxxx>
> Signed-off-by: Yin Xiujiang <yinxiujiang@xxxxxxxxxx>
> ---
>  drivers/of/unittest.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index 5b85a2a3792a..094f9f4444d0 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -1907,7 +1907,7 @@ static int overlay_first_id = -1;
>  
>  static long of_unittest_overlay_tracked(int id)
>  {
> -	if (WARN_ON(id >= MAX_UNITTEST_OVERLAYS))
> +	if (WARN_ON(id >= MAX_UNITTEST_OVERLAYS || id < 0))
>  		return 0;
>  	return overlay_id_bits[BIT_WORD(id)] & BIT_MASK(id);
>  }
> @@ -1918,7 +1918,7 @@ static void of_unittest_track_overlay(int id)
>  		overlay_first_id = id;
>  	id -= overlay_first_id;
>  
> -	if (WARN_ON(id >= MAX_UNITTEST_OVERLAYS))
> +	if (WARN_ON(id >= MAX_UNITTEST_OVERLAYS || id < 0))
>  		return;
>  	overlay_id_bits[BIT_WORD(id)] |= BIT_MASK(id);
>  }
> @@ -1928,7 +1928,7 @@ static void of_unittest_untrack_overlay(int id)
>  	if (overlay_first_id < 0)
>  		return;
>  	id -= overlay_first_id;
> -	if (WARN_ON(id >= MAX_UNITTEST_OVERLAYS))
> +	if (WARN_ON(id >= MAX_UNITTEST_OVERLAYS || id < 0))
>  		return;
>  	overlay_id_bits[BIT_WORD(id)] &= ~BIT_MASK(id);
>  }
>