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
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.
On Wed, May 9, 2012 at 6:12 PM, Al Viro <[email protected]> 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?
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...
On Wed, May 09, 2012 at 06:23:30PM +0200, Sasha Levin wrote:
> On Wed, May 9, 2012 at 6:12 PM, Al Viro <[email protected]> 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.
On Wed, May 9, 2012 at 6:28 PM, Al Viro <[email protected]> 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 <[email protected]> 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.
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 <[email protected]> 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...
On Wed, May 9, 2012 at 6:37 PM, Al Viro <[email protected]> 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?
On Wed, May 09, 2012 at 07:13:29PM +0200, Sasha Levin wrote:
> On Wed, May 9, 2012 at 6:37 PM, Al Viro <[email protected]> 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