* [PATCH] drm/kmb: fix dereference before NULL check in kmb_plane_atomic_update() @ 2022-07-29 3:07 ` Zeng Jingxiang 0 siblings, 0 replies; 7+ messages in thread From: Zeng Jingxiang @ 2022-07-29 3:07 UTC (permalink / raw) To: anitha.chrisanthus, edmund.j.dea, airlied, daniel, tzimmermann, laurent.pinchart, maxime, ville.syrjala Cc: dri-devel, linux-kernel, Zeng Jingxiang From: Zeng Jingxiang <linuszeng@tencent.com> The "plane" pointer was access before checking if it was NULL. The drm_atomic_get_old_plane_state() function will dereference the pointer "plane". 345 struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane); 346 struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane); A NULL check for "plane" indicates that it may be NULL 363 if (!plane || !new_plane_state || !old_plane_state) Fixes: 977697e20b3d ("drm/atomic: Pass the full state to planes atomic disable and update") Fixes: 37418bf14c13 ("drm: Use state helper instead of the plane state pointer") Signed-off-by: Zeng Jingxiang <linuszeng@tencent.com> --- drivers/gpu/drm/kmb/kmb_plane.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c index 2735b8eb3537..d2bc998b65ce 100644 --- a/drivers/gpu/drm/kmb/kmb_plane.c +++ b/drivers/gpu/drm/kmb/kmb_plane.c @@ -342,10 +342,7 @@ static void kmb_plane_set_alpha(struct kmb_drm_private *kmb, static void kmb_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_state *state) { - struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, - plane); - struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, - plane); + struct drm_plane_state *old_plane_state, *new_plane_state; struct drm_framebuffer *fb; struct kmb_drm_private *kmb; unsigned int width; @@ -360,7 +357,13 @@ static void kmb_plane_atomic_update(struct drm_plane *plane, static dma_addr_t addr[MAX_SUB_PLANES]; struct disp_cfg *init_disp_cfg; - if (!plane || !new_plane_state || !old_plane_state) + if (!plane) + return; + + old_plane_state = drm_atomic_get_old_plane_state(state, plane); + new_plane_state = drm_atomic_get_new_plane_state(state, plane); + + if (!new_plane_state || !old_plane_state) return; fb = new_plane_state->fb; -- 2.27.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH] drm/kmb: fix dereference before NULL check in kmb_plane_atomic_update() @ 2022-07-29 3:07 ` Zeng Jingxiang 0 siblings, 0 replies; 7+ messages in thread From: Zeng Jingxiang @ 2022-07-29 3:07 UTC (permalink / raw) To: anitha.chrisanthus, edmund.j.dea, airlied, daniel, tzimmermann, laurent.pinchart, maxime, ville.syrjala Cc: Zeng Jingxiang, linux-kernel, dri-devel From: Zeng Jingxiang <linuszeng@tencent.com> The "plane" pointer was access before checking if it was NULL. The drm_atomic_get_old_plane_state() function will dereference the pointer "plane". 345 struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, plane); 346 struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, plane); A NULL check for "plane" indicates that it may be NULL 363 if (!plane || !new_plane_state || !old_plane_state) Fixes: 977697e20b3d ("drm/atomic: Pass the full state to planes atomic disable and update") Fixes: 37418bf14c13 ("drm: Use state helper instead of the plane state pointer") Signed-off-by: Zeng Jingxiang <linuszeng@tencent.com> --- drivers/gpu/drm/kmb/kmb_plane.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c index 2735b8eb3537..d2bc998b65ce 100644 --- a/drivers/gpu/drm/kmb/kmb_plane.c +++ b/drivers/gpu/drm/kmb/kmb_plane.c @@ -342,10 +342,7 @@ static void kmb_plane_set_alpha(struct kmb_drm_private *kmb, static void kmb_plane_atomic_update(struct drm_plane *plane, struct drm_atomic_state *state) { - struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, - plane); - struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, - plane); + struct drm_plane_state *old_plane_state, *new_plane_state; struct drm_framebuffer *fb; struct kmb_drm_private *kmb; unsigned int width; @@ -360,7 +357,13 @@ static void kmb_plane_atomic_update(struct drm_plane *plane, static dma_addr_t addr[MAX_SUB_PLANES]; struct disp_cfg *init_disp_cfg; - if (!plane || !new_plane_state || !old_plane_state) + if (!plane) + return; + + old_plane_state = drm_atomic_get_old_plane_state(state, plane); + new_plane_state = drm_atomic_get_new_plane_state(state, plane); + + if (!new_plane_state || !old_plane_state) return; fb = new_plane_state->fb; -- 2.27.0 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/kmb: fix dereference before NULL check in kmb_plane_atomic_update() 2022-07-29 3:07 ` Zeng Jingxiang @ 2022-07-29 14:23 ` Thomas Zimmermann -1 siblings, 0 replies; 7+ messages in thread From: Thomas Zimmermann @ 2022-07-29 14:23 UTC (permalink / raw) To: Zeng Jingxiang, anitha.chrisanthus, edmund.j.dea, airlied, daniel, laurent.pinchart, maxime, ville.syrjala Cc: dri-devel, linux-kernel, Zeng Jingxiang [-- Attachment #1.1: Type: text/plain, Size: 2626 bytes --] Hi Am 29.07.22 um 05:07 schrieb Zeng Jingxiang: > From: Zeng Jingxiang <linuszeng@tencent.com> > > The "plane" pointer was access before checking if it was NULL. > > The drm_atomic_get_old_plane_state() function will dereference > the pointer "plane". > 345 struct drm_plane_state *old_plane_state = > drm_atomic_get_old_plane_state(state, plane); > 346 struct drm_plane_state *new_plane_state = > drm_atomic_get_new_plane_state(state, plane); > > A NULL check for "plane" indicates that it may be NULL > 363 if (!plane || !new_plane_state || !old_plane_state) Is this an actual bug that happens? All planes should always have a state. Therefore the tests for !new_plane_state and !old_plane_state can be removed, I'd say. Best regards Thomas > > Fixes: 977697e20b3d ("drm/atomic: Pass the full state to planes atomic disable and update") > Fixes: 37418bf14c13 ("drm: Use state helper instead of the plane state pointer") > Signed-off-by: Zeng Jingxiang <linuszeng@tencent.com> > --- > drivers/gpu/drm/kmb/kmb_plane.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c > index 2735b8eb3537..d2bc998b65ce 100644 > --- a/drivers/gpu/drm/kmb/kmb_plane.c > +++ b/drivers/gpu/drm/kmb/kmb_plane.c > @@ -342,10 +342,7 @@ static void kmb_plane_set_alpha(struct kmb_drm_private *kmb, > static void kmb_plane_atomic_update(struct drm_plane *plane, > struct drm_atomic_state *state) > { > - struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, > - plane); > - struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, > - plane); > + struct drm_plane_state *old_plane_state, *new_plane_state; > struct drm_framebuffer *fb; > struct kmb_drm_private *kmb; > unsigned int width; > @@ -360,7 +357,13 @@ static void kmb_plane_atomic_update(struct drm_plane *plane, > static dma_addr_t addr[MAX_SUB_PLANES]; > struct disp_cfg *init_disp_cfg; > > - if (!plane || !new_plane_state || !old_plane_state) > + if (!plane) > + return; > + > + old_plane_state = drm_atomic_get_old_plane_state(state, plane); > + new_plane_state = drm_atomic_get_new_plane_state(state, plane); > + > + if (!new_plane_state || !old_plane_state) > return; > > fb = new_plane_state->fb; -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/kmb: fix dereference before NULL check in kmb_plane_atomic_update() @ 2022-07-29 14:23 ` Thomas Zimmermann 0 siblings, 0 replies; 7+ messages in thread From: Thomas Zimmermann @ 2022-07-29 14:23 UTC (permalink / raw) To: Zeng Jingxiang, anitha.chrisanthus, edmund.j.dea, airlied, daniel, laurent.pinchart, maxime, ville.syrjala Cc: Zeng Jingxiang, linux-kernel, dri-devel [-- Attachment #1.1: Type: text/plain, Size: 2626 bytes --] Hi Am 29.07.22 um 05:07 schrieb Zeng Jingxiang: > From: Zeng Jingxiang <linuszeng@tencent.com> > > The "plane" pointer was access before checking if it was NULL. > > The drm_atomic_get_old_plane_state() function will dereference > the pointer "plane". > 345 struct drm_plane_state *old_plane_state = > drm_atomic_get_old_plane_state(state, plane); > 346 struct drm_plane_state *new_plane_state = > drm_atomic_get_new_plane_state(state, plane); > > A NULL check for "plane" indicates that it may be NULL > 363 if (!plane || !new_plane_state || !old_plane_state) Is this an actual bug that happens? All planes should always have a state. Therefore the tests for !new_plane_state and !old_plane_state can be removed, I'd say. Best regards Thomas > > Fixes: 977697e20b3d ("drm/atomic: Pass the full state to planes atomic disable and update") > Fixes: 37418bf14c13 ("drm: Use state helper instead of the plane state pointer") > Signed-off-by: Zeng Jingxiang <linuszeng@tencent.com> > --- > drivers/gpu/drm/kmb/kmb_plane.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/kmb/kmb_plane.c b/drivers/gpu/drm/kmb/kmb_plane.c > index 2735b8eb3537..d2bc998b65ce 100644 > --- a/drivers/gpu/drm/kmb/kmb_plane.c > +++ b/drivers/gpu/drm/kmb/kmb_plane.c > @@ -342,10 +342,7 @@ static void kmb_plane_set_alpha(struct kmb_drm_private *kmb, > static void kmb_plane_atomic_update(struct drm_plane *plane, > struct drm_atomic_state *state) > { > - struct drm_plane_state *old_plane_state = drm_atomic_get_old_plane_state(state, > - plane); > - struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state, > - plane); > + struct drm_plane_state *old_plane_state, *new_plane_state; > struct drm_framebuffer *fb; > struct kmb_drm_private *kmb; > unsigned int width; > @@ -360,7 +357,13 @@ static void kmb_plane_atomic_update(struct drm_plane *plane, > static dma_addr_t addr[MAX_SUB_PLANES]; > struct disp_cfg *init_disp_cfg; > > - if (!plane || !new_plane_state || !old_plane_state) > + if (!plane) > + return; > + > + old_plane_state = drm_atomic_get_old_plane_state(state, plane); > + new_plane_state = drm_atomic_get_new_plane_state(state, plane); > + > + if (!new_plane_state || !old_plane_state) > return; > > fb = new_plane_state->fb; -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/kmb: fix dereference before NULL check in kmb_plane_atomic_update() 2022-07-29 14:23 ` Thomas Zimmermann (?) @ 2022-07-29 14:25 ` Thomas Zimmermann 2022-07-29 16:40 ` Chrisanthus, Anitha -1 siblings, 1 reply; 7+ messages in thread From: Thomas Zimmermann @ 2022-07-29 14:25 UTC (permalink / raw) To: Zeng Jingxiang, anitha.chrisanthus, edmund.j.dea, airlied, daniel, laurent.pinchart, maxime, ville.syrjala Cc: Zeng Jingxiang, linux-kernel, dri-devel [-- Attachment #1.1: Type: text/plain, Size: 3268 bytes --] Hi Am 29.07.22 um 16:23 schrieb Thomas Zimmermann: > Hi > > Am 29.07.22 um 05:07 schrieb Zeng Jingxiang: >> From: Zeng Jingxiang <linuszeng@tencent.com> >> >> The "plane" pointer was access before checking if it was NULL. >> >> The drm_atomic_get_old_plane_state() function will dereference >> the pointer "plane". >> 345 struct drm_plane_state *old_plane_state = >> drm_atomic_get_old_plane_state(state, plane); >> 346 struct drm_plane_state *new_plane_state = >> drm_atomic_get_new_plane_state(state, plane); >> >> A NULL check for "plane" indicates that it may be NULL >> 363 if (!plane || !new_plane_state || !old_plane_state) > > Is this an actual bug that happens? > > All planes should always have a state. Therefore the tests for > !new_plane_state and !old_plane_state can be removed, I'd say. Just to clarify: moving the test for !plane before using one of the get_plane_state functions is a correct bugfix. Best regards Thomas > > Best regards > Thomas > >> >> Fixes: 977697e20b3d ("drm/atomic: Pass the full state to planes atomic >> disable and update") >> Fixes: 37418bf14c13 ("drm: Use state helper instead of the plane state >> pointer") >> Signed-off-by: Zeng Jingxiang <linuszeng@tencent.com> >> --- >> drivers/gpu/drm/kmb/kmb_plane.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/kmb/kmb_plane.c >> b/drivers/gpu/drm/kmb/kmb_plane.c >> index 2735b8eb3537..d2bc998b65ce 100644 >> --- a/drivers/gpu/drm/kmb/kmb_plane.c >> +++ b/drivers/gpu/drm/kmb/kmb_plane.c >> @@ -342,10 +342,7 @@ static void kmb_plane_set_alpha(struct >> kmb_drm_private *kmb, >> static void kmb_plane_atomic_update(struct drm_plane *plane, >> struct drm_atomic_state *state) >> { >> - struct drm_plane_state *old_plane_state = >> drm_atomic_get_old_plane_state(state, >> - plane); >> - struct drm_plane_state *new_plane_state = >> drm_atomic_get_new_plane_state(state, >> - plane); >> + struct drm_plane_state *old_plane_state, *new_plane_state; >> struct drm_framebuffer *fb; >> struct kmb_drm_private *kmb; >> unsigned int width; >> @@ -360,7 +357,13 @@ static void kmb_plane_atomic_update(struct >> drm_plane *plane, >> static dma_addr_t addr[MAX_SUB_PLANES]; >> struct disp_cfg *init_disp_cfg; >> - if (!plane || !new_plane_state || !old_plane_state) >> + if (!plane) >> + return; >> + >> + old_plane_state = drm_atomic_get_old_plane_state(state, plane); >> + new_plane_state = drm_atomic_get_new_plane_state(state, plane); >> + >> + if (!new_plane_state || !old_plane_state) >> return; >> fb = new_plane_state->fb; > -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] drm/kmb: fix dereference before NULL check in kmb_plane_atomic_update() 2022-07-29 14:25 ` Thomas Zimmermann @ 2022-07-29 16:40 ` Chrisanthus, Anitha 2022-08-01 10:27 ` Thomas Zimmermann 0 siblings, 1 reply; 7+ messages in thread From: Chrisanthus, Anitha @ 2022-07-29 16:40 UTC (permalink / raw) To: Thomas Zimmermann, Zeng Jingxiang, edmund.j.dea, airlied, daniel, laurent.pinchart, maxime, ville.syrjala Cc: Zeng Jingxiang, linux-kernel, dri-devel Agree with Thomas, drm_atomic_commit() will not call kmb_atomic_update() with a NULL plane. This is not an actual bug. Thanks, Anitha > -----Original Message----- > From: Thomas Zimmermann <tzimmermann@suse.de> > Sent: Friday, July 29, 2022 7:26 AM > To: Zeng Jingxiang <zengjx95@gmail.com>; Chrisanthus, Anitha > <anitha.chrisanthus@intel.com>; edmund.j.dea@intel.com; airlied@linux.ie; > daniel@ffwll.ch; laurent.pinchart@ideasonboard.com; maxime@cerno.tech; > ville.syrjala@linux.intel.com > Cc: Zeng Jingxiang <linuszeng@tencent.com>; linux-kernel@vger.kernel.org; > dri-devel@lists.freedesktop.org > Subject: Re: [PATCH] drm/kmb: fix dereference before NULL check in > kmb_plane_atomic_update() > > Hi > > Am 29.07.22 um 16:23 schrieb Thomas Zimmermann: > > Hi > > > > Am 29.07.22 um 05:07 schrieb Zeng Jingxiang: > >> From: Zeng Jingxiang <linuszeng@tencent.com> > >> > >> The "plane" pointer was access before checking if it was NULL. > >> > >> The drm_atomic_get_old_plane_state() function will dereference > >> the pointer "plane". > >> 345 struct drm_plane_state *old_plane_state = > >> drm_atomic_get_old_plane_state(state, plane); > >> 346 struct drm_plane_state *new_plane_state = > >> drm_atomic_get_new_plane_state(state, plane); > >> > >> A NULL check for "plane" indicates that it may be NULL > >> 363 if (!plane || !new_plane_state || !old_plane_state) > > > > Is this an actual bug that happens? > > > > All planes should always have a state. Therefore the tests for > > !new_plane_state and !old_plane_state can be removed, I'd say. > > Just to clarify: moving the test for !plane before using one of the > get_plane_state functions is a correct bugfix. > > Best regards > Thomas > > > > > Best regards > > Thomas > > > >> > >> Fixes: 977697e20b3d ("drm/atomic: Pass the full state to planes atomic > >> disable and update") > >> Fixes: 37418bf14c13 ("drm: Use state helper instead of the plane state > >> pointer") > >> Signed-off-by: Zeng Jingxiang <linuszeng@tencent.com> > >> --- > >> drivers/gpu/drm/kmb/kmb_plane.c | 13 ++++++++----- > >> 1 file changed, 8 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/kmb/kmb_plane.c > >> b/drivers/gpu/drm/kmb/kmb_plane.c > >> index 2735b8eb3537..d2bc998b65ce 100644 > >> --- a/drivers/gpu/drm/kmb/kmb_plane.c > >> +++ b/drivers/gpu/drm/kmb/kmb_plane.c > >> @@ -342,10 +342,7 @@ static void kmb_plane_set_alpha(struct > >> kmb_drm_private *kmb, > >> static void kmb_plane_atomic_update(struct drm_plane *plane, > >> struct drm_atomic_state *state) > >> { > >> - struct drm_plane_state *old_plane_state = > >> drm_atomic_get_old_plane_state(state, > >> - plane); > >> - struct drm_plane_state *new_plane_state = > >> drm_atomic_get_new_plane_state(state, > >> - plane); > >> + struct drm_plane_state *old_plane_state, *new_plane_state; > >> struct drm_framebuffer *fb; > >> struct kmb_drm_private *kmb; > >> unsigned int width; > >> @@ -360,7 +357,13 @@ static void kmb_plane_atomic_update(struct > >> drm_plane *plane, > >> static dma_addr_t addr[MAX_SUB_PLANES]; > >> struct disp_cfg *init_disp_cfg; > >> - if (!plane || !new_plane_state || !old_plane_state) > >> + if (!plane) > >> + return; > >> + > >> + old_plane_state = drm_atomic_get_old_plane_state(state, plane); > >> + new_plane_state = drm_atomic_get_new_plane_state(state, plane); > >> + > >> + if (!new_plane_state || !old_plane_state) > >> return; > >> fb = new_plane_state->fb; > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Ivo Totev ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/kmb: fix dereference before NULL check in kmb_plane_atomic_update() 2022-07-29 16:40 ` Chrisanthus, Anitha @ 2022-08-01 10:27 ` Thomas Zimmermann 0 siblings, 0 replies; 7+ messages in thread From: Thomas Zimmermann @ 2022-08-01 10:27 UTC (permalink / raw) To: Chrisanthus, Anitha, Zeng Jingxiang, edmund.j.dea, airlied, daniel, laurent.pinchart, maxime, ville.syrjala Cc: Zeng Jingxiang, linux-kernel, dri-devel [-- Attachment #1.1: Type: text/plain, Size: 4782 bytes --] Hi Am 29.07.22 um 18:40 schrieb Chrisanthus, Anitha: > Agree with Thomas, drm_atomic_commit() will not call kmb_atomic_update() with a NULL plane. This is not an actual bug. Indeed, it's the atomic_update function. I didn't notice at first. > > Thanks, > Anitha > >> -----Original Message----- >> From: Thomas Zimmermann <tzimmermann@suse.de> >> Sent: Friday, July 29, 2022 7:26 AM >> To: Zeng Jingxiang <zengjx95@gmail.com>; Chrisanthus, Anitha >> <anitha.chrisanthus@intel.com>; edmund.j.dea@intel.com; airlied@linux.ie; >> daniel@ffwll.ch; laurent.pinchart@ideasonboard.com; maxime@cerno.tech; >> ville.syrjala@linux.intel.com >> Cc: Zeng Jingxiang <linuszeng@tencent.com>; linux-kernel@vger.kernel.org; >> dri-devel@lists.freedesktop.org >> Subject: Re: [PATCH] drm/kmb: fix dereference before NULL check in >> kmb_plane_atomic_update() >> >> Hi >> >> Am 29.07.22 um 16:23 schrieb Thomas Zimmermann: >>> Hi >>> >>> Am 29.07.22 um 05:07 schrieb Zeng Jingxiang: >>>> From: Zeng Jingxiang <linuszeng@tencent.com> >>>> >>>> The "plane" pointer was access before checking if it was NULL. >>>> >>>> The drm_atomic_get_old_plane_state() function will dereference >>>> the pointer "plane". >>>> 345 struct drm_plane_state *old_plane_state = >>>> drm_atomic_get_old_plane_state(state, plane); >>>> 346 struct drm_plane_state *new_plane_state = >>>> drm_atomic_get_new_plane_state(state, plane); >>>> >>>> A NULL check for "plane" indicates that it may be NULL >>>> 363 if (!plane || !new_plane_state || !old_plane_state) >>> >>> Is this an actual bug that happens? >>> >>> All planes should always have a state. Therefore the tests for >>> !new_plane_state and !old_plane_state can be removed, I'd say. >> >> Just to clarify: moving the test for !plane before using one of the >> get_plane_state functions is a correct bugfix. >> >> Best regards >> Thomas >> >>> >>> Best regards >>> Thomas >>> >>>> >>>> Fixes: 977697e20b3d ("drm/atomic: Pass the full state to planes atomic >>>> disable and update") >>>> Fixes: 37418bf14c13 ("drm: Use state helper instead of the plane state >>>> pointer") >>>> Signed-off-by: Zeng Jingxiang <linuszeng@tencent.com> >>>> --- >>>> drivers/gpu/drm/kmb/kmb_plane.c | 13 ++++++++----- >>>> 1 file changed, 8 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/kmb/kmb_plane.c >>>> b/drivers/gpu/drm/kmb/kmb_plane.c >>>> index 2735b8eb3537..d2bc998b65ce 100644 >>>> --- a/drivers/gpu/drm/kmb/kmb_plane.c >>>> +++ b/drivers/gpu/drm/kmb/kmb_plane.c >>>> @@ -342,10 +342,7 @@ static void kmb_plane_set_alpha(struct >>>> kmb_drm_private *kmb, >>>> static void kmb_plane_atomic_update(struct drm_plane *plane, >>>> struct drm_atomic_state *state) >>>> { >>>> - struct drm_plane_state *old_plane_state = >>>> drm_atomic_get_old_plane_state(state, >>>> - plane); >>>> - struct drm_plane_state *new_plane_state = >>>> drm_atomic_get_new_plane_state(state, >>>> - plane); >>>> + struct drm_plane_state *old_plane_state, *new_plane_state; >>>> struct drm_framebuffer *fb; >>>> struct kmb_drm_private *kmb; >>>> unsigned int width; >>>> @@ -360,7 +357,13 @@ static void kmb_plane_atomic_update(struct >>>> drm_plane *plane, >>>> static dma_addr_t addr[MAX_SUB_PLANES]; >>>> struct disp_cfg *init_disp_cfg; >>>> - if (!plane || !new_plane_state || !old_plane_state) >>>> + if (!plane) >>>> + return; >>>> + >>>> + old_plane_state = drm_atomic_get_old_plane_state(state, plane); >>>> + new_plane_state = drm_atomic_get_new_plane_state(state, plane); >>>> + >>>> + if (!new_plane_state || !old_plane_state) To add to my previous reply, none of these variables can legally be NULL here. If plane if NULL, DRM helpers would run atomic_disable instead. If you want to fix anything, just remove the tests entirely and save a few CPU cycles. Best regards Thomas >>>> return; >>>> fb = new_plane_state->fb; >>> >> >> -- >> Thomas Zimmermann >> Graphics Driver Developer >> SUSE Software Solutions Germany GmbH >> Maxfeldstr. 5, 90409 Nürnberg, Germany >> (HRB 36809, AG Nürnberg) >> Geschäftsführer: Ivo Totev -- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 Nürnberg, Germany (HRB 36809, AG Nürnberg) Geschäftsführer: Ivo Totev [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 840 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-01 10:28 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-07-29 3:07 [PATCH] drm/kmb: fix dereference before NULL check in kmb_plane_atomic_update() Zeng Jingxiang 2022-07-29 3:07 ` Zeng Jingxiang 2022-07-29 14:23 ` Thomas Zimmermann 2022-07-29 14:23 ` Thomas Zimmermann 2022-07-29 14:25 ` Thomas Zimmermann 2022-07-29 16:40 ` Chrisanthus, Anitha 2022-08-01 10:27 ` Thomas Zimmermann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.