vfs: INFO: possible circular locking dependency detected

vfs: INFO: possible circular locking dependency detected

[Sasha Levin]

Hi all,

I've started seeing the following warning while fuzzing inside a KVM guest with the latest -next:

[   91.892449] ======================================================
[   91.894167] [ INFO: possible circular locking dependency detected ]
[   91.894167] 3.4.0-rc6-next-20120508-sasha-00003-g9f34dd7-dirty #120 Tainted: G        W   
[   91.894167] -------------------------------------------------------
[   91.894167] trinity/21513 is trying to acquire lock:
[   91.894167]  (&p->lock){+.+.+.}, at: [<ffffffff811fe53a>] seq_read+0x3a/0x420
[   91.894167] 
[   91.894167] but task is already holding lock:
[   91.894167]  (&sig->cred_guard_mutex){+.+.+.}, at: [<ffffffff811e2cb2>] prepare_bprm_creds+0x32/0x80
[   91.894167] 
[   91.894167] which lock already depends on the new lock.
[   91.894167] 
[   91.894167] 
[   91.894167] the existing dependency chain (in reverse order) is:
[   91.894167] 
[   91.894167] -> #1 (&sig->cred_guard_mutex){+.+.+.}:
[   91.894167]        [<ffffffff81115e3e>] validate_chain.clone.26+0x70e/0x8c0
[   91.894167]        [<ffffffff8111910d>] __lock_acquire+0xa3d/0xb00
[   91.894167]        [<ffffffff81119881>] lock_acquire+0xd1/0x110
[   91.894167]        [<ffffffff82d89515>] __mutex_lock_common+0x65/0x570
[   91.894167]        [<ffffffff82d89b00>] mutex_lock_killable_nested+0x40/0x50
[   91.894167]        [<ffffffff810b1a7f>] mm_access+0x2f/0xb0
[   91.894167]        [<ffffffff8123f0c4>] m_start+0x74/0x1c0
[   91.894167]        [<ffffffff811fe687>] seq_read+0x187/0x420
[   91.894167]        [<ffffffff811dac8d>] vfs_read+0xbd/0x190
[   91.894167]        [<ffffffff811db04f>] sys_read+0x4f/0x90
[   91.894167]        [<ffffffff82d8dbbd>] system_call_fastpath+0x1a/0x1f
[   91.894167] 
[   91.894167] -> #0 (&p->lock){+.+.+.}:
[   91.894167]        [<ffffffff8111523e>] check_prev_add+0x11e/0x610
[   91.894167]        [<ffffffff81115e3e>] validate_chain.clone.26+0x70e/0x8c0
[   91.894167]        [<ffffffff8111910d>] __lock_acquire+0xa3d/0xb00
[   91.894167]        [<ffffffff81119881>] lock_acquire+0xd1/0x110
[   91.894167]        [<ffffffff82d89515>] __mutex_lock_common+0x65/0x570
[   91.894167]        [<ffffffff82d89a60>] mutex_lock_nested+0x40/0x50
[   91.894167]        [<ffffffff811fe53a>] seq_read+0x3a/0x420
[   91.894167]        [<ffffffff81241769>] proc_reg_read+0xa9/0xe0
[   91.894167]        [<ffffffff811dac8d>] vfs_read+0xbd/0x190
[   91.894167]        [<ffffffff811e2a51>] kernel_read+0x41/0x60
[   91.894167]        [<ffffffff811e2ed3>] prepare_binprm+0x123/0x140
[   91.894167]        [<ffffffff811e3487>] do_execve_common.clone.32+0x197/0x320
[   91.894167]        [<ffffffff811e3626>] do_execve+0x16/0x20
[   91.894167]        [<ffffffff81056571>] sys_execve+0x51/0x80
[   91.894167]        [<ffffffff82d8e08c>] stub_execve+0x6c/0xc0
[   91.894167] 
[   91.894167] other info that might help us debug this:
[   91.894167] 
[   91.894167]  Possible unsafe locking scenario:
[   91.894167] 
[   91.894167]        CPU0                    CPU1
[   91.894167]        ----                    ----
[   91.894167]   lock(&sig->cred_guard_mutex);
[   91.894167]                                lock(&p->lock);
[   91.894167]                                lock(&sig->cred_guard_mutex);
[   91.894167]   lock(&p->lock);
[   91.894167] 
[   91.894167]  *** DEADLOCK ***
[   91.894167] 
[   91.894167] 1 lock held by trinity/21513:
[   91.894167]  #0:  (&sig->cred_guard_mutex){+.+.+.}, at: [<ffffffff811e2cb2>] prepare_bprm_creds+0x32/0x80
[   91.894167] 
[   91.894167] stack backtrace:
[   91.894167] Pid: 21513, comm: trinity Tainted: G        W    3.4.0-rc6-next-20120508-sasha-00003-g9f34dd7-dirty #120
[   91.894167] Call Trace:
[   91.894167]  [<ffffffff811144d7>] print_circular_bug+0x107/0x130
[   91.894167]  [<ffffffff8111523e>] check_prev_add+0x11e/0x610
[   91.894167]  [<ffffffff81115e3e>] validate_chain.clone.26+0x70e/0x8c0
[   91.894167]  [<ffffffff8111910d>] __lock_acquire+0xa3d/0xb00
[   91.894167]  [<ffffffff81116c06>] ? mark_held_locks+0xe6/0x120
[   91.894167]  [<ffffffff81119881>] lock_acquire+0xd1/0x110
[   91.894167]  [<ffffffff811fe53a>] ? seq_read+0x3a/0x420
[   91.894167]  [<ffffffff82d8d426>] ? retint_kernel+0x26/0x30
[   91.894167]  [<ffffffff82d89515>] __mutex_lock_common+0x65/0x570
[   91.894167]  [<ffffffff811fe53a>] ? seq_read+0x3a/0x420
[   91.894167]  [<ffffffff810b4879>] ? vprintk+0x3f9/0x480
[   91.894167]  [<ffffffff811fe53a>] ? seq_read+0x3a/0x420
[   91.894167]  [<ffffffff811fe500>] ? seq_open+0xb0/0xb0
[   91.894167]  [<ffffffff82d89a60>] mutex_lock_nested+0x40/0x50
[   91.894167]  [<ffffffff811fe53a>] seq_read+0x3a/0x420
[   91.894167]  [<ffffffff810e891e>] ? sub_preempt_count+0xae/0xe0
[   91.894167]  [<ffffffff811fe500>] ? seq_open+0xb0/0xb0
[   91.894167]  [<ffffffff81241769>] proc_reg_read+0xa9/0xe0
[   91.894167]  [<ffffffff811dac8d>] vfs_read+0xbd/0x190
[   91.894167]  [<ffffffff811e2a51>] kernel_read+0x41/0x60
[   91.894167]  [<ffffffff811e2ed3>] prepare_binprm+0x123/0x140
[   91.894167]  [<ffffffff811e3487>] do_execve_common.clone.32+0x197/0x320
[   91.894167]  [<ffffffff811e3626>] do_execve+0x16/0x20
[   91.894167]  [<ffffffff81056571>] sys_execve+0x51/0x80
[   91.894167]  [<ffffffff82d8e08c>] stub_execve+0x6c/0xc0

