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 |
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); >
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 --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);