From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v2 5/6] drm/crtc: add sanity checks to create_dumb() Date: Thu, 23 Jan 2014 15:55:16 +0200 Message-ID: <20140123135516.GV9454@intel.com> References: <1390245989-13280-6-git-send-email-dh.herrmann@gmail.com> <1390481595-512-1-git-send-email-dh.herrmann@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id C3116FBEE0 for ; Thu, 23 Jan 2014 05:55:21 -0800 (PST) Content-Disposition: inline In-Reply-To: <1390481595-512-1-git-send-email-dh.herrmann@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces@lists.freedesktop.org Errors-To: dri-devel-bounces@lists.freedesktop.org To: David Herrmann Cc: Daniel Vetter , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org On Thu, Jan 23, 2014 at 01:53:15PM +0100, David Herrmann wrote: > Lets make sure some basic expressions are always true: > bpp !=3D NULL > width !=3D NULL > height !=3D NULL > stride =3D bpp * width < 2^32 > size =3D stride * height < 2^32 > PAGE_ALIGN(size) < 2^32 > = > At least the udl driver doesn't check for multiplication-overflows, so > lets just make sure it will never happen. These checks allow drivers to do > any 32bit math without having to test for mult-overflows themselves. > = > The two divisions might hurt performance a bit, but dumb_create() is only > used for scanout-buffers, so that should be fine. We could use 64bit math > to avoid the divisions, but that may be slow on 32bit machines.. Or maybe > there should just be a "safe_mult32()" helper, which currently doesn't > exist (I think?). > = > Reviewed-by: Daniel Vetter > Signed-off-by: David Herrmann > --- > drivers/gpu/drm/drm_crtc.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > = > diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c > index 266a01d..2b50702 100644 > --- a/drivers/gpu/drm/drm_crtc.c > +++ b/drivers/gpu/drm/drm_crtc.c > @@ -3738,9 +3738,26 @@ int drm_mode_create_dumb_ioctl(struct drm_device *= dev, > void *data, struct drm_file *file_priv) > { > struct drm_mode_create_dumb *args =3D data; > + u32 cpp, stride, size; > = > if (!dev->driver->dumb_create) > return -ENOSYS; > + if (!args->width || !args->height || !args->bpp) > + return -EINVAL; > + > + /* overflow checks for 32bit size calculations */ > + cpp =3D DIV_ROUND_UP(args->bpp, 8); > + if (cpp > 0xffffffffU / args->width) > + return -EINVAL; IIRC I used -ERANGE for such things in some drm_framebuffer checks. Not sure if that's the best error value or not, but using the same one in both places would be nice. > + stride =3D cpp * args->width; > + if (args->height > 0xffffffffU / stride) > + return -EINVAL; > + > + /* test for wrap-around */ > + size =3D args->height * stride; > + if (PAGE_ALIGN(size) =3D=3D 0) > + return -EINVAL; > + > return dev->driver->dumb_create(file_priv, dev, args); > } > = > -- = > 1.8.5.3 > = > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel -- = Ville Syrj=E4l=E4 Intel OTC