Re: vfs: INFO: possible circular locking dependency detected

[Al Viro]

On Wed, May 09, 2012 at 05:25:14PM +0200, Sasha Levin wrote:
> Hi all,
> 
> I've started seeing the following warning while fuzzing inside a KVM guest with the latest -next:

[->read() may grab ->cred_guard_mutex, but it may itself be called by
prepare_binprm() after having ->cred_guard_mutex grabbed]

Nasty, that...  What's more, it's not just prepare_binprm() itself -
->load_binary() might end up calling read(); it doesn't have to
limit itself to mmap(), so essentially anything that can be grabbed
by ->read() of a regular file might nest under ->cred_guard_mutex.

	AFAICS, /proc/*/stack, /proc/*/syscall, /proc/*/personality,
/proc/*/io_accounting, /proc/*/auxv, /proc/*/environ, /proc/*/*maps
and /proc/*/pagemap have ->cred_guard_mutex grabbed on read.  seq_file
is a red herring here - io_accounting has the same issue and it does
things directly, without seq_read().

	It's not a realistic attack, fortunately, since you need root
to get past open_exec() on any of those...  Wait.  How _did_ you get
past open_exec(), anyway?  MAY_EXEC is not supposed to be granted on
anything that has no exec bits at all and AFAICS none of those files
have them.

