diff mbox series

drm/komeda: Handle NULL pointer access code path in error case

Message ID 20201127110054.133686-1-carsten.haitzler@foss.arm.com (mailing list archive)
State New, archived
Headers show
Series drm/komeda: Handle NULL pointer access code path in error case | expand

Commit Message

Carsten Haitzler Nov. 27, 2020, 11 a.m. UTC
From: Carsten Haitzler <carsten.haitzler@arm.com>

komeda_component_get_old_state() technically can return a NULL
pointer. komeda_compiz_set_input() even warns when this happens, but
then proceeeds to use that NULL pointer tocompare memory content there
agains the new sate to see if it changed. In this case, it's better to
assume that the input changed as there is no old state to compare
against and thus assume the changes happen anyway.

Signed-off-by: Carsten Haitzler <carsten.haitzler@arm.com>
---
 drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Steven Price Nov. 27, 2020, 3:52 p.m. UTC | #1
On 27/11/2020 11:00, carsten.haitzler@foss.arm.com wrote:
> From: Carsten Haitzler <carsten.haitzler@arm.com>
> 
> komeda_component_get_old_state() technically can return a NULL
> pointer. komeda_compiz_set_input() even warns when this happens, but
> then proceeeds to use that NULL pointer tocompare memory content there
> agains the new sate to see if it changed. In this case, it's better to
> assume that the input changed as there is no old state to compare
> against and thus assume the changes happen anyway.
> 
> Signed-off-by: Carsten Haitzler <carsten.haitzler@arm.com>
> ---
>   drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index 8f32ae7c25d0..e8b1e15312d8 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -707,7 +707,8 @@ komeda_compiz_set_input(struct komeda_compiz *compiz,
>   	WARN_ON(!old_st);
>   
>   	/* compare with old to check if this input has been changed */
> -	if (memcmp(&(to_compiz_st(old_st)->cins[idx]), cin, sizeof(*cin)))
> +	if (!old_st ||
> +	    memcmp(&(to_compiz_st(old_st)->cins[idx]), cin, sizeof(*cin)))
>   		c_st->changed_active_inputs |= BIT(idx);

Even better, you can move the WARN_ON into the if:

	if (WARN_ON(!old_st) || ...

Either way:

Reviewed-by: Steven Price <steven.price@arm.com>

Steve

>   
>   	komeda_component_add_input(c_st, &dflow->input, idx);
>
Liviu Dudau Nov. 27, 2020, 10:58 p.m. UTC | #2
On Fri, Nov 27, 2020 at 11:00:54AM +0000, carsten.haitzler@foss.arm.com wrote:
> From: Carsten Haitzler <carsten.haitzler@arm.com>
> 
> komeda_component_get_old_state() technically can return a NULL
> pointer. komeda_compiz_set_input() even warns when this happens, but
> then proceeeds to use that NULL pointer tocompare memory content there
> agains the new sate to see if it changed. In this case, it's better to

s/sate/state/

> assume that the input changed as there is no old state to compare
> against and thus assume the changes happen anyway.
> 
> Signed-off-by: Carsten Haitzler <carsten.haitzler@arm.com>

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

I can make the small fix when pulling the patch for merge into drm-misc-next-fixes.

Best regards,
Liviu

> ---
>  drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> index 8f32ae7c25d0..e8b1e15312d8 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
> @@ -707,7 +707,8 @@ komeda_compiz_set_input(struct komeda_compiz *compiz,
>  	WARN_ON(!old_st);
>  
>  	/* compare with old to check if this input has been changed */
> -	if (memcmp(&(to_compiz_st(old_st)->cins[idx]), cin, sizeof(*cin)))
> +	if (!old_st ||
> +	    memcmp(&(to_compiz_st(old_st)->cins[idx]), cin, sizeof(*cin)))
>  		c_st->changed_active_inputs |= BIT(idx);
>  
>  	komeda_component_add_input(c_st, &dflow->input, idx);
> -- 
> 2.29.2
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
index 8f32ae7c25d0..e8b1e15312d8 100644
--- a/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
+++ b/drivers/gpu/drm/arm/display/komeda/komeda_pipeline_state.c
@@ -707,7 +707,8 @@  komeda_compiz_set_input(struct komeda_compiz *compiz,
 	WARN_ON(!old_st);
 
 	/* compare with old to check if this input has been changed */
-	if (memcmp(&(to_compiz_st(old_st)->cins[idx]), cin, sizeof(*cin)))
+	if (!old_st ||
+	    memcmp(&(to_compiz_st(old_st)->cins[idx]), cin, sizeof(*cin)))
 		c_st->changed_active_inputs |= BIT(idx);
 
 	komeda_component_add_input(c_st, &dflow->input, idx);