From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Date: Wed, 09 May 2018 08:23:07 +0000 Subject: Re: [PATCH] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl() Message-Id: <20180509082307.GS28661@phenom.ffwll.local> List-Id: References: <20180509072249.GA12754@mwanda> In-Reply-To: <20180509072249.GA12754@mwanda> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Dan Carpenter Cc: David Airlie , kernel-janitors@vger.kernel.org, dri-devel@lists.freedesktop.org On Wed, May 09, 2018 at 10:22:49AM +0300, Dan Carpenter wrote: > There is a comment here which says that DIV_ROUND_UP() and that's where > the problem comes from. Say you pick: > > args->bpp = UINT_MAX - 7; > args->width = 4; > args->height = 1; > > The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and > because of how we picked args->width that means cpp < UINT_MAX / 4. > > Signed-off-by: Dan Carpenter > --- > Btw, DIV_ROUND_UP() integer overflows have been a recurring source of > bugs so I have an unreleased static checker warning specific for that. > This line triggers three warnings for me on my unreleased code: > > drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: negative user subtract: 0-u32max - 1 > drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: potential integer overflow from user '(args->bpp) + (8)' > drivers/gpu/drm/drm_dumb_buffers.c:69 drm_mode_create_dumb_ioctl() warn: potential integer overflow in 'DIV_ROUND_UP' > > It's a pretty common idiom in the kernel to overflow and then test for > it later so I'm not able to release this code because of the number of > false positives that this idiom causes... > > diff --git a/drivers/gpu/drm/drm_dumb_buffers.c b/drivers/gpu/drm/drm_dumb_buffers.c > index 39ac15ce4702..45b0b5bbb5f8 100644 > --- a/drivers/gpu/drm/drm_dumb_buffers.c > +++ b/drivers/gpu/drm/drm_dumb_buffers.c > @@ -65,7 +65,8 @@ int drm_mode_create_dumb_ioctl(struct drm_device *dev, > return -EINVAL; > > /* overflow checks for 32bit size calculations */ > - /* NOTE: DIV_ROUND_UP() can overflow */ > + if (args->bpp > UINT_MAX - 8) > + return -EINVAL; > cpp = DIV_ROUND_UP(args->bpp, 8); > if (!cpp || cpp > 0xffffffffU / args->width) The !cpp check is now redundant, this was our minimal overflow check. Note that we only really care for cpp != 0 and that the size calculation doesn't overflow. Userspace specifying a completely bogus bpp value is ok otherwise (reasonable values only go up to about 128). So I think there's no security issue here. Anyway, can you pls respin with the !cpp check removed? See also commit 6a77e80e55cacace60ff03aa717a6d364a401d2b (HEAD -> stuff) Author: Daniel Vetter Date: Mon Apr 30 17:04:10 2018 +0200 backlight: remove obsolete comment for ->state for context. Thanks, Daniel > return -EINVAL; > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/dumb-buffers: Integer overflow in drm_mode_create_ioctl() Date: Wed, 9 May 2018 10:23:07 +0200 Message-ID: <20180509082307.GS28661@phenom.ffwll.local> References: <20180509072249.GA12754@mwanda> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-x243.google.com (mail-wm0-x243.google.com [IPv6:2a00:1450:400c:c09::243]) by gabe.freedesktop.org (Postfix) with ESMTPS id 81BAF6ECA5 for ; Wed, 9 May 2018 08:23:12 +0000 (UTC) Received: by mail-wm0-x243.google.com with SMTP id o78-v6so26096423wmg.0 for ; Wed, 09 May 2018 01:23:12 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20180509072249.GA12754@mwanda> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Dan Carpenter Cc: David Airlie , kernel-janitors@vger.kernel.org, dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org T24gV2VkLCBNYXkgMDksIDIwMTggYXQgMTA6MjI6NDlBTSArMDMwMCwgRGFuIENhcnBlbnRlciB3 cm90ZToKPiBUaGVyZSBpcyBhIGNvbW1lbnQgaGVyZSB3aGljaCBzYXlzIHRoYXQgRElWX1JPVU5E X1VQKCkgYW5kIHRoYXQncyB3aGVyZQo+IHRoZSBwcm9ibGVtIGNvbWVzIGZyb20uICBTYXkgeW91 IHBpY2s6Cj4gCj4gCWFyZ3MtPmJwcCA9IFVJTlRfTUFYIC0gNzsKPiAJYXJncy0+d2lkdGggPSA0 Owo+IAlhcmdzLT5oZWlnaHQgPSAxOwo+IAo+IFRoZSBpbnRlZ2VyIG92ZXJmbG93IGluIERJVl9S T1VORF9VUCgpIG1lYW5zICJjcHAiIGlzIFVJTlRfTUFYIC8gOCBhbmQKPiBiZWNhdXNlIG9mIGhv dyB3ZSBwaWNrZWQgYXJncy0+d2lkdGggdGhhdCBtZWFucyBjcHAgPCBVSU5UX01BWCAvIDQuCj4g Cj4gU2lnbmVkLW9mZi1ieTogRGFuIENhcnBlbnRlciA8ZGFuLmNhcnBlbnRlckBvcmFjbGUuY29t Pgo+IC0tLQo+IEJ0dywgRElWX1JPVU5EX1VQKCkgaW50ZWdlciBvdmVyZmxvd3MgaGF2ZSBiZWVu IGEgcmVjdXJyaW5nIHNvdXJjZSBvZgo+IGJ1Z3Mgc28gSSBoYXZlIGFuIHVucmVsZWFzZWQgc3Rh dGljIGNoZWNrZXIgd2FybmluZyBzcGVjaWZpYyBmb3IgdGhhdC4KPiBUaGlzIGxpbmUgdHJpZ2dl cnMgdGhyZWUgd2FybmluZ3MgZm9yIG1lIG9uIG15IHVucmVsZWFzZWQgY29kZToKPiAKPiBkcml2 ZXJzL2dwdS9kcm0vZHJtX2R1bWJfYnVmZmVycy5jOjY5IGRybV9tb2RlX2NyZWF0ZV9kdW1iX2lv Y3RsKCkgd2FybjogbmVnYXRpdmUgdXNlciBzdWJ0cmFjdDogMC11MzJtYXggLSAxCj4gZHJpdmVy cy9ncHUvZHJtL2RybV9kdW1iX2J1ZmZlcnMuYzo2OSBkcm1fbW9kZV9jcmVhdGVfZHVtYl9pb2N0 bCgpIHdhcm46IHBvdGVudGlhbCBpbnRlZ2VyIG92ZXJmbG93IGZyb20gdXNlciAnKGFyZ3MtPmJw cCkgKyAoOCknCj4gZHJpdmVycy9ncHUvZHJtL2RybV9kdW1iX2J1ZmZlcnMuYzo2OSBkcm1fbW9k ZV9jcmVhdGVfZHVtYl9pb2N0bCgpIHdhcm46IHBvdGVudGlhbCBpbnRlZ2VyIG92ZXJmbG93IGlu ICdESVZfUk9VTkRfVVAnCj4gCj4gSXQncyBhIHByZXR0eSBjb21tb24gaWRpb20gaW4gdGhlIGtl cm5lbCB0byBvdmVyZmxvdyBhbmQgdGhlbiB0ZXN0IGZvcgo+IGl0IGxhdGVyIHNvIEknbSBub3Qg YWJsZSB0byByZWxlYXNlIHRoaXMgY29kZSBiZWNhdXNlIG9mIHRoZSBudW1iZXIgb2YKPiBmYWxz ZSBwb3NpdGl2ZXMgdGhhdCB0aGlzIGlkaW9tIGNhdXNlcy4uLgo+IAo+IGRpZmYgLS1naXQgYS9k cml2ZXJzL2dwdS9kcm0vZHJtX2R1bWJfYnVmZmVycy5jIGIvZHJpdmVycy9ncHUvZHJtL2RybV9k dW1iX2J1ZmZlcnMuYwo+IGluZGV4IDM5YWMxNWNlNDcwMi4uNDViMGI1YmJiNWY4IDEwMDY0NAo+ IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fZHVtYl9idWZmZXJzLmMKPiArKysgYi9kcml2ZXJz L2dwdS9kcm0vZHJtX2R1bWJfYnVmZmVycy5jCj4gQEAgLTY1LDcgKzY1LDggQEAgaW50IGRybV9t b2RlX2NyZWF0ZV9kdW1iX2lvY3RsKHN0cnVjdCBkcm1fZGV2aWNlICpkZXYsCj4gIAkJcmV0dXJu IC1FSU5WQUw7Cj4gIAo+ICAJLyogb3ZlcmZsb3cgY2hlY2tzIGZvciAzMmJpdCBzaXplIGNhbGN1 bGF0aW9ucyAqLwo+IC0JLyogTk9URTogRElWX1JPVU5EX1VQKCkgY2FuIG92ZXJmbG93ICovCj4g KwlpZiAoYXJncy0+YnBwID4gVUlOVF9NQVggLSA4KQo+ICsJCXJldHVybiAtRUlOVkFMOwo+ICAJ Y3BwID0gRElWX1JPVU5EX1VQKGFyZ3MtPmJwcCwgOCk7Cj4gIAlpZiAoIWNwcCB8fCBjcHAgPiAw eGZmZmZmZmZmVSAvIGFyZ3MtPndpZHRoKQoKVGhlICFjcHAgY2hlY2sgaXMgbm93IHJlZHVuZGFu dCwgdGhpcyB3YXMgb3VyIG1pbmltYWwgb3ZlcmZsb3cgY2hlY2suIE5vdGUKdGhhdCB3ZSBvbmx5 IHJlYWxseSBjYXJlIGZvciBjcHAgIT0gMCBhbmQgdGhhdCB0aGUgc2l6ZSBjYWxjdWxhdGlvbiBk b2Vzbid0Cm92ZXJmbG93LiBVc2Vyc3BhY2Ugc3BlY2lmeWluZyBhIGNvbXBsZXRlbHkgYm9ndXMg YnBwIHZhbHVlIGlzIG9rCm90aGVyd2lzZSAocmVhc29uYWJsZSB2YWx1ZXMgb25seSBnbyB1cCB0 byBhYm91dCAxMjgpLiBTbyBJIHRoaW5rIHRoZXJlJ3MKbm8gc2VjdXJpdHkgaXNzdWUgaGVyZS4K CkFueXdheSwgY2FuIHlvdSBwbHMgcmVzcGluIHdpdGggdGhlICFjcHAgY2hlY2sgcmVtb3ZlZD8g U2VlIGFsc28KCmNvbW1pdCA2YTc3ZTgwZTU1Y2FjYWNlNjBmZjAzYWE3MTdhNmQzNjRhNDAxZDJi IChIRUFEIC0+IHN0dWZmKQpBdXRob3I6IERhbmllbCBWZXR0ZXIgPGRhbmllbC52ZXR0ZXJAZmZ3 bGwuY2g+CkRhdGU6ICAgTW9uIEFwciAzMCAxNzowNDoxMCAyMDE4ICswMjAwCgogICAgYmFja2xp Z2h0OiByZW1vdmUgb2Jzb2xldGUgY29tbWVudCBmb3IgLT5zdGF0ZQoKZm9yIGNvbnRleHQuCgpU aGFua3MsIERhbmllbAoKPiAgCQlyZXR1cm4gLUVJTlZBTDsKPiBfX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fXwo+IGRyaS1kZXZlbCBtYWlsaW5nIGxpc3QKPiBk cmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCj4gaHR0cHM6Ly9saXN0cy5mcmVlZGVza3Rv cC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwKCi0tIApEYW5pZWwgVmV0dGVyClNvZnR3 YXJlIEVuZ2luZWVyLCBJbnRlbCBDb3Jwb3JhdGlvbgpodHRwOi8vYmxvZy5mZndsbC5jaApfX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFp bGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5m cmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK