[Intel-gfx] [PATCH v2] drm: Release reference from blob lookup after replacing property

[Daniel Vetter]

On Wed, Oct 26, 2016 at 10:48:07AM +0300, Ville Syrjälä wrote:
> On Tue, Oct 25, 2016 at 10:28:08PM +0100, Chris Wilson wrote:
> > From: Felix Monninger <felix.monninger at gmail.com>
> > 
> > drm_property_lookup_blob() returns a reference to the returned blob, and
> > drm_atomic_replace_property_blob() takes a references to the blob it
> > stores, so afterwards we are left owning a reference to the new_blob that
> > we never release, and thus leak memory every time we update a property
> > such as during drm_atomic_helper_legacy_gamma_set().
> > 
> > v2: update credentials, drm_property_unreference_blob() is NULL safe and
> > NULL is passed consistently to it throughout drm_atomic.c so do so here.
> > 
> > Reported-by: Felix Monninger <felix.monninger at gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98420
> > Signed-off-by: Felix Monninger <felix.monninger at gmail.com>
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> 
> Cc: stable at vger.kernel.org
> Fixes: 5488dc16fde7 ("drm: introduce pipe color correction properties")
> Reviewed-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> The single return path bikeshed looked OK to me too, so my r-b can
> be applied there as well, if people prefer that version.

Applied to drm-misc-fixes (yes I'm trying out something new).
-Daniel

> 
> > ---
> >  drivers/gpu/drm/drm_atomic.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 1b5a32df9a9a..e0760c138355 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -416,18 +416,21 @@ drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
> >  					 ssize_t expected_size,
> >  					 bool *replaced)
> >  {
> > -	struct drm_device *dev = crtc->dev;
> >  	struct drm_property_blob *new_blob = NULL;
> >  
> >  	if (blob_id != 0) {
> > -		new_blob = drm_property_lookup_blob(dev, blob_id);
> > +		new_blob = drm_property_lookup_blob(crtc->dev, blob_id);
> >  		if (new_blob == NULL)
> >  			return -EINVAL;
> > -		if (expected_size > 0 && expected_size != new_blob->length)
> > +
> > +		if (expected_size > 0 && expected_size != new_blob->length) {
> > +			drm_property_unreference_blob(new_blob);
> >  			return -EINVAL;
> > +		}
> >  	}
> >  
> >  	drm_atomic_replace_property_blob(blob, new_blob, replaced);
> > +	drm_property_unreference_blob(new_blob);
> >  
> >  	return 0;
> >  }
> > -- 
> > 2.10.1
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch