From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matt Roper Subject: Re: [PATCH] drm/tegra: restrict plane loops to legacy planes Date: Wed, 23 Apr 2014 07:48:45 -0700 Message-ID: <20140423144845.GW1063@intel.com> References: <1398258932-2933-1-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 4F5BC6EACB for ; Wed, 23 Apr 2014 07:45:55 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1398258932-2933-1-git-send-email-daniel.vetter@ffwll.ch> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: DRI Development List-Id: dri-devel@lists.freedesktop.org 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 > 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 > 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 > Signed-off-by: Daniel Vetter Yep, this is definitely the right fix. I'm not sure how I missed this plane loop in cscope before. Reviewed-by: Matt Roper > --- > 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 > -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795