* next-20090220: XFS, IMA: BUG: sleeping function called from invalid context at mm/slub.c:1613
@ 2009-02-20 11:00 Alexander Beregalov
2009-02-20 20:22 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Alexander Beregalov @ 2009-02-20 11:00 UTC (permalink / raw)
To: LKML, linux-next, xfs, Mimi Zohar
Hi
I have applied the following patch from Mimi Zohar
http://marc.info/?l=linux-next&m=123509665514552
That is why it is dirty.
The kernel can not boot without it when IMA is enabled.
BUG: sleeping function called from invalid context at mm/slub.c:1613
in_atomic(): 1, irqs_disabled(): 0, pid: 1514, name: mkdir
3 locks held by mkdir/1514:
#0: (&type->i_mutex_dir_key#2/1){+.+.+.}, at: [<ffffffff802d3460>]
lookup_create+0x30/0xd0
#1: (&(&ip->i_lock)->mr_lock/1){+.+.+.}, at: [<ffffffff803ca77f>]
xfs_ilock+0xdf/0x120
#2: (&pag->pag_ici_lock){++++.+}, at: [<ffffffff803caed6>]
xfs_iget+0x156/0x650
Pid: 1514, comm: mkdir Not tainted 2.6.29-rc5-next-20090220-dirty #1
Call Trace:
[<ffffffff8026ad33>] ? __debug_show_held_locks+0x13/0x30
[<ffffffff80234fe5>] __might_sleep+0x105/0x140
[<ffffffff802c6191>] kmem_cache_alloc+0xd1/0x100
[<ffffffff8045fb29>] ima_iint_insert+0x49/0xf0
[<ffffffff8045fbed>] ima_inode_alloc+0x1d/0x30
[<ffffffff802dfa5f>] inode_init_always+0xaf/0x250
[<ffffffff803caf86>] xfs_iget+0x206/0x650
[<ffffffff803eb8e8>] xfs_trans_iget+0x208/0x250
[<ffffffff803ce571>] xfs_ialloc+0xc1/0x700
[<ffffffff803ec6f9>] xfs_dir_ialloc+0xa9/0x340
[<ffffffff8025e009>] ? down_write_nested+0x79/0x90
[<ffffffff803ef0c1>] xfs_create+0x3e1/0x690
[<ffffffff803fb813>] xfs_vn_mknod+0x63/0xf0
[<ffffffff803fb8ae>] xfs_vn_mkdir+0xe/0x10
[<ffffffff802d487c>] vfs_mkdir+0x8c/0xd0
[<ffffffff802d6a56>] sys_mkdirat+0x106/0x120
[<ffffffff8020bc0c>] ? sysret_check+0x27/0x62
[<ffffffff8026c79d>] ? trace_hardirqs_on_caller+0x17d/0x1e0
[<ffffffff802d6a83>] sys_mkdir+0x13/0x20
[<ffffffff8020bbdb>] system_call_fastpath+0x16/0x1b
BUG: sleeping function called from invalid context at mm/slub.c:1613
in_atomic(): 1, irqs_disabled(): 0, pid: 1514, name: mkdir
3 locks held by mkdir/1514:
#0: (&type->i_mutex_dir_key#2/1){+.+.+.}, at: [<ffffffff802d3460>]
lookup_create+0x30/0xd0
#1: (&(&ip->i_lock)->mr_lock/1){+.+.+.}, at: [<ffffffff803ca77f>]
xfs_ilock+0xdf/0x120
#2: (&pag->pag_ici_lock){++++.+}, at: [<ffffffff803caed6>]
xfs_iget+0x156/0x650
Pid: 1514, comm: mkdir Not tainted 2.6.29-rc5-next-20090220-dirty #1
Call Trace:
[<ffffffff8026ad33>] ? __debug_show_held_locks+0x13/0x30
[<ffffffff80234fe5>] __might_sleep+0x105/0x140
[<ffffffff802c6191>] kmem_cache_alloc+0xd1/0x100
[<ffffffff8048131a>] radix_tree_preload+0x6a/0xf0
[<ffffffff8045fb3b>] ima_iint_insert+0x5b/0xf0
[<ffffffff8045fbed>] ima_inode_alloc+0x1d/0x30
[<ffffffff802dfa5f>] inode_init_always+0xaf/0x250
[<ffffffff803caf86>] xfs_iget+0x206/0x650
[<ffffffff803eb8e8>] xfs_trans_iget+0x208/0x250
[<ffffffff803ce571>] xfs_ialloc+0xc1/0x700
[<ffffffff803ec6f9>] xfs_dir_ialloc+0xa9/0x340
[<ffffffff8025e009>] ? down_write_nested+0x79/0x90
[<ffffffff803ef0c1>] xfs_create+0x3e1/0x690
[<ffffffff803fb813>] xfs_vn_mknod+0x63/0xf0
[<ffffffff803fb8ae>] xfs_vn_mkdir+0xe/0x10
[<ffffffff802d487c>] vfs_mkdir+0x8c/0xd0
[<ffffffff802d6a56>] sys_mkdirat+0x106/0x120
[<ffffffff8020bc0c>] ? sysret_check+0x27/0x62
[<ffffffff8026c79d>] ? trace_hardirqs_on_caller+0x17d/0x1e0
[<ffffffff802d6a83>] sys_mkdir+0x13/0x20
[<ffffffff8020bbdb>] system_call_fastpath+0x16/0x1b
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: next-20090220: XFS, IMA: BUG: sleeping function called from invalid context at mm/slub.c:1613
2009-02-20 11:00 next-20090220: XFS, IMA: BUG: sleeping function called from invalid context at mm/slub.c:1613 Alexander Beregalov
@ 2009-02-20 20:22 ` Andrew Morton
2009-02-20 22:16 ` Mimi Zohar
2009-02-24 22:39 ` Mimi Zohar
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2009-02-20 20:22 UTC (permalink / raw)
To: Alexander Beregalov; +Cc: linux-kernel, linux-next, xfs, zohar
On Fri, 20 Feb 2009 14:00:21 +0300
Alexander Beregalov <a.beregalov@gmail.com> wrote:
> Hi
>
> I have applied the following patch from Mimi Zohar
> http://marc.info/?l=linux-next&m=123509665514552
>
> That is why it is dirty.
> The kernel can not boot without it when IMA is enabled.
>
>
> BUG: sleeping function called from invalid context at mm/slub.c:1613
> in_atomic(): 1, irqs_disabled(): 0, pid: 1514, name: mkdir
> 3 locks held by mkdir/1514:
> #0: (&type->i_mutex_dir_key#2/1){+.+.+.}, at: [<ffffffff802d3460>]
> lookup_create+0x30/0xd0
> #1: (&(&ip->i_lock)->mr_lock/1){+.+.+.}, at: [<ffffffff803ca77f>]
> xfs_ilock+0xdf/0x120
> #2: (&pag->pag_ici_lock){++++.+}, at: [<ffffffff803caed6>]
> xfs_iget+0x156/0x650
> Pid: 1514, comm: mkdir Not tainted 2.6.29-rc5-next-20090220-dirty #1
> Call Trace:
> [<ffffffff8026ad33>] ? __debug_show_held_locks+0x13/0x30
> [<ffffffff80234fe5>] __might_sleep+0x105/0x140
> [<ffffffff802c6191>] kmem_cache_alloc+0xd1/0x100
> [<ffffffff8045fb29>] ima_iint_insert+0x49/0xf0
> [<ffffffff8045fbed>] ima_inode_alloc+0x1d/0x30
> [<ffffffff802dfa5f>] inode_init_always+0xaf/0x250
> [<ffffffff803caf86>] xfs_iget+0x206/0x650
> [<ffffffff803eb8e8>] xfs_trans_iget+0x208/0x250
> [<ffffffff803ce571>] xfs_ialloc+0xc1/0x700
> [<ffffffff803ec6f9>] xfs_dir_ialloc+0xa9/0x340
> [<ffffffff8025e009>] ? down_write_nested+0x79/0x90
> [<ffffffff803ef0c1>] xfs_create+0x3e1/0x690
> [<ffffffff803fb813>] xfs_vn_mknod+0x63/0xf0
> [<ffffffff803fb8ae>] xfs_vn_mkdir+0xe/0x10
> [<ffffffff802d487c>] vfs_mkdir+0x8c/0xd0
> [<ffffffff802d6a56>] sys_mkdirat+0x106/0x120
> [<ffffffff8020bc0c>] ? sysret_check+0x27/0x62
> [<ffffffff8026c79d>] ? trace_hardirqs_on_caller+0x17d/0x1e0
> [<ffffffff802d6a83>] sys_mkdir+0x13/0x20
> [<ffffffff8020bbdb>] system_call_fastpath+0x16/0x1b
> BUG: sleeping function called from invalid context at mm/slub.c:1613
Please do not be tempted to switch ima_iint_insert() to use GFP_ATOMIC
to "fix" this.
Arguably, ima_iint_insert() should requite that the caller pass in the
gfp_t rather than assuming that GFP_KERNEL can be used. But that is
not a suitable fix for this bug.
We may need to make that change to ima_iint_insert() anyway, as there's
a good chance that callers will require GFP_NOFS.
But to fix this bug, xfs needs to stop calling inode_init_always()
under read_lock(). Because inode_alloc_security() also can sleep (see
new_inode_smack()).
Also, ima_iint_insert() does a radix_tree_lookup() without holding
ima_iint_lock, which appears to be a bug.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: next-20090220: XFS, IMA: BUG: sleeping function called from invalid context at mm/slub.c:1613
2009-02-20 20:22 ` Andrew Morton
@ 2009-02-20 22:16 ` Mimi Zohar
2009-02-20 22:28 ` Andrew Morton
2009-02-24 22:39 ` Mimi Zohar
1 sibling, 1 reply; 6+ messages in thread
From: Mimi Zohar @ 2009-02-20 22:16 UTC (permalink / raw)
To: Andrew Morton
Cc: Alexander Beregalov, linux-kernel, linux-next, xfs, James Morris
On Fri, 2009-02-20 at 12:22 -0800, Andrew Morton wrote:
> On Fri, 20 Feb 2009 14:00:21 +0300
> Alexander Beregalov <a.beregalov@gmail.com> wrote:
>
> > Hi
> >
> > I have applied the following patch from Mimi Zohar
> > http://marc.info/?l=linux-next&m=123509665514552
> >
> > That is why it is dirty.
> > The kernel can not boot without it when IMA is enabled.
With CONFIG_DEBUG_SG and IMA defined, it couldn't boot.
> > BUG: sleeping function called from invalid context at mm/slub.c:1613
> > in_atomic(): 1, irqs_disabled(): 0, pid: 1514, name: mkdir
> > 3 locks held by mkdir/1514:
> > #0: (&type->i_mutex_dir_key#2/1){+.+.+.}, at: [<ffffffff802d3460>]
> > lookup_create+0x30/0xd0
> > #1: (&(&ip->i_lock)->mr_lock/1){+.+.+.}, at: [<ffffffff803ca77f>]
> > xfs_ilock+0xdf/0x120
> > #2: (&pag->pag_ici_lock){++++.+}, at: [<ffffffff803caed6>]
> > xfs_iget+0x156/0x650
> > Pid: 1514, comm: mkdir Not tainted 2.6.29-rc5-next-20090220-dirty #1
> > Call Trace:
> > [<ffffffff8026ad33>] ? __debug_show_held_locks+0x13/0x30
> > [<ffffffff80234fe5>] __might_sleep+0x105/0x140
> > [<ffffffff802c6191>] kmem_cache_alloc+0xd1/0x100
> > [<ffffffff8045fb29>] ima_iint_insert+0x49/0xf0
> > [<ffffffff8045fbed>] ima_inode_alloc+0x1d/0x30
> > [<ffffffff802dfa5f>] inode_init_always+0xaf/0x250
> > [<ffffffff803caf86>] xfs_iget+0x206/0x650
> > [<ffffffff803eb8e8>] xfs_trans_iget+0x208/0x250
> > [<ffffffff803ce571>] xfs_ialloc+0xc1/0x700
> > [<ffffffff803ec6f9>] xfs_dir_ialloc+0xa9/0x340
> > [<ffffffff8025e009>] ? down_write_nested+0x79/0x90
> > [<ffffffff803ef0c1>] xfs_create+0x3e1/0x690
> > [<ffffffff803fb813>] xfs_vn_mknod+0x63/0xf0
> > [<ffffffff803fb8ae>] xfs_vn_mkdir+0xe/0x10
> > [<ffffffff802d487c>] vfs_mkdir+0x8c/0xd0
> > [<ffffffff802d6a56>] sys_mkdirat+0x106/0x120
> > [<ffffffff8020bc0c>] ? sysret_check+0x27/0x62
> > [<ffffffff8026c79d>] ? trace_hardirqs_on_caller+0x17d/0x1e0
> > [<ffffffff802d6a83>] sys_mkdir+0x13/0x20
> > [<ffffffff8020bbdb>] system_call_fastpath+0x16/0x1b
> > BUG: sleeping function called from invalid context at mm/slub.c:1613
>
> Please do not be tempted to switch ima_iint_insert() to use GFP_ATOMIC
> to "fix" this.
>
> Arguably, ima_iint_insert() should requite that the caller pass in the
> gfp_t rather than assuming that GFP_KERNEL can be used. But that is
> not a suitable fix for this bug.
>
> We may need to make that change to ima_iint_insert() anyway, as there's
> a good chance that callers will require GFP_NOFS.
>
>
> But to fix this bug, xfs needs to stop calling inode_init_always()
> under read_lock(). Because inode_alloc_security() also can sleep (see
> new_inode_smack()).
>
> Also, ima_iint_insert() does a radix_tree_lookup() without holding
> ima_iint_lock, which appears to be a bug.
Thank you. For now, here's a patch to add locking around the
radix_tree_lookup(). I'll look into passing gfp_t.
Mimi
integrity: ima iint radix_tree_lookup locking fix
Based on Andrew Morton's comments:
- add missing locks around radix_tree_lookup in ima_iint_insert()
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
Index: security-testing-2.6/security/integrity/ima/ima_iint.c
===================================================================
--- security-testing-2.6.orig/security/integrity/ima/ima_iint.c
+++ security-testing-2.6/security/integrity/ima/ima_iint.c
@@ -73,8 +73,10 @@ out:
if (rc < 0) {
kmem_cache_free(iint_cache, iint);
if (rc == -EEXIST) {
+ spin_lock(&ima_iint_lock);
iint = radix_tree_lookup(&ima_iint_store,
(unsigned long)inode);
+ spin_unlock(&ima_iint_lock);
} else
iint = NULL;
}
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: next-20090220: XFS, IMA: BUG: sleeping function called from invalid context at mm/slub.c:1613
2009-02-20 22:16 ` Mimi Zohar
@ 2009-02-20 22:28 ` Andrew Morton
2009-02-22 1:49 ` Mimi Zohar
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2009-02-20 22:28 UTC (permalink / raw)
To: Mimi Zohar; +Cc: a.beregalov, linux-kernel, linux-next, xfs, jmorris
On Fri, 20 Feb 2009 17:16:59 -0500
Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> integrity: ima iint radix_tree_lookup locking fix
>
> Based on Andrew Morton's comments:
> - add missing locks around radix_tree_lookup in ima_iint_insert()
>
> Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
>
> Index: security-testing-2.6/security/integrity/ima/ima_iint.c
> ===================================================================
> --- security-testing-2.6.orig/security/integrity/ima/ima_iint.c
> +++ security-testing-2.6/security/integrity/ima/ima_iint.c
> @@ -73,8 +73,10 @@ out:
> if (rc < 0) {
> kmem_cache_free(iint_cache, iint);
> if (rc == -EEXIST) {
> + spin_lock(&ima_iint_lock);
> iint = radix_tree_lookup(&ima_iint_store,
> (unsigned long)inode);
> + spin_unlock(&ima_iint_lock);
> } else
> iint = NULL;
> }
Can the -EEXIST ever actually happen?
On the inode_init_always() path (at least), I don't think that any
other thread of control can have access to this inode*, so there is no
way in which a race can result in someone else adding this inode
first?
Also, idle question: why does the radix tree exist at all? Would it
have been possible to just add a `struct ima_iint_cache *' field to the
inode instead?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: next-20090220: XFS, IMA: BUG: sleeping function called from invalid context at mm/slub.c:1613
2009-02-20 22:28 ` Andrew Morton
@ 2009-02-22 1:49 ` Mimi Zohar
0 siblings, 0 replies; 6+ messages in thread
From: Mimi Zohar @ 2009-02-22 1:49 UTC (permalink / raw)
To: Andrew Morton
Cc: a.beregalov, linux-kernel, linux-next, xfs, jmorris, david safford
On Fri, 2009-02-20 at 14:28 -0800, Andrew Morton wrote:
> On Fri, 20 Feb 2009 17:16:59 -0500
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>
> > integrity: ima iint radix_tree_lookup locking fix
> >
> > Based on Andrew Morton's comments:
> > - add missing locks around radix_tree_lookup in ima_iint_insert()
> >
> > Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
> >
> > Index: security-testing-2.6/security/integrity/ima/ima_iint.c
> > ===================================================================
> > --- security-testing-2.6.orig/security/integrity/ima/ima_iint.c
> > +++ security-testing-2.6/security/integrity/ima/ima_iint.c
> > @@ -73,8 +73,10 @@ out:
> > if (rc < 0) {
> > kmem_cache_free(iint_cache, iint);
> > if (rc == -EEXIST) {
> > + spin_lock(&ima_iint_lock);
> > iint = radix_tree_lookup(&ima_iint_store,
> > (unsigned long)inode);
> > + spin_unlock(&ima_iint_lock);
> > } else
> > iint = NULL;
> > }
>
> Can the -EEXIST ever actually happen?
> On the inode_init_always() path (at least), I don't think that any
> other thread of control can have access to this inode*, so there is no
> way in which a race can result in someone else adding this inode
> first?
True, but for those inodes which were allocated before IMA was enabled
and are being allocated in ima_iint_find_insert_get(), it could be an
issue.
> Also, idle question: why does the radix tree exist at all? Would it
> have been possible to just add a `struct ima_iint_cache *' field to the
> inode instead?
Up until November the iint was defined directly in the inode. This
changed based on Christoph Hellwig's posting
http://lkml.org/lkml/2008/10/14/170 where he said, "bloating the inode
for this is not an option".
Mimi Zohar
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: next-20090220: XFS, IMA: BUG: sleeping function called from invalid context at mm/slub.c:1613
2009-02-20 20:22 ` Andrew Morton
2009-02-20 22:16 ` Mimi Zohar
@ 2009-02-24 22:39 ` Mimi Zohar
1 sibling, 0 replies; 6+ messages in thread
From: Mimi Zohar @ 2009-02-24 22:39 UTC (permalink / raw)
To: Andrew Morton; +Cc: Alexander Beregalov, linux-kernel, linux-next, xfs
On Fri, 2009-02-20 at 12:22 -0800, Andrew Morton wrote:
> But to fix this bug, xfs needs to stop calling inode_init_always()
> under read_lock(). Because inode_alloc_security() also can sleep (see
> new_inode_smack()).
Yes, even without IMA enabled, with SELINUX, PREEMPT, and DEBUG_PREEMPT
enabled, I'm seeing the problem:
Feb 24 17:37:52 iicu10 kernel: BUG: sleeping function called from invalid context at mm/slub.c:1613
Feb 24 17:37:52 iicu10 kernel: in_atomic(): 1, irqs_disabled(): 0, pid: 3209, name: vi
Feb 24 17:37:52 iicu10 kernel: Pid: 3209, comm: vi Tainted: P 2.6.29-rc6-next-20090224 #5
Feb 24 17:37:52 iicu10 kernel: Call Trace:
Feb 24 17:37:52 iicu10 kernel: [<ffffffff8103d2c1>] __might_sleep+0x120/0x122
Feb 24 17:37:52 iicu10 kernel: [<ffffffff810c9a96>] kmem_cache_alloc+0x32/0xd1
Feb 24 17:37:52 iicu10 kernel: [<ffffffff811563ca>] selinux_inode_alloc_security+0x39/0x90
Feb 24 17:37:52 iicu10 kernel: [<ffffffff8114ef12>] security_inode_alloc+0x1c/0x1e
Feb 24 17:37:52 iicu10 kernel: [<ffffffff810e15b3>] inode_init_always+0xc3/0x1dc
Feb 24 17:37:52 iicu10 kernel: [<ffffffffa0031a34>] xfs_iget+0x14e/0x49e [xfs]
Feb 24 17:37:52 iicu10 kernel: [<ffffffffa00462c2>] xfs_trans_iget+0xa6/0xd4 [xfs]
Feb 24 17:37:52 iicu10 kernel: [<ffffffffa003463c>] xfs_ialloc+0x96/0x517 [xfs]
Feb 24 17:37:52 iicu10 kernel: [<ffffffff8118b5e2>] ? _raw_spin_lock+0x6c/0x112
Feb 24 17:37:52 iicu10 kernel: [<ffffffff8118b5e2>] ? _raw_spin_lock+0x6c/0x112
Feb 24 17:37:52 iicu10 kernel: [<ffffffffa0046bb1>] xfs_dir_ialloc+0x73/0x26a [x
Mimi Zohar
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-02-24 22:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-02-20 11:00 next-20090220: XFS, IMA: BUG: sleeping function called from invalid context at mm/slub.c:1613 Alexander Beregalov
2009-02-20 20:22 ` Andrew Morton
2009-02-20 22:16 ` Mimi Zohar
2009-02-20 22:28 ` Andrew Morton
2009-02-22 1:49 ` Mimi Zohar
2009-02-24 22:39 ` Mimi Zohar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).