KMB_MAX_PLANES is defined as 1, yet kmb_plane_atomic_disable() had code for writing beyond 1. It is gated by a WARN_ON() that would skip it, though, but under some compiler versions, poor Dead Code Elimination wasn't optimizing away the unused switch cases, leading to array bounds warnings when building with -Warray-bounds:
drivers/gpu/drm/kmb/kmb_plane.c:135:20: warning: array subscript 3 is above array bounds of 'struct layer_status[1]' [-Warray-bounds] drivers/gpu/drm/kmb/kmb_plane.c:132:20: warning: array subscript 2 is above array bounds of 'struct layer_status[1]' [-Warray-bounds] drivers/gpu/drm/kmb/kmb_plane.c:129:20: warning: array subscript 1 is above array bounds of 'struct layer_status[1]' [-Warray-bounds]
Instead, just remove the switch statement entirely and adjust the index type to match the struct "id" member.
Cc: Anitha Chrisanthus anitha.chrisanthus@intel.com Cc: Edmund Dea edmund.j.dea@intel.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: dri-devel@lists.freedesktop.org Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display") Signed-off-by: Kees Cook keescook@chromium.org --- drivers/gpu/drm/kmb/kmb_plane.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c index ecee6782612d..3d46e756f2fe 100644 --- a/drivers/gpu/drm/kmb/kmb_plane.c +++ b/drivers/gpu/drm/kmb/kmb_plane.c @@ -113,7 +113,7 @@ static void kmb_plane_atomic_disable(struct drm_plane *plane, struct drm_atomic_state *state) { struct kmb_plane *kmb_plane = to_kmb_plane(plane); - int plane_id = kmb_plane->id; + unsigned char plane_id = kmb_plane->id; struct kmb_drm_private *kmb;
kmb = to_kmb(plane->dev); @@ -121,21 +121,7 @@ static void kmb_plane_atomic_disable(struct drm_plane *plane, if (WARN_ON(plane_id >= KMB_MAX_PLANES)) return;
- switch (plane_id) { - case LAYER_0: - kmb->plane_status[plane_id].ctrl = LCD_CTRL_VL1_ENABLE; - break; - case LAYER_1: - kmb->plane_status[plane_id].ctrl = LCD_CTRL_VL2_ENABLE; - break; - case LAYER_2: - kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL1_ENABLE; - break; - case LAYER_3: - kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL2_ENABLE; - break; - } - + kmb->plane_status[plane_id].ctrl = LCD_CTRL_VL1_ENABLE; kmb->plane_status[plane_id].disable = true; }
Hi Kees, Thanks for your patch. The switch statement is needed for multiple planes which is already approved in this patch series. https://patchwork.kernel.org/project/dri-devel/patch/20210728003126.1425028-...
This patch has dependencies on the previous patches in this series, and we are waiting for reviews on those before this can be checked in.
Thanks, Anitha
-----Original Message----- From: Kees Cook keescook@chromium.org Sent: Wednesday, August 25, 2021 11:18 AM To: Chrisanthus, Anitha anitha.chrisanthus@intel.com Cc: Kees Cook keescook@chromium.org; Dea, Edmund J edmund.j.dea@intel.com; David Airlie airlied@linux.ie; Daniel Vetter daniel@ffwll.ch; dri-devel@lists.freedesktop.org; Sam Ravnborg sam@ravnborg.org; linux-kernel@vger.kernel.org; linux- hardening@vger.kernel.org Subject: [PATCH] drm/kmb: Avoid warnings on impossible plane_id
KMB_MAX_PLANES is defined as 1, yet kmb_plane_atomic_disable() had code for writing beyond 1. It is gated by a WARN_ON() that would skip it, though, but under some compiler versions, poor Dead Code Elimination wasn't optimizing away the unused switch cases, leading to array bounds warnings when building with -Warray-bounds:
drivers/gpu/drm/kmb/kmb_plane.c:135:20: warning: array subscript 3 is above array bounds of 'struct layer_status[1]' [-Warray-bounds] drivers/gpu/drm/kmb/kmb_plane.c:132:20: warning: array subscript 2 is above array bounds of 'struct layer_status[1]' [-Warray-bounds] drivers/gpu/drm/kmb/kmb_plane.c:129:20: warning: array subscript 1 is above array bounds of 'struct layer_status[1]' [-Warray-bounds]
Instead, just remove the switch statement entirely and adjust the index type to match the struct "id" member.
Cc: Anitha Chrisanthus anitha.chrisanthus@intel.com Cc: Edmund Dea edmund.j.dea@intel.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: dri-devel@lists.freedesktop.org Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display") Signed-off-by: Kees Cook keescook@chromium.org
drivers/gpu/drm/kmb/kmb_plane.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c index ecee6782612d..3d46e756f2fe 100644 --- a/drivers/gpu/drm/kmb/kmb_plane.c +++ b/drivers/gpu/drm/kmb/kmb_plane.c @@ -113,7 +113,7 @@ static void kmb_plane_atomic_disable(struct drm_plane *plane, struct drm_atomic_state *state) { struct kmb_plane *kmb_plane = to_kmb_plane(plane);
- int plane_id = kmb_plane->id;
unsigned char plane_id = kmb_plane->id; struct kmb_drm_private *kmb;
kmb = to_kmb(plane->dev);
@@ -121,21 +121,7 @@ static void kmb_plane_atomic_disable(struct drm_plane *plane, if (WARN_ON(plane_id >= KMB_MAX_PLANES)) return;
- switch (plane_id) {
- case LAYER_0:
kmb->plane_status[plane_id].ctrl = LCD_CTRL_VL1_ENABLE;break;- case LAYER_1:
kmb->plane_status[plane_id].ctrl = LCD_CTRL_VL2_ENABLE;break;- case LAYER_2:
kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL1_ENABLE;break;- case LAYER_3:
kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL2_ENABLE;break;- }
- kmb->plane_status[plane_id].ctrl = LCD_CTRL_VL1_ENABLE; kmb->plane_status[plane_id].disable = true;
}
-- 2.30.2
Hi Kees, This patch https://patchwork.kernel.org/project/dri-devel/patch/20210728003126.1425028-... is pushed to drm-misc-fixes. This change should fix the below warnings.
I apologize for all the inconveniences.
Thanks, Anitha
-----Original Message----- From: Chrisanthus, Anitha Sent: Wednesday, August 25, 2021 1:44 PM To: Kees Cook keescook@chromium.org Cc: Dea, Edmund J edmund.j.dea@intel.com; David Airlie airlied@linux.ie; Daniel Vetter daniel@ffwll.ch; dri-devel@lists.freedesktop.org; Sam Ravnborg sam@ravnborg.org; linux-kernel@vger.kernel.org; linux- hardening@vger.kernel.org Subject: RE: [PATCH] drm/kmb: Avoid warnings on impossible plane_id
Hi Kees, Thanks for your patch. The switch statement is needed for multiple planes which is already approved in this patch series. https://patchwork.kernel.org/project/dri- devel/patch/20210728003126.1425028-13-anitha.chrisanthus@intel.com/
This patch has dependencies on the previous patches in this series, and we are waiting for reviews on those before this can be checked in.
Thanks, Anitha
-----Original Message----- From: Kees Cook keescook@chromium.org Sent: Wednesday, August 25, 2021 11:18 AM To: Chrisanthus, Anitha anitha.chrisanthus@intel.com Cc: Kees Cook keescook@chromium.org; Dea, Edmund J edmund.j.dea@intel.com; David Airlie airlied@linux.ie; Daniel Vetter daniel@ffwll.ch; dri-devel@lists.freedesktop.org; Sam Ravnborg sam@ravnborg.org; linux-kernel@vger.kernel.org; linux- hardening@vger.kernel.org Subject: [PATCH] drm/kmb: Avoid warnings on impossible plane_id
KMB_MAX_PLANES is defined as 1, yet kmb_plane_atomic_disable() had
code
for writing beyond 1. It is gated by a WARN_ON() that would skip it, though, but under some compiler versions, poor Dead Code Elimination wasn't optimizing away the unused switch cases, leading to array bounds warnings when building with -Warray-bounds:
drivers/gpu/drm/kmb/kmb_plane.c:135:20: warning: array subscript 3 is above array bounds of 'struct layer_status[1]' [-Warray-bounds] drivers/gpu/drm/kmb/kmb_plane.c:132:20: warning: array subscript 2 is above array bounds of 'struct layer_status[1]' [-Warray-bounds] drivers/gpu/drm/kmb/kmb_plane.c:129:20: warning: array subscript 1 is above array bounds of 'struct layer_status[1]' [-Warray-bounds]
Instead, just remove the switch statement entirely and adjust the index type to match the struct "id" member.
Cc: Anitha Chrisanthus anitha.chrisanthus@intel.com Cc: Edmund Dea edmund.j.dea@intel.com Cc: David Airlie airlied@linux.ie Cc: Daniel Vetter daniel@ffwll.ch Cc: dri-devel@lists.freedesktop.org Fixes: 7f7b96a8a0a1 ("drm/kmb: Add support for KeemBay Display") Signed-off-by: Kees Cook keescook@chromium.org
drivers/gpu/drm/kmb/kmb_plane.c | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c index ecee6782612d..3d46e756f2fe 100644 --- a/drivers/gpu/drm/kmb/kmb_plane.c +++ b/drivers/gpu/drm/kmb/kmb_plane.c @@ -113,7 +113,7 @@ static void kmb_plane_atomic_disable(struct drm_plane *plane, struct drm_atomic_state *state) { struct kmb_plane *kmb_plane = to_kmb_plane(plane);
- int plane_id = kmb_plane->id;
unsigned char plane_id = kmb_plane->id; struct kmb_drm_private *kmb;
kmb = to_kmb(plane->dev);
@@ -121,21 +121,7 @@ static void kmb_plane_atomic_disable(struct drm_plane *plane, if (WARN_ON(plane_id >= KMB_MAX_PLANES)) return;
- switch (plane_id) {
- case LAYER_0:
kmb->plane_status[plane_id].ctrl = LCD_CTRL_VL1_ENABLE;break;- case LAYER_1:
kmb->plane_status[plane_id].ctrl = LCD_CTRL_VL2_ENABLE;break;- case LAYER_2:
kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL1_ENABLE;break;- case LAYER_3:
kmb->plane_status[plane_id].ctrl = LCD_CTRL_GL2_ENABLE;break;- }
- kmb->plane_status[plane_id].ctrl = LCD_CTRL_VL1_ENABLE; kmb->plane_status[plane_id].disable = true;
}
-- 2.30.2
On Tue, Sep 07, 2021 at 05:32:53PM +0000, Chrisanthus, Anitha wrote:
Hi Kees, This patch https://patchwork.kernel.org/project/dri-devel/patch/20210728003126.1425028-... is pushed to drm-misc-fixes. This change should fix the below warnings.
I apologize for all the inconveniences.
No worries; thanks for the heads-up!
(sfr, this change should fix the warning[1] uncovered by the KSPP tree now.)
-Kees
[1] https://lore.kernel.org/linux-next/20210830184429.1ee4b4d8@canb.auug.org.au/
dri-devel@lists.freedesktop.org
-
Chrisanthus, Anitha -
Kees Cook