| Message ID | 1398258932-2933-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 23, 2014 at 03:15:32PM +0200, Daniel Vetter wrote: > In Matt Ropers primary plane series a set of prep patches like > > commit af2b653bfb4ef40931b4d101ca842ce0c5da57ef > Author: Matt Roper <matthew.d.roper@intel.com> > Date: Tue Apr 1 15:22:32 2014 -0700 > > drm/i915: Restrict plane loops to only operate on overlay planes (v2) > > ensured that all exisiting users of the mode_config->plane_list > wouldn't change behaviour. Unfortunately tegra seems to have fallen > through the cracks. Fix it. > > This regression was introduced in > > commit e13161af80c185ecd8dc4641d0f5df58f9e3e0af > Author: Matt Roper <matthew.d.roper@intel.com> > Date: Tue Apr 1 15:22:38 2014 -0700 > > drm: Add drm_crtc_init_with_planes() (v2) > > The result was that we've unref'ed the fb for the primary plane twice, > leading to a use-after free bug. This is because the drm core will > already set crtc->primary->fb to NULL and do the unref for us, and the > crtc disable hook is called by the drm crtc helpers for exactly this > case. > > Aside: Now that the fbdev helpers clean up planes there's no longer a > need to do this in drivers. So this could probably be nuked entirely > in linux-next. > > Cc: Matt Roper <matthew.d.roper@intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Yep, this is definitely the right fix. I'm not sure how I missed this plane loop in cscope before. Reviewed-by: Matt Roper <matthew.d.roper@intel.com> > --- > drivers/gpu/drm/tegra/dc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c > index 36c717af6cf9..edb871d7d395 100644 > --- a/drivers/gpu/drm/tegra/dc.c > +++ b/drivers/gpu/drm/tegra/dc.c > @@ -312,7 +312,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc) > struct drm_device *drm = crtc->dev; > struct drm_plane *plane; > > - list_for_each_entry(plane, &drm->mode_config.plane_list, head) { > + drm_for_each_legacy_plane(plane, &drm->mode_config.plane_list) { > if (plane->crtc == crtc) { > tegra_plane_disable(plane); > plane->crtc = NULL; > -- > 1.9.2 >
diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c index 36c717af6cf9..edb871d7d395 100644 --- a/drivers/gpu/drm/tegra/dc.c +++ b/drivers/gpu/drm/tegra/dc.c @@ -312,7 +312,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc) struct drm_device *drm = crtc->dev; struct drm_plane *plane; - list_for_each_entry(plane, &drm->mode_config.plane_list, head) { + drm_for_each_legacy_plane(plane, &drm->mode_config.plane_list) { if (plane->crtc == crtc) { tegra_plane_disable(plane); plane->crtc = NULL;
In Matt Ropers primary plane series a set of prep patches like commit af2b653bfb4ef40931b4d101ca842ce0c5da57ef Author: Matt Roper <matthew.d.roper@intel.com> Date: Tue Apr 1 15:22:32 2014 -0700 drm/i915: Restrict plane loops to only operate on overlay planes (v2) ensured that all exisiting users of the mode_config->plane_list wouldn't change behaviour. Unfortunately tegra seems to have fallen through the cracks. Fix it. This regression was introduced in commit e13161af80c185ecd8dc4641d0f5df58f9e3e0af Author: Matt Roper <matthew.d.roper@intel.com> Date: Tue Apr 1 15:22:38 2014 -0700 drm: Add drm_crtc_init_with_planes() (v2) The result was that we've unref'ed the fb for the primary plane twice, leading to a use-after free bug. This is because the drm core will already set crtc->primary->fb to NULL and do the unref for us, and the crtc disable hook is called by the drm crtc helpers for exactly this case. Aside: Now that the fbdev helpers clean up planes there's no longer a need to do this in drivers. So this could probably be nuked entirely in linux-next. Cc: Matt Roper <matthew.d.roper@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/tegra/dc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)