From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drivers/gpu/vga: use __GFP_NOWARN for user-controlled kmalloc Date: Thu, 4 Feb 2016 18:59:55 +0200 Message-ID: <20160204165955.GS23290@intel.com> References: <1454600989-123018-1-git-send-email-dvyukov@google.com> <20160204163244.GQ23290@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 137B76E0D4 for ; Thu, 4 Feb 2016 09:00:00 -0800 (PST) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Dmitry Vyukov Cc: LKML , dri-devel@lists.freedesktop.org, Kostya Serebryany , syzkaller , Alexander Potapenko List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBGZWIgMDQsIDIwMTYgYXQgMDU6Mzc6NDlQTSArMDEwMCwgRG1pdHJ5IFZ5dWtvdiB3 cm90ZToKPiBPbiBUaHUsIEZlYiA0LCAyMDE2IGF0IDU6MzIgUE0sIFZpbGxlIFN5cmrDpGzDpAo+ IDx2aWxsZS5zeXJqYWxhQGxpbnV4LmludGVsLmNvbT4gd3JvdGU6Cj4gPiBPbiBUaHUsIEZlYiAw NCwgMjAxNiBhdCAwNDo0OTo0OVBNICswMTAwLCBEbWl0cnkgVnl1a292IHdyb3RlOgo+ID4+IFNp emUgb2Yga21hbGxvYygpIGluIHZnYV9hcmJfd3JpdGUoKSBpcyBjb250cm9sbGVkIGJ5IHVzZXIu Cj4gPj4gVG9vIGxhcmdlIGttYWxsb2MoKSBzaXplIHRyaWdnZXJzIFdBUk5JTkcgbWVzc2FnZSBv biBjb25zb2xlLgo+ID4+Cj4gPj4gVXNlIEdGUF9VU0VSIHwgX19HRlBfTk9XQVJOIGZvciB0aGlz IGttYWxsb2MoKSB0byBub3Qgc2NhcmUgYWRtaW5zLgo+ID4+Cj4gPj4gU2lnbmVkLW9mZi1ieTog RG1pdHJ5IFZ5dWtvdiA8ZHZ5dWtvdkBnb29nbGUuY29tPgo+ID4+IC0tLQo+ID4+IEV4YW1wbGUg V0FSTklORzoKPiA+Pgo+ID4+IFdBUk5JTkc6IENQVTogMiBQSUQ6IDI5MzIyIGF0IG1tL3BhZ2Vf YWxsb2MuYzoyOTk5Cj4gPj4gX19hbGxvY19wYWdlc19ub2RlbWFzaysweDdkMi8weDE3NjAoKQo+ ID4+IE1vZHVsZXMgbGlua2VkIGluOgo+ID4+IENQVTogMiBQSUQ6IDI5MzIyIENvbW06IHN5ei1l eGVjdXRvciBUYWludGVkOiBHICAgIEIgIDQuNS4wLXJjMSsgIzI4Mwo+ID4+IEhhcmR3YXJlIG5h bWU6IFFFTVUgU3RhbmRhcmQgUEMgKGk0NDBGWCArIFBJSVgsIDE5OTYpLCBCSU9TIEJvY2hzIDAx LzAxLzIwMTEKPiA+PiAgMDAwMDAwMDBmZmZmZmZmZiBmZmZmODgwMDY5ZWZmNjcwIGZmZmZmZmZm ODI5OWEwNmQgMDAwMDAwMDAwMDAwMDAwMAo+ID4+ICBmZmZmODgwMDY1OGE0NzQwIGZmZmZmZmZm ODY0OTg1YTAgZmZmZjg4MDA2OWVmZjZiMCBmZmZmZmZmZjgxMzRmY2Y5Cj4gPj4gIGZmZmZmZmZm ODE2NmRlMzIgZmZmZmZmZmY4NjQ5ODVhMCAwMDAwMDAwMDAwMDAwYmI3IDAwMDAwMDAwMDI0MDQw YzAKPiA+PiBDYWxsIFRyYWNlOgo+ID4+ICBbPCAgICAgaW5saW5lICAgICA+XSBfX2R1bXBfc3Rh Y2sgbGliL2R1bXBfc3RhY2suYzoxNQo+ID4+ICBbPGZmZmZmZmZmODI5OWEwNmQ+XSBkdW1wX3N0 YWNrKzB4NmYvMHhhMiBsaWIvZHVtcF9zdGFjay5jOjUwCj4gPj4gIFs8ZmZmZmZmZmY4MTM0ZmNm OT5dIHdhcm5fc2xvd3BhdGhfY29tbW9uKzB4ZDkvMHgxNDAga2VybmVsL3BhbmljLmM6NDgyCj4g Pj4gIFs8ZmZmZmZmZmY4MTM0ZmYyOT5dIHdhcm5fc2xvd3BhdGhfbnVsbCsweDI5LzB4MzAga2Vy bmVsL3BhbmljLmM6NTE1Cj4gPj4gIFs8ICAgICBpbmxpbmUgICAgID5dIF9fYWxsb2NfcGFnZXNf c2xvd3BhdGggbW0vcGFnZV9hbGxvYy5jOjI5OTkKPiA+PiAgWzxmZmZmZmZmZjgxNjZkZTMyPl0g X19hbGxvY19wYWdlc19ub2RlbWFzaysweDdkMi8weDE3NjAgbW0vcGFnZV9hbGxvYy5jOjMyNTMK PiA+PiAgWzxmZmZmZmZmZjgxNzQ1Yzk5Pl0gYWxsb2NfcGFnZXNfY3VycmVudCsweGU5LzB4NDUw IG1tL21lbXBvbGljeS5jOjIwOTAKPiA+PiAgWzwgICAgIGlubGluZSAgICAgPl0gYWxsb2NfcGFn ZXMgaW5jbHVkZS9saW51eC9nZnAuaDo0NTkKPiA+PiAgWzxmZmZmZmZmZjgxNjY5YmI2Pl0gYWxs b2Nfa21lbV9wYWdlcysweDE2LzB4MTAwIG1tL3BhZ2VfYWxsb2MuYzozNDMzCj4gPj4gIFs8ZmZm ZmZmZmY4MTZjMjBhZj5dIGttYWxsb2Nfb3JkZXIrMHgxZi8weDgwIG1tL3NsYWJfY29tbW9uLmM6 MTAwOAo+ID4+ICBbPGZmZmZmZmZmODE2YzIxMmY+XSBrbWFsbG9jX29yZGVyX3RyYWNlKzB4MWYv MHgxNDAgbW0vc2xhYl9jb21tb24uYzoxMDE5Cj4gPj4gIFs8ICAgICBpbmxpbmUgICAgID5dIGtt YWxsb2NfbGFyZ2UgaW5jbHVkZS9saW51eC9zbGFiLmg6Mzk1Cj4gPj4gIFs8ZmZmZmZmZmY4MTc1 NmIyND5dIF9fa21hbGxvYysweDJmNC8weDM0MCBtbS9zbHViLmM6MzU1Nwo+ID4+ICBbPCAgICAg aW5saW5lICAgICA+XSBrbWFsbG9jIGluY2x1ZGUvbGludXgvc2xhYi5oOjQ2OAo+ID4+ICBbPGZm ZmZmZmZmODMyYzY1YTQ+XSB2Z2FfYXJiX3dyaXRlKzB4ZDQvMHhlNDAgZHJpdmVycy9ncHUvdmdh L3ZnYWFyYi5jOjkyNgo+ID4+ICBbPGZmZmZmZmZmODE3YTk4MzE+XSBkb19sb29wX3JlYWR2X3dy aXRldisweDE0MS8weDFlMCBmcy9yZWFkX3dyaXRlLmM6NzE5Cj4gPj4gIFs8ZmZmZmZmZmY4MTdh ZDY5OD5dIGRvX3JlYWR2X3dyaXRldisweDVmOC8weDZlMCBmcy9yZWFkX3dyaXRlLmM6ODQ5Cj4g Pj4gIFs8ZmZmZmZmZmY4MTdhZDhiNj5dIHZmc193cml0ZXYrMHg4Ni8weGMwIGZzL3JlYWRfd3Jp dGUuYzo4ODYKPiA+PiAgWzwgICAgIGlubGluZSAgICAgPl0gU1lTQ193cml0ZXYgZnMvcmVhZF93 cml0ZS5jOjkxOQo+ID4+ICBbPGZmZmZmZmZmODE3YjBhMjE+XSBTeVNfd3JpdGV2KzB4MTExLzB4 MmIwIGZzL3JlYWRfd3JpdGUuYzo5MTEKPiA+PiAgWzxmZmZmZmZmZjg2MzU5NjM2Pl0gZW50cnlf U1lTQ0FMTF82NF9mYXN0cGF0aCsweDE2LzB4N2EKPiA+PiBhcmNoL3g4Ni9lbnRyeS9lbnRyeV82 NC5TOjE4NQo+ID4+IC0tLQo+ID4+ICBkcml2ZXJzL2dwdS92Z2EvdmdhYXJiLmMgfCAyICstCj4g Pj4gIDEgZmlsZSBjaGFuZ2VkLCAxIGluc2VydGlvbigrKSwgMSBkZWxldGlvbigtKQo+ID4+Cj4g Pj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L3ZnYS92Z2FhcmIuYyBiL2RyaXZlcnMvZ3B1L3Zn YS92Z2FhcmIuYwo+ID4+IGluZGV4IGYxN2NiMDQuLmQ3M2I4NWIgMTAwNjQ0Cj4gPj4gLS0tIGEv ZHJpdmVycy9ncHUvdmdhL3ZnYWFyYi5jCj4gPj4gKysrIGIvZHJpdmVycy9ncHUvdmdhL3ZnYWFy Yi5jCj4gPj4gQEAgLTkyMyw3ICs5MjMsNyBAQCBzdGF0aWMgc3NpemVfdCB2Z2FfYXJiX3dyaXRl KHN0cnVjdCBmaWxlICpmaWxlLCBjb25zdCBjaGFyIF9fdXNlciAqYnVmLAo+ID4+ICAgICAgIGlu dCBpOwo+ID4+Cj4gPj4KPiA+PiAtICAgICBrYnVmID0ga21hbGxvYyhjb3VudCArIDEsIEdGUF9L RVJORUwpOwo+ID4+ICsgICAgIGtidWYgPSBrbWFsbG9jKGNvdW50ICsgMSwgR0ZQX1VTRVIgfCBf X0dGUF9OT1dBUk4pOwo+ID4KPiA+IEkgZG9uJ3QgcmVhbGx5IHNlZSB3aHkgaXQgZG9lcyB0aGlz IHVzZXIgY29udHJvbGxlZCBtYWxsb2MgaW4gdGhlCj4gPiBmaXJzdCBwbGFjZS4gVGhlIG1heCBs ZWd0aCBvZiB0aGUgc3RyaW5nIGl0IHdpbGwgYWN0dWFsbHkgaGFuZGxlIGxvb2tzCj4gPiB3ZWxs IGJvdW5kZWQsIHNvIGl0IGNvdWxkIGp1c3QgdXNlIHNvbWUgZml4ZWQgbGVuZ3RoIGJ1ZmZlciAo b24gc3RhY2sKPiA+IGV2ZW4pLgo+IAo+IAo+IFdoYXQgd291bGQgYmUgdGhlIHJpZ2h0IGxpbWl0 IG9uIGRhdGEgbGVuPwoKRnJvbSB0aGUgbG9va3Mgb2YgdGhpbmdzIHRoZSBsb25nZXN0IGNvbW1h bmQgY291bGQgYmUgdGhlCiJ0YXJnZXQgUENJOmRvbWFpbjpidXM6ZGV2LmZuIiB0aGluZy4gRXZl biBhc3N1bWluZyBzb21ldGhpbmcgc2lsbHkgbGlrZQpoYXZpbmcgMTAgY2hhcmFjdGVycyBmb3Ig ZWFjaCBkb21haW4sYnVzLGRldixmbiB0aGF0IHdvdWxkIHN0aWxsIGJlIG9ubHkKNTUgYnl0ZXMu IFNvIGJhc2VkIG9uIHRoYXQgZXZlbiBzb21ldGhpbmcgbGlrZSA2NCBieXRlcyBzaG91bGQgYmUg bW9yZQp0aGFuIGVub3VnaCBBRkFJQ1MuCgpUaGUgb3RoZXIgdGhpbmcgdGhhdCBzdHJpa2VzIG1l IGFzIGJpdCBvZGQgaW4gdGhpcyBjb2RlIGlzIHRoYXQgaXQKanVzdCBpZ25vcmVzIHdoYXRldmVy IGRhdGEgaXMgbGVmdCBvdmVyIGFmdGVyIGl0J3MgZG9uZSBwYXJzaW5nIHRoZQpzdHJpbmcuIEJ1 dCBpdCByZXR1cm5zIHRoZSBmdWxsIGNvdW50IHRvIHVzZXJzcGFjZSwgaW5kaWNhdGluZyBpdAph dGUgYWxsIG9mIGl0LiBJIGd1ZXNzIHRoYXQncyBmYWlybHkgc2FuZSB3aGVuIHVzZXJzcGFjZSBq dXN0IHVzZXMgYQpmaXhlZCBzaXplIGJ1ZmZlciBhbmQgY2hlY2tzIHRoYXQgdGhlIGtlcm5lbCBj b25zdW1lZCBpdCBhbGwuIEJ1dAptYXliZSB0aGVyZSBzaG91bGQgYmUgYW4gYWN0dWFsIGNoZWNr IHRvIHNlZSB0aGF0IHRoZXJlJ3MgYSAnXDAnCm9yIG1heWJlIDxhbnkgYW1vdW50IG9mIHdoaXRl c3BhY2U+KydcMCcgYWZ0ZXIgdGhlIHBhcnNlZCBzdHJpbmcuCgotLSAKVmlsbGUgU3lyasOkbMOk CkludGVsIE9UQwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f XwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcK aHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934208AbcBDRAD (ORCPT ); Thu, 4 Feb 2016 12:00:03 -0500 Received: from mga14.intel.com ([192.55.52.115]:38256 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934052AbcBDRAA (ORCPT ); Thu, 4 Feb 2016 12:00:00 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.22,396,1449561600"; d="scan'208";a="908590797" Date: Thu, 4 Feb 2016 18:59:55 +0200 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Dmitry Vyukov Cc: daniel@ffwll.ch, airlied@linux.ie, dri-devel@lists.freedesktop.org, Kostya Serebryany , syzkaller , Alexander Potapenko , LKML Subject: Re: [PATCH] drivers/gpu/vga: use __GFP_NOWARN for user-controlled kmalloc Message-ID: <20160204165955.GS23290@intel.com> References: <1454600989-123018-1-git-send-email-dvyukov@google.com> <20160204163244.GQ23290@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Feb 04, 2016 at 05:37:49PM +0100, Dmitry Vyukov wrote: > On Thu, Feb 4, 2016 at 5:32 PM, Ville Syrjälä > wrote: > > On Thu, Feb 04, 2016 at 04:49:49PM +0100, Dmitry Vyukov wrote: > >> Size of kmalloc() in vga_arb_write() is controlled by user. > >> Too large kmalloc() size triggers WARNING message on console. > >> > >> Use GFP_USER | __GFP_NOWARN for this kmalloc() to not scare admins. > >> > >> Signed-off-by: Dmitry Vyukov > >> --- > >> Example WARNING: > >> > >> WARNING: CPU: 2 PID: 29322 at mm/page_alloc.c:2999 > >> __alloc_pages_nodemask+0x7d2/0x1760() > >> Modules linked in: > >> CPU: 2 PID: 29322 Comm: syz-executor Tainted: G B 4.5.0-rc1+ #283 > >> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > >> 00000000ffffffff ffff880069eff670 ffffffff8299a06d 0000000000000000 > >> ffff8800658a4740 ffffffff864985a0 ffff880069eff6b0 ffffffff8134fcf9 > >> ffffffff8166de32 ffffffff864985a0 0000000000000bb7 00000000024040c0 > >> Call Trace: > >> [< inline >] __dump_stack lib/dump_stack.c:15 > >> [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50 > >> [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482 > >> [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515 > >> [< inline >] __alloc_pages_slowpath mm/page_alloc.c:2999 > >> [] __alloc_pages_nodemask+0x7d2/0x1760 mm/page_alloc.c:3253 > >> [] alloc_pages_current+0xe9/0x450 mm/mempolicy.c:2090 > >> [< inline >] alloc_pages include/linux/gfp.h:459 > >> [] alloc_kmem_pages+0x16/0x100 mm/page_alloc.c:3433 > >> [] kmalloc_order+0x1f/0x80 mm/slab_common.c:1008 > >> [] kmalloc_order_trace+0x1f/0x140 mm/slab_common.c:1019 > >> [< inline >] kmalloc_large include/linux/slab.h:395 > >> [] __kmalloc+0x2f4/0x340 mm/slub.c:3557 > >> [< inline >] kmalloc include/linux/slab.h:468 > >> [] vga_arb_write+0xd4/0xe40 drivers/gpu/vga/vgaarb.c:926 > >> [] do_loop_readv_writev+0x141/0x1e0 fs/read_write.c:719 > >> [] do_readv_writev+0x5f8/0x6e0 fs/read_write.c:849 > >> [] vfs_writev+0x86/0xc0 fs/read_write.c:886 > >> [< inline >] SYSC_writev fs/read_write.c:919 > >> [] SyS_writev+0x111/0x2b0 fs/read_write.c:911 > >> [] entry_SYSCALL_64_fastpath+0x16/0x7a > >> arch/x86/entry/entry_64.S:185 > >> --- > >> drivers/gpu/vga/vgaarb.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c > >> index f17cb04..d73b85b 100644 > >> --- a/drivers/gpu/vga/vgaarb.c > >> +++ b/drivers/gpu/vga/vgaarb.c > >> @@ -923,7 +923,7 @@ static ssize_t vga_arb_write(struct file *file, const char __user *buf, > >> int i; > >> > >> > >> - kbuf = kmalloc(count + 1, GFP_KERNEL); > >> + kbuf = kmalloc(count + 1, GFP_USER | __GFP_NOWARN); > > > > I don't really see why it does this user controlled malloc in the > > first place. The max legth of the string it will actually handle looks > > well bounded, so it could just use some fixed length buffer (on stack > > even). > > > What would be the right limit on data len? >>From the looks of things the longest command could be the "target PCI:domain:bus:dev.fn" thing. Even assuming something silly like having 10 characters for each domain,bus,dev,fn that would still be only 55 bytes. So based on that even something like 64 bytes should be more than enough AFAICS. The other thing that strikes me as bit odd in this code is that it just ignores whatever data is left over after it's done parsing the string. But it returns the full count to userspace, indicating it ate all of it. I guess that's fairly sane when userspace just uses a fixed size buffer and checks that the kernel consumed it all. But maybe there should be an actual check to see that there's a '\0' or maybe +'\0' after the parsed string. -- Ville Syrjälä Intel OTC