Re: vfs: INFO: possible circular locking dependency detected

[Sasha Levin]

On Wed, May 9, 2012 at 6:12 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Wed, May 09, 2012 at 05:25:14PM +0200, Sasha Levin wrote:
>> Hi all,
>>
>> I've started seeing the following warning while fuzzing inside a KVM guest with the latest -next:
>        It's not a realistic attack, fortunately, since you need root
> to get past open_exec() on any of those...  Wait.  How _did_ you get
> past open_exec(), anyway?  MAY_EXEC is not supposed to be granted on
> anything that has no exec bits at all and AFAICS none of those files
> have them.

You could chmod +x and run them, no?

Re: vfs: INFO: possible circular locking dependency detected

[Al Viro]

On Wed, May 09, 2012 at 06:23:30PM +0200, Sasha Levin wrote:
> On Wed, May 9, 2012 at 6:12 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > On Wed, May 09, 2012 at 05:25:14PM +0200, Sasha Levin wrote:
> >> Hi all,
> >>
> >> I've started seeing the following warning while fuzzing inside a KVM guest with the latest -next:
> > ? ? ? ?It's not a realistic attack, fortunately, since you need root
> > to get past open_exec() on any of those... ?Wait. ?How _did_ you get
> > past open_exec(), anyway? ?MAY_EXEC is not supposed to be granted on
> > anything that has no exec bits at all and AFAICS none of those files
> > have them.
> 
> You could chmod +x and run them, no?

Can't.  proc_setattr() will give you -EPERM and refuse to do anything
if you call it with ATTR_MODE in ->ia_valid.

Re: vfs: INFO: possible circular locking dependency detected

[Sasha Levin]

On Wed, May 9, 2012 at 6:28 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Wed, May 09, 2012 at 06:23:30PM +0200, Sasha Levin wrote:
>> On Wed, May 9, 2012 at 6:12 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>> > On Wed, May 09, 2012 at 05:25:14PM +0200, Sasha Levin wrote:
>> >> Hi all,
>> >>
>> >> I've started seeing the following warning while fuzzing inside a KVM guest with the latest -next:
>> > ? ? ? ?It's not a realistic attack, fortunately, since you need root
>> > to get past open_exec() on any of those... ?Wait. ?How _did_ you get
>> > past open_exec(), anyway? ?MAY_EXEC is not supposed to be granted on
>> > anything that has no exec bits at all and AFAICS none of those files
>> > have them.
>>
>> You could chmod +x and run them, no?
>
> Can't.  proc_setattr() will give you -EPERM and refuse to do anything
> if you call it with ATTR_MODE in ->ia_valid.

