Thread (13 messages) 13 messages, 4 authors, 2023-03-15

Re: [PATCH v3 1/6] drm/rockchip: vop2: initialize possible_crtcs properly

From: Nathan Chancellor <nathan@kernel.org>
Date: 2023-03-14 16:08:33
Also in: dri-devel, linux-devicetree, linux-rockchip, lkml, llvm

Hi Michael,

On Tue, Jan 24, 2023 at 06:47:01AM +0100, Michael Riesch wrote:
quoted hunk ↗ jump to hunk
The variable possible_crtcs is only initialized for primary and
overlay planes. Since the VOP2 driver only supports these plane
types at the moment, the current code is safe. However, in order
to provide a future-proof solution, fix the initialization of
the variable.

Reported-by: kernel test robot <redacted>
Reported-by: Dan Carpenter <redacted>
Signed-off-by: Michael Riesch <redacted>
---
v3:
 - no changes
v2:
 - new patch

 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 8cecf81a5ae0..374ef821b453 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
@@ -2322,10 +2322,11 @@ static int vop2_create_crtc(struct vop2 *vop2)
 				/* change the unused primary window to overlay window */
 				win->type = DRM_PLANE_TYPE_OVERLAY;
 			}
-		}
-
-		if (win->type == DRM_PLANE_TYPE_OVERLAY)
+		} else if (win->type == DRM_PLANE_TYPE_OVERLAY) {
 			possible_crtcs = (1 << nvps) - 1;
+		} else {
+			possible_crtcs = 0;
+		}
 
 		ret = vop2_plane_init(vop2, win, possible_crtcs);
 		if (ret) {
-- 
2.30.2
This patch is now in -next as commit 368419a2d429 ("drm/rockchip: vop2:
initialize possible_crtcs properly") and it actually appears to
introduce a path where possible_crtcs could be used uninitialized.

  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2316:8: error: variable 'possible_crtcs' is used uninitialized whenever 'if' condition is false [-Werror,-Wsometimes-uninitialized]
                          if (vp) {
                              ^~
  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2330:36: note: uninitialized use occurs here
                  ret = vop2_plane_init(vop2, win, possible_crtcs);
                                                   ^~~~~~~~~~~~~~
  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2316:4: note: remove the 'if' if its condition is always true
                          if (vp) {
                          ^~~~~~~~
  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2298:21: note: initialize the variable 'possible_crtcs' to silence this warning
                  u32 possible_crtcs;
                                    ^
                                     = 0
  1 error generated.

Prior to this change, if that else path was hit, clang recognized based on
the assignment that the next if statement would always be true. Now, if
the else path is taken, the possible_crtcs assignment will be missed. Is
that intentional?

Cheers,
Nathan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help