| Message ID | 20181115102303.24744-1-boris.brezillon@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] drm/vc4: Fix NULL pointer dereference in the async update path | expand |
On Thu, 15 Nov 2018 11:23:02 +0100 Boris Brezillon <boris.brezillon@bootlin.com> wrote: > vc4_plane_atomic_async_update() calls vc4_plane_atomic_check() > which in turn calls vc4_plane_setup_clipping_and_scaling(), and since > commit 58a6a36fe8e0 ("drm/vc4: Use > drm_atomic_helper_check_plane_state() to simplify the logic"), this > function accesses plane_state->state which will be NULL when called > from the async update path because we're passing the current plane > state, and plane_state->state has been assigned to NULL in > drm_atomic_helper_swap_state(). > > Pass the new state instead of the current one (the new state has > ->state set to a non-NULL value). > Forgot Fixes: 58a6a36fe8e0 ("drm/vc4: Use drm_atomic_helper_check_plane_state() to simplify the logic") > Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> > --- > Changes in v2: > - Pass the new state instead of plane->state (suggested by Eric) > --- > drivers/gpu/drm/vc4/vc4_plane.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c > index 1728fb7d00ba..c3ded0ba0441 100644 > --- a/drivers/gpu/drm/vc4/vc4_plane.c > +++ b/drivers/gpu/drm/vc4/vc4_plane.c > @@ -854,7 +854,7 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb) > static void vc4_plane_atomic_async_update(struct drm_plane *plane, > struct drm_plane_state *state) > { > - struct vc4_plane_state *vc4_state = to_vc4_plane_state(plane->state); > + struct vc4_plane_state *vc4_state, *new_vc4_state; > > if (plane->state->fb != state->fb) { > vc4_plane_async_set_fb(plane, state->fb); > @@ -875,7 +875,18 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, > plane->state->src_y = state->src_y; > > /* Update the display list based on the new crtc_x/y. */ > - vc4_plane_atomic_check(plane, plane->state); > + vc4_plane_atomic_check(plane, state); > + > + new_vc4_state = to_vc4_plane_state(state); > + vc4_state = to_vc4_plane_state(plane->state); > + > + /* Update the current vc4_state pos0, pos2 and ptr0 dlist entries. */ > + vc4_state->dlist[vc4_state->pos0_offset] = > + new_vc4_state->dlist[vc4_state->pos0_offset]; > + vc4_state->dlist[vc4_state->pos2_offset] = > + new_vc4_state->dlist[vc4_state->pos2_offset]; > + vc4_state->dlist[vc4_state->ptr0_offset] = > + new_vc4_state->dlist[vc4_state->ptr0_offset]; > > /* Note that we can't just call vc4_plane_write_dlist() > * because that would smash the context data that the HVS is
diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c index 1728fb7d00ba..c3ded0ba0441 100644 --- a/drivers/gpu/drm/vc4/vc4_plane.c +++ b/drivers/gpu/drm/vc4/vc4_plane.c @@ -854,7 +854,7 @@ void vc4_plane_async_set_fb(struct drm_plane *plane, struct drm_framebuffer *fb) static void vc4_plane_atomic_async_update(struct drm_plane *plane, struct drm_plane_state *state) { - struct vc4_plane_state *vc4_state = to_vc4_plane_state(plane->state); + struct vc4_plane_state *vc4_state, *new_vc4_state; if (plane->state->fb != state->fb) { vc4_plane_async_set_fb(plane, state->fb); @@ -875,7 +875,18 @@ static void vc4_plane_atomic_async_update(struct drm_plane *plane, plane->state->src_y = state->src_y; /* Update the display list based on the new crtc_x/y. */ - vc4_plane_atomic_check(plane, plane->state); + vc4_plane_atomic_check(plane, state); + + new_vc4_state = to_vc4_plane_state(state); + vc4_state = to_vc4_plane_state(plane->state); + + /* Update the current vc4_state pos0, pos2 and ptr0 dlist entries. */ + vc4_state->dlist[vc4_state->pos0_offset] = + new_vc4_state->dlist[vc4_state->pos0_offset]; + vc4_state->dlist[vc4_state->pos2_offset] = + new_vc4_state->dlist[vc4_state->pos2_offset]; + vc4_state->dlist[vc4_state->ptr0_offset] = + new_vc4_state->dlist[vc4_state->ptr0_offset]; /* Note that we can't just call vc4_plane_write_dlist() * because that would smash the context data that the HVS is
vc4_plane_atomic_async_update() calls vc4_plane_atomic_check() which in turn calls vc4_plane_setup_clipping_and_scaling(), and since commit 58a6a36fe8e0 ("drm/vc4: Use drm_atomic_helper_check_plane_state() to simplify the logic"), this function accesses plane_state->state which will be NULL when called from the async update path because we're passing the current plane state, and plane_state->state has been assigned to NULL in drm_atomic_helper_swap_state(). Pass the new state instead of the current one (the new state has ->state set to a non-NULL value). Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com> --- Changes in v2: - Pass the new state instead of plane->state (suggested by Eric) --- drivers/gpu/drm/vc4/vc4_plane.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)