[PATCH] stm: ltdc: fix two incorrect NULL checks on list iterator

[PATCH] stm: ltdc: fix two incorrect NULL checks on list iterator

[Xiaomeng Tong]

The two bugs are here:
	if (encoder) {
	if (bridge && bridge->timings)

The list iterator value 'encoder/bridge' will *always* be set and
non-NULL by drm_for_each_encoder()/list_for_each_entry(), so it is
incorrect to assume that the iterator value will be NULL if the
list is empty or no element is found.

To fix the bug, use a new variable '*_iter' as the list iterator,
while use the old variable 'encoder/bridge' as a dedicated pointer
to point to the found element.

Cc: stable@vger.kernel.org
Fixes: 99e360442f223 ("drm/stm: Fix bus_flags handling")
Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
---
 drivers/gpu/drm/stm/ltdc.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/stm/ltdc.c b/drivers/gpu/drm/stm/ltdc.c
index dbdee954692a..d6124aa873e5 100644
--- a/drivers/gpu/drm/stm/ltdc.c
+++ b/drivers/gpu/drm/stm/ltdc.c
@@ -528,8 +528,8 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	struct drm_device *ddev = crtc->dev;
 	struct drm_connector_list_iter iter;
 	struct drm_connector *connector = NULL;
-	struct drm_encoder *encoder = NULL;
-	struct drm_bridge *bridge = NULL;
+	struct drm_encoder *encoder = NULL, *en_iter;
+	struct drm_bridge *bridge = NULL, *br_iter;
 	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
 	u32 hsync, vsync, accum_hbp, accum_vbp, accum_act_w, accum_act_h;
 	u32 total_width, total_height;
@@ -538,15 +538,19 @@ static void ltdc_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	int ret;
 
 	/* get encoder from crtc */
-	drm_for_each_encoder(encoder, ddev)
-		if (encoder->crtc == crtc)
+	drm_for_each_encoder(en_iter, ddev)
+		if (en_iter->crtc == crtc) {
+			encoder = en_iter;
 			break;
+		}
 
 	if (encoder) {
 		/* get bridge from encoder */
-		list_for_each_entry(bridge, &encoder->bridge_chain, chain_node)
-			if (bridge->encoder == encoder)
+		list_for_each_entry(br_iter, &encoder->bridge_chain, chain_node)
+			if (br_iter->encoder == encoder) {
+				bridge = br_iter;
 				break;
+			}
 
 		/* Get the connector from encoder */
 		drm_connector_list_iter_begin(ddev, &iter);
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] stm: ltdc: fix two incorrect NULL checks on list iterator

[Raphael Gallais-Pou]

Hello Xiaomeng

On 3/27/22 07:53, Xiaomeng Tong wrote:
> The two bugs are here:
> 	if (encoder) {
> 	if (bridge && bridge->timings)
>
> The list iterator value 'encoder/bridge' will *always* be set and
> non-NULL by drm_for_each_encoder()/list_for_each_entry(), so it is
> incorrect to assume that the iterator value will be NULL if the
> list is empty or no element is found.
>
> To fix the bug, use a new variable '*_iter' as the list iterator,
> while use the old variable 'encoder/bridge' as a dedicated pointer
> to point to the found element.
>
> Cc: stable@vger.kernel.org
> Fixes: 99e360442f223 ("drm/stm: Fix bus_flags handling")
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
> ---
>  drivers/gpu/drm/stm/ltdc.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)


Thanks for your fix

Acked-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>


Raphaël Gallais-Pou


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] stm: ltdc: fix two incorrect NULL checks on list iterator

[Philippe CORNU]

On 3/28/22 14:37, Raphael Gallais-Pou wrote:
> Hello Xiaomeng
> 
> On 3/27/22 07:53, Xiaomeng Tong wrote:
>> The two bugs are here:
>> 	if (encoder) {
>> 	if (bridge && bridge->timings)
>>
>> The list iterator value 'encoder/bridge' will *always* be set and
>> non-NULL by drm_for_each_encoder()/list_for_each_entry(), so it is
>> incorrect to assume that the iterator value will be NULL if the
>> list is empty or no element is found.
>>
>> To fix the bug, use a new variable '*_iter' as the list iterator,
>> while use the old variable 'encoder/bridge' as a dedicated pointer
>> to point to the found element.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 99e360442f223 ("drm/stm: Fix bus_flags handling")
>> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
>> ---
>>   drivers/gpu/drm/stm/ltdc.c | 16 ++++++++++------
>>   1 file changed, 10 insertions(+), 6 deletions(-)
> 
> 
> Thanks for your fix
> 
> Acked-by: Raphael Gallais-Pou <raphael.gallais-pou@foss.st.com>
> 
> 
> Raphaël Gallais-Pou
> 

Applied on drm-misc-next.
Many thanks for your patch,
Philippe :-)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel