From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756420AbYKDV61 (ORCPT ); Tue, 4 Nov 2008 16:58:27 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754230AbYKDV6Q (ORCPT ); Tue, 4 Nov 2008 16:58:16 -0500 Received: from kroah.org ([198.145.64.141]:54977 "EHLO coco.kroah.org" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1753975AbYKDV6O (ORCPT ); Tue, 4 Nov 2008 16:58:14 -0500 Date: Tue, 4 Nov 2008 13:38:59 -0800 From: Greg KH To: Ingo Molnar Cc: Vegard Nossum , Dave Airlie , Sitsofe Wheeler , Pekka Enberg , linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm: fix leak of uninitialized data to userspace Message-ID: <20081104213859.GA3516@kroah.com> References: <20081010100200.GA752@thuin.ifi.uio.no> <20081010101753.GE15139@elte.hu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20081010101753.GE15139@elte.hu> User-Agent: Mutt/1.5.16 (2007-06-09) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 10, 2008 at 12:17:53PM +0200, Ingo Molnar wrote: > > * Vegard Nossum wrote: > > > >From f2e1569413900fa3ce5dad571e1a24d31307ab75 Mon Sep 17 00:00:00 2001 > > From: Vegard Nossum > > Date: Fri, 10 Oct 2008 11:50:57 +0200 > > Subject: [PATCH] drm: fix leak of uninitialized data to userspace > > > > On Fri, Oct 10, 2008 at 10:54 AM, Sitsofe Wheeler wrote: > > > > > > [ 175.375036] WARNING: kmemcheck: Caught 32-bit read from uninitialized memory (f65d2294) > > > [ 175.375049] 7063693a303030303a30303a30322e3000a76c080800000000a76c080e000000 > > > [ 175.375096] i i i i i i i i i i i i i i i i i u u u u u u u u u u u u u u u > > > [ 175.375137] ^ > > > [ 175.375142] > > > [ 175.375148] Pid: 2288, comm: Xorg Not tainted (2.6.27-tipskw-00069-g37cb0b7-dirty #81) 900 > > > [ 175.375155] EIP: 0060:[] EFLAGS: 00003246 CPU: 0 > > > [ 175.375169] EIP is at __copy_to_user_ll+0x43/0x60 > > > [ 175.375174] EAX: 00000000 EBX: 00000028 ECX: 00000005 EDX: f65d2280 > > > [ 175.375180] ESI: f65d2294 EDI: 086cb6b4 EBP: f631bef4 ESP: c055bd68 > > > [ 175.375186] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 0068 > > > [ 175.375191] CR0: 8005003b CR2: f67c1c44 CR3: 36368000 CR4: 000006c0 > > > [ 175.375197] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000 > > > [ 175.375202] DR6: ffff4ff0 DR7: 00000400 > > > [ 175.375206] [] copy_to_user+0x43/0x60 > > > [ 175.375214] [] drm_getunique+0x38/0x50 > > > [ 175.375224] [] drm_ioctl+0x1b9/0x2f0 > > > [ 175.375231] [] vfs_ioctl+0x67/0x70 > > > [ 175.375239] [] do_vfs_ioctl+0x5c/0x270 > > > [ 175.375246] [] sys_ioctl+0x3e/0x60 > > > [ 175.375253] [] sysenter_do_call+0x12/0x35 > > > [ 175.375261] [] 0xffffffff > > > > The hexdump decodes to: > > > > vegardno@thuin ~ $ ./a.out 7063693a303030303a30303a30322e3000a76c080800000000a76c080e000000 > > pci:0000:00:02.0<0><167>l<8><8><0><0><0><0><167>l<8><14><0><0><0> > > > > ...so drm_getunique() is trying to copy some uninitialized data to > > userspace. The ECX register contains the number of words that are > > left to copy -- so there are 5 * 4 = 20 bytes left. The offset of the > > first uninitialized byte (counting from the start of the string) is > > also 20 (i.e. 0xf65d2294&((1 << 5)-1) == 20). So somebody tried to > > copy 40 bytes when the string was only 19 long. > > > > In drm_set_busid() we have this code: > > > > dev->unique_len = 40; > > dev->unique = drm_alloc(dev->unique_len + 1, DRM_MEM_DRIVER); > > ... > > len = snprintf(dev->unique, dev->unique_len, "pci:%04x:%02x:%02x.%d", > > > > ...so it seems that dev->unique is never updated to reflect the > > actual length of the string. The remaining bytes (20 in this case) > > are random uninitialized bytes that are copied into userspace. > > > > This patch fixes the problem by setting dev->unique_len after the > > snprintf(). > > > > Completely untested. > > > > Reported-by: Sitsofe Wheeler > > Signed-off-by: Vegard Nossum > > i've stuck it into the tip/out-of-tree quick-fixes branch. > > Sitsofe, could you please check very latest tip/master with > CONFIG_KMEMCHECK=y, does it find any other uninitialized memory access? What ever happened to this patch? Did it go into Linus's tree? If so, I can't seem to find it :( thanks, greg k-h