[Intel-gfx] [PATCH] drm: check for garbage in unused addfb2 fields

[Daniel Vetter]

On Tue, May 26, 2015 at 02:38:44PM +0200, Thierry Reding wrote:
> On Wed, May 20, 2015 at 04:53:53PM +0200, Daniel Vetter wrote:
> > Unfortunately old userspace didn't clear this properly, but since
> > we've added fb modifiers that's fixed. Checking properly that unused
> > fields is important for abi extensions, and just right now there's a
> > bunch of discussions going on about how exactly the additional aux
> > planes for render compression should be specified.
> > 
> > So let's first make sure that the values in those additional fields
> > can be indeed used.
> > 
> > Cc: Thierry Reding <thierry.reding at gmail.com>
> > Testcase: igt/kms_addfb/unused-*
> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > ---
> >  drivers/gpu/drm/drm_crtc.c | 26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> 
> I think this is fine for now. There's still some unclarity about how
> exactly to pass in auxiliary planes, but in order to do so we need to
> prevent userspace from passing in garbage, so:
> 
> Reviewed-by: Thierry Reding <treding at nvidia.com>

Thanks for the review, patch merged.
-Daniel

> 
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 2e26988a9762..8ad582f4026b 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -3308,6 +3308,32 @@ static int framebuffer_check(const struct drm_mode_fb_cmd2 *r)
> >  		}
> >  	}
> >  
> > +	for (i = num_planes; i < 4; i++) {
> > +		if (r->modifier[i]) {
> > +			DRM_DEBUG_KMS("non-zero modifier for unused plane %d\n", i);
> > +			return -EINVAL;
> > +		}
> > +
> > +		/* Pre-FB_MODIFIERS userspace didn't clear the structs properly. */
> > +		if (!(r->flags & DRM_MODE_FB_MODIFIERS))
> > +			continue;
> > +
> > +		if (r->handles[i]) {
> > +			DRM_DEBUG_KMS("buffer object handle for unused plane %d\n", i);
> > +			return -EINVAL;
> > +		}
> > +
> > +		if (r->pitches[i]) {
> > +			DRM_DEBUG_KMS("non-zero pitch for unused plane %d\n", i);
> > +			return -EINVAL;
> > +		}
> > +
> > +		if (r->offsets[i]) {
> > +			DRM_DEBUG_KMS("non-zero offset for unused plane %d\n", i);
> > +			return -EINVAL;
> > +		}
> > +	}
> > +
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.1.4
> > 



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