linux-kernel - Re: [ima_inode_post_setattr] kernel BUG at mm/slub.c:3479!

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]

Message-ID: <alpine.DEB.2.00.1210171957380.32271@chino.kir.corp.google.com>
Date:	Wed, 17 Oct 2012 20:07:45 -0700 (PDT)
From:	David Rientjes <rientjes@...gle.com>
To:	Fengguang Wu <fengguang.wu@...el.com>,
	Hugh Dickins <hughd@...gle.com>, Li Zefan <lizefan@...wei.com>,
	Aristeu Rozanski <aris@...hat.com>, Tejun Heo <tj@...nel.org>
cc:	Mimi Zohar <zohar@...ux.vnet.ibm.com>,
	James Morris <jmorris@...ei.org>,
	Dmitry Kasatkin <dmitry.kasatkin@...el.com>,
	linux-security-module@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Subject: Re: [ima_inode_post_setattr] kernel BUG at mm/slub.c:3479!

On Thu, 18 Oct 2012, Fengguang Wu wrote:

> Mimi,
> 
> Although this occurs in the xen tree head, it's more likely related to
> ima_inode_post_setattr().
> 
> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git devel/for-linus-3.8
> head:   d01b193de1a961e028ee6df5ad0b34c3e56a995f
> commit: d01b193de1a961e028ee6df5ad0b34c3e56a995f [0/0] xen/acpi: Support ACPI hotplug of CPUs.
> 
> [  115.499469] Write protecting the kernel read-only data: 2752k
> [  134.707390] ------------[ cut here ]------------
> [  134.707390] kernel BUG at /c/kernel-tests/src/linux/mm/slub.c:3479!
> [  134.707390] invalid opcode: 0000 [#1] 
> [  134.707390] Pid: 1, comm: init Not tainted 3.6.0-00451-gd01b193 #2 Bochs Bochs
> [  134.707390] EIP: 0060:[<c1235dbe>] EFLAGS: 00000246 CPU: 0
> [  134.707390] EIP is at kfree+0x2be/0x490
> [  134.707390] EAX: 00000001 EBX: 00000001 ECX: 00000000 EDX: 00000000
> [  134.707390] ESI: cdeef760 EDI: cd83bdac EBP: cd83bd70 ESP: cd83bd50
> [  134.707390]  DS: 007b ES: 007b FS: 0000 GS: 00e0 SS: 0068
> [  134.707390] CR0: 80050033 CR2: b7782000 CR3: 0f9a8000 CR4: 00000690
> [  134.707390] DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> [  134.707390] DR6: 00000000 DR7: 00000000
> [  134.707390] Process init (pid: 1, ti=cd83a000 task=cd838000 task.ti=cd83a000)
> [  134.707390] Stack:
> [  134.707390]  00000000 cd83bd5c c1085b74 cd83bd88 c1298a5e cd83a000 ffffffc3 cd83bd90
> [  134.707390]  cd83bd88 c1298a5e cd83bd90 c1ffdf63 00000001 cd832914 cd83bd98 c1298d4a
> [  134.707390]  00000000 00000002 cd83bdac c11e0400 c995c980 cd832914 00000000 cd83bdc4
> [  134.707390] Call Trace:
> [  134.707390]  [<c1085b74>] ? irq_exit+0x74/0x180
> [  134.707390]  [<c1298a5e>] ? __simple_xattr_set+0x28e/0x360
> [  134.707390]  [<c1298a5e>] __simple_xattr_set+0x28e/0x360
> [  134.707390]  [<c1298d4a>] simple_xattr_remove+0x2a/0x40
> [  134.707390]  [<c11e0400>] shmem_removexattr+0xc0/0x120
> [  134.707390]  [<c14f11ed>] ima_inode_post_setattr+0x21d/0x250

This is a scary one because it involves kfree() of memory that is not 
allocated through the slab allocator, and not bypassed because its too 
large or otherwise we would have PageCompound() set.

Examining __simple_xattr_set(), though, we know flags has XATTR_REPLACE 
set and value == NULL since it's being called from simple_xattr_remove().  
That means the local new_xattr variable is initialized to itself, which is 
just going to be anything that happens to be on the stack.  If the name 
passed to simple_xattr_remove() isn't found in the list of xattrs, then 
this will call kfree(xattr) which was never set to anything else.

This was changed in commit 38f38657444d ("xattr: extract simple_xattr code 
from tmpfs") and the old code actually properly initialized new_xattr to 
NULL in which case this would simply fail with -ENODATA instead of causing 
the BUG().

So while it's probably not advised to call simple_xattr_remove() for a 
name that does not exist in the xattr list, this seems to be as a result 
of that commit rather than anything from ima_inode_post_setattr().

Let's add Aristeu Rozanski, Li, Hugh, and Tejun to the cc list.
--
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/