[PATCH] drm/tegra: restrict plane loops to legacy planes

[PATCH] drm/tegra: restrict plane loops to legacy planes

[Daniel Vetter]

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

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

Re: [PATCH] drm/tegra: restrict plane loops to legacy planes

[Matt Roper]

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
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795