[PATCH 2/3] drm: Be more paranoid with integer overflows

Ville Syrjälä ville.syrjala at linux.intel.com
Thu May 24 11:54:37 PDT 2012


On Thu, May 24, 2012 at 08:30:23PM +0200, Daniel Vetter wrote:
> On Thu, May 24, 2012 at 08:53:59PM +0300, ville.syrjala at linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > 
> > Make sure 'width * cpp' and 'height * pitch + offset' don't exceed
> > UINT_MAX.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_crtc.c |   10 +++++++++-
> >  1 files changed, 9 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> > index 80a34e7..e1b53fb 100644
> > --- a/drivers/gpu/drm/drm_crtc.c
> > +++ b/drivers/gpu/drm/drm_crtc.c
> > @@ -2211,13 +2211,21 @@ static int framebuffer_check(struct drm_mode_fb_cmd2 *r)
> >  
> >  	for (i = 0; i < num_planes; i++) {
> >  		unsigned int width = r->width / (i != 0 ? hsub : 1);
> > +		unsigned int height = r->height / (i != 0 ? vsub : 1);
> > +		unsigned int cpp = drm_format_plane_cpp(r->pixel_format, i);
> >  
> >  		if (!r->handles[i]) {
> >  			DRM_DEBUG_KMS("no buffer object handle for plane %d\n", i);
> >  			return -EINVAL;
> >  		}
> >  
> > -		if (r->pitches[i] < drm_format_plane_cpp(r->pixel_format, i) * width) {
> > +		if ((uint64_t) width * cpp > UINT_MAX)
> > +			return -ERANGE;
> > +
> 
> iirc that blows up on 32bit because gcc likes to use a compiler built-in.

I think that problem only happens w/ 64bit divs, which is why you have do_div()
and friends. At least with a small test app 'gcc -O2 -m32' generates the obvious
mul+cmp code, and mul+add+adc+cmp for the case w/ offsets[i] added. Maybe other
archs can't do it so neatly though.

> And the usual pattern I've seen is if (UINT_MAX / a < b) return -ERANGE;

I'm not a fan of divs :)

-- 
Ville Syrjälä
Intel OTC


More information about the dri-devel mailing list