If we look at /proc/irq/*/smp_affinity, which uses seq file ops, we can do this:

sh-4.2# ls -al /proc/irq/5/smp_affinity
-rw------- 1 root 0 0 May  9 16:35 /proc/irq/5/smp_affinity
sh-4.2# chmod +x /proc/irq/5/smp_affinity
sh-4.2# ls -al /proc/irq/5/smp_affinity
-rwx--x--x 1 root 0 0 May  9 16:35 /proc/irq/5/smp_affinity
sh-4.2# /proc/irq/5/smp_affinity
/proc/irq/5/smp_affinity: line 1: 1f: command not found

There are quite a lot of files under /proc that let me do that.

Re: vfs: INFO: possible circular locking dependency detected

[Al Viro]

On Wed, May 09, 2012 at 05:28:54PM +0100, Al Viro wrote:
> On Wed, May 09, 2012 at 06:23:30PM +0200, Sasha Levin wrote:
> > On Wed, May 9, 2012 at 6:12 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > > On Wed, May 09, 2012 at 05:25:14PM +0200, Sasha Levin wrote:
> > >> Hi all,
> > >>
> > >> I've started seeing the following warning while fuzzing inside a KVM guest with the latest -next:
> > > ? ? ? ?It's not a realistic attack, fortunately, since you need root
> > > to get past open_exec() on any of those... ?Wait. ?How _did_ you get
> > > past open_exec(), anyway? ?MAY_EXEC is not supposed to be granted on
> > > anything that has no exec bits at all and AFAICS none of those files
> > > have them.
> > 
> > You could chmod +x and run them, no?
> 
> Can't.  proc_setattr() will give you -EPERM and refuse to do anything
> if you call it with ATTR_MODE in ->ia_valid.

OTOH, you probably can do that on unrelated seq_file outside of per-process
part of procfs.  So, yes, one could get a warning like that if they, as root,
would do e.g.
chmod +x /proc/swaps
attempt to execve() /proc/swaps
cat /proc/self/environ
and enjoy the hard-earned false positive (it's a different seq_file, so
we have no deadlock).  If that's _all_ that happened, I'm not particulary
concerned; it's not pretty, but saying "thou shalt not grab ->cred_guard_mutex
anywhere in ->read() on anything that has exec bits or might get one" is
not too terrible.  If that's something else, though, we might have a real
problem...

Re: vfs: INFO: possible circular locking dependency detected

[Sasha Levin]

On Wed, May 9, 2012 at 6:37 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> If that's _all_ that happened, I'm not particulary
> concerned; it's not pretty, but saying "thou shalt not grab ->cred_guard_mutex
> anywhere in ->read() on anything that has exec bits or might get one" is
> not too terrible.  If that's something else, though, we might have a real
> problem...

That's probably the case. The proc file got chmodded and exec'ed.

Can we do something to eliminate this false positive though? I can't
think of anything nice...

btw,
I've never seen this issue before, even though I run same tests for a
while now. What could have triggered it now?

Re: vfs: INFO: possible circular locking dependency detected

[Dave Jones]

On Wed, May 09, 2012 at 07:13:29PM +0200, Sasha Levin wrote:
 > On Wed, May 9, 2012 at 6:37 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
 > > If that's _all_ that happened, I'm not particulary
 > > concerned; it's not pretty, but saying "thou shalt not grab ->cred_guard_mutex
 > > anywhere in ->read() on anything that has exec bits or might get one" is
 > > not too terrible.  If that's something else, though, we might have a real
 > > problem...
 > 
 > That's probably the case. The proc file got chmodded and exec'ed.
 > 
 > Can we do something to eliminate this false positive though? I can't
 > think of anything nice...
 > 
 > btw,
 > I've never seen this issue before, even though I run same tests for a
 > while now. What could have triggered it now?

Assuming you're running trinity du-jour, I only added fuzzing of execve yesterday.

	Dave

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: vfs: INFO: possible circular locking dependency detected

[Al Viro]

On Wed, May 09, 2012 at 05:12:03PM +0100, Al Viro wrote:
> On Wed, May 09, 2012 at 05:25:14PM +0200, Sasha Levin wrote:
> > Hi all,
> > 
> > I've started seeing the following warning while fuzzing inside a KVM guest with the latest -next:
> 
> [->read() may grab ->cred_guard_mutex, but it may itself be called by
> prepare_binprm() after having ->cred_guard_mutex grabbed]
> 
> Nasty, that...  What's more, it's not just prepare_binprm() itself -
> ->load_binary() might end up calling read(); it doesn't have to
> limit itself to mmap(), so essentially anything that can be grabbed
> by ->read() of a regular file might nest under ->cred_guard_mutex.
> 
> 	AFAICS, /proc/*/stack, /proc/*/syscall, /proc/*/personality,
> /proc/*/io_accounting, /proc/*/auxv, /proc/*/environ, /proc/*/*maps
> and /proc/*/pagemap have ->cred_guard_mutex grabbed on read.  seq_file
> is a red herring here - io_accounting has the same issue and it does
> things directly, without seq_read().
> 
> 	It's not a realistic attack, fortunately, since you need root
> to get past open_exec() on any of those...  Wait.  How _did_ you get
> past open_exec(), anyway?  MAY_EXEC is not supposed to be granted on
> anything that has no exec bits at all and AFAICS none of those files
> have them.

FWIW, that's _probably_ a false positive, but I really wonder what has
triggered it.  It would take seq_file-based file somewhere with _some_
exec bits set (otherwise it shouldn't have been seen by prepare_binprm()).
The file itself isn't one of those that grab ->cred_guard_mutex anywhere
in their ->read(), but since lockdep can't tell one seq_file from another,
we get the warning.

	The interesting part is who the hell had managed to do executable
seq_file-based anything - false positive or not, it's almost certainly
a bug...