2024-03-01 17:11:19

by Mikhail Lobanov

Subject: [PATCH] media: v4l2-tpg: Fix division by zero error in color_to_hsv

In the color_to_hsv function, division by zero is possible due to
attributes r,g,b are equal so diff_rgb = 0.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: 54fb15348385 ("[media] vivid: Add support for HSV formats")
Signed-off-by: Mikhail Lobanov <[email protected]>
---
drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
index a366566f22c3..943aab3ad97c 100644
--- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
+++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
@@ -597,6 +597,11 @@ static void color_to_hsv(struct tpg_data *tpg, int r, int g, int b,
third = third_size * 2;
}

+ if (!diff_rgb) {
+ *s = 0;
+ return;
+ }
+
aux *= third_size / 2;
aux += diff_rgb / 2;
aux /= diff_rgb;
--
2.39.2

2024-04-08 09:13:46

by Hans Verkuil

Subject: Re: [PATCH] media: v4l2-tpg: Fix division by zero error in color_to_hsv

On 01/03/2024 18:08, Mikhail Lobanov wrote:
> In the color_to_hsv function, division by zero is possible due to
> attributes r,g,b are equal so diff_rgb = 0.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 54fb15348385 ("[media] vivid: Add support for HSV formats")
> Signed-off-by: Mikhail Lobanov <[email protected]>

With this patch I get this compiler warning:

drivers/media/common/v4l2-tpg/v4l2-tpg-core.c: In function 'precalculate_color':
drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:1004:35: warning: 'h' may be used uninitialized [-Wmaybe-uninitialized]
1004 | tpg->colors[k][0] = h;
| ~~~~~~~~~~~~~~~~~~^~~
drivers/media/common/v4l2-tpg/v4l2-tpg-core.c:1001:21: note: 'h' was declared here
1001 | int h, s, v;
| ^

In any case, I think this patch is wrong.

Based on this formula:

https://www.rapidtables.com/convert/color/rgb-to-hsv.html

The bug is in:

*s = aux;
if (!aux) {
*h = 0;
return;
}

'if (!aux)' should be 'if (!diff_rgb)'.

Regards,

Hans

> ---
> drivers/media/common/v4l2-tpg/v4l2-tpg-core.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
> index a366566f22c3..943aab3ad97c 100644
> --- a/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
> +++ b/drivers/media/common/v4l2-tpg/v4l2-tpg-core.c
> @@ -597,6 +597,11 @@ static void color_to_hsv(struct tpg_data *tpg, int r, int g, int b,
> third = third_size * 2;
> }
>
> + if (!diff_rgb) {
> + *s = 0;
> + return;
> + }
> +
> aux *= third_size / 2;
> aux += diff_rgb / 2;
> aux /= diff_rgb;