linux-kernel - Re: [PATCH] drm: fix leak of uninitialized data to userspace

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 4 Nov 2008 14:23:26 -0800
From:	Greg KH <greg@...ah.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Vegard Nossum <vegardno@....uio.no>,
	Dave Airlie <airlied@...hat.com>,
	Sitsofe Wheeler <sitsofe@...oo.com>,
	Pekka Enberg <penberg@...helsinki.fi>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] drm: fix leak of uninitialized data to userspace

On Tue, Nov 04, 2008 at 11:17:30PM +0100, Ingo Molnar wrote:
> 
> * Greg KH <greg@...ah.com> wrote:
> 
> > On Fri, Oct 10, 2008 at 12:17:53PM +0200, Ingo Molnar wrote:
> > > 
> > > * Vegard Nossum <vegardno@....uio.no> wrote:
> > > 
> > > > >From f2e1569413900fa3ce5dad571e1a24d31307ab75 Mon Sep 17 00:00:00 2001
> > > > From: Vegard Nossum <vegardno@...in.ifi.uio.no>
> > > > 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 <sitsofe@...oo.com> 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:[<c020d283>] 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]  [<c020d6f3>] copy_to_user+0x43/0x60
> > > > > [  175.375214]  [<c028df28>] drm_getunique+0x38/0x50
> > > > > [  175.375224]  [<c028d329>] drm_ioctl+0x1b9/0x2f0
> > > > > [  175.375231]  [<c0188a07>] vfs_ioctl+0x67/0x70
> > > > > [  175.375239]  [<c0188a6c>] do_vfs_ioctl+0x5c/0x270
> > > > > [  175.375246]  [<c0188cbe>] sys_ioctl+0x3e/0x60
> > > > > [  175.375253]  [<c010336d>] sysenter_do_call+0x12/0x35
> > > > > [  175.375261]  [<ffffffff>] 0xffffffff
> > > > 
> > > > The hexdump decodes to:
> > > > 
> > > > vegardno@...in ~ $ ./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 <sitsofe@...oo.com>
> > > > Signed-off-by: Vegard Nossum <vegardno@...in.ifi.uio.no>
> > > 
> > > 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 :(
> 
> i'm not doing DRM patches - tip/out-of-tree is for pure out-of-tree 
> hotfixes.

So it goes no where?

Dave, any plan to pick this up and get it to Linus?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/