[PATCH AUTOSEL 5.10 8/8] fbdev: omapfb: Add 'plane' value check

[PATCH AUTOSEL 5.10 8/8] fbdev: omapfb: Add 'plane' value check

[Sasha Levin]

From: Leonid Arapov <arapovl839@gmail.com>

[ Upstream commit 3e411827f31db7f938a30a3c7a7599839401ec30 ]

Function dispc_ovl_setup is not intended to work with the value OMAP_DSS_WB
of the enum parameter plane.

The value of this parameter is initialized in dss_init_overlays and in the
current state of the code it cannot take this value so it's not a real
problem.

For the purposes of defensive coding it wouldn't be superfluous to check
the parameter value, because some functions down the call stack process
this value correctly and some not.

For example, in dispc_ovl_setup_global_alpha it may lead to buffer
overflow.

Add check for this value.

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

Signed-off-by: Leonid Arapov <arapovl839@gmail.com>
Signed-off-by: Helge Deller <deller@gmx.de>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/video/fbdev/omap2/omapfb/dss/dispc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
index b2d6e6df21615..d852bef1d507f 100644
--- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
+++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
@@ -2751,9 +2751,13 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
 		bool mem_to_mem)
 {
 	int r;
-	enum omap_overlay_caps caps = dss_feat_get_overlay_caps(plane);
+	enum omap_overlay_caps caps;
 	enum omap_channel channel;
 
+	if (plane == OMAP_DSS_WB)
+		return -EINVAL;
+
+	caps = dss_feat_get_overlay_caps(plane);
 	channel = dispc_ovl_get_channel_out(plane);
 
 	DSSDBG("dispc_ovl_setup %d, pa %pad, pa_uv %pad, sw %d, %d,%d, %dx%d ->"
-- 
2.39.5

Re: [PATCH AUTOSEL 5.10 8/8] fbdev: omapfb: Add 'plane' value check

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1655 bytes --]

Hi!

> Function dispc_ovl_setup is not intended to work with the value OMAP_DSS_WB
> of the enum parameter plane.
> 
> The value of this parameter is initialized in dss_init_overlays and in the
> current state of the code it cannot take this value so it's not a real
> problem.
> 
> For the purposes of defensive coding it wouldn't be superfluous to check
> the parameter value, because some functions down the call stack process
> this value correctly and some not.
> 
> For example, in dispc_ovl_setup_global_alpha it may lead to buffer
> overflow.
> 
> Add check for this value.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE static
> analysis tool.

As changelog explains, this is robustness, not really a bug fix. We
should not need it in -stable. (Or maybe rules file should be updated,
because noone seems to be following this rule).

Best regards,
								Pavel
								
> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c
> @@ -2751,9 +2751,13 @@ int dispc_ovl_setup(enum omap_plane plane, const struct omap_overlay_info *oi,
>  		bool mem_to_mem)
>  {
>  	int r;
> -	enum omap_overlay_caps caps = dss_feat_get_overlay_caps(plane);
> +	enum omap_overlay_caps caps;
>  	enum omap_channel channel;
>  
> +	if (plane == OMAP_DSS_WB)
> +		return -EINVAL;
> +
> +	caps = dss_feat_get_overlay_caps(plane);
>  	channel = dispc_ovl_get_channel_out(plane);
>  
>  	DSSDBG("dispc_ovl_setup %d, pa %pad, pa_uv %pad, sw %d, %d,%d, %dx%d ->"

-- 
DENX Software Engineering GmbH,        Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]