Hi
On Thu, Jan 23, 2014 at 2:55 PM, Ville Syrjälä ville.syrjala@linux.intel.com wrote:
On Thu, Jan 23, 2014 at 01:53:15PM +0100, David Herrmann wrote:
Lets make sure some basic expressions are always true: bpp != NULL width != NULL height != NULL stride = bpp * width < 2^32 size = 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 daniel.vetter@ffwll.ch Signed-off-by: David Herrmann dh.herrmann@gmail.com
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 = 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 = 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.
I'm actually a fan of EINVAL here, as this is really more about "do the arguments make sense?" than "does the range exceed the driver's capabilities?" But what do I know.. So more comments on that are welcome. And btw., we already mix EINVAL and ERANGE so this would just contribute to the already existing mess (see the stride verifications which usually return EINVAL, but the overflow checks often return ERANGE).
Thanks David
stride = cpp * args->width;if (args->height > 0xffffffffU / stride)return -EINVAL;/* test for wrap-around */size = args->height * stride;if (PAGE_ALIGN(size) == 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älä Intel OTC