From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Oleg Nesterov <oleg@redhat.com>,
"David S. Miller" <davem@davemloft.net>,
Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Alexander Viro <viro@zeniv.linux.org.uk>,
Cyrill Gorcunov <gorcunov@openvz.org>,
David Howells <dhowells@redhat.com>,
"Eric W. Biederman" <ebiederm@xmission.com>,
"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
Peter Zijlstra <peterz@infradead.org>,
Sasha Levin <levinsasha928@gmail.com>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
Alexey Dobriyan <adobriyan@gmail.com>,
netdev@vger.kernel.org
Subject: Re: [PATCH v2 4/7] fs/proc/task_mmu.c: shift mm_access() from m_start() to proc_maps_open()
Date: Wed, 3 Dec 2014 16:14:33 +0200 [thread overview]
Message-ID: <20141203141433.GA25683@node.dhcp.inet.fi> (raw)
In-Reply-To: <20140805194655.GA30728@redhat.com>
On Tue, Aug 05, 2014 at 09:46:55PM +0200, Oleg Nesterov wrote:
> A simple test-case from Kirill Shutemov
>
> cat /proc/self/maps >/dev/null
> chmod +x /proc/self/net/packet
> exec /proc/self/net/packet
>
> makes lockdep unhappy, cat/exec take seq_file->lock + cred_guard_mutex in
> the opposite order.
Oleg, I see it again with almost the same test-case:
cat /proc/self/stack >/dev/null
chmod +x /proc/self/net/packet
exec /proc/self/net/packet
Looks like bunch of proc files were converted to use seq_file by Alexey
Dobriyan around the same time you've fixed the issue for /proc/pid/maps.
More generic test-case:
find /proc/self/ -type f -exec dd if='{}' of=/dev/null bs=1 count=1 ';' 2>/dev/null
chmod +x /proc/self/net/packet
exec /proc/self/net/packet
David, any justification for allowing chmod +x for files under
/proc/pid/net?
[ 2.042212] ======================================================
[ 2.042930] [ INFO: possible circular locking dependency detected ]
[ 2.043648] 3.18.0-rc7-00003-g3a18ca061311-dirty #237 Not tainted
[ 2.044350] -------------------------------------------------------
[ 2.045054] sh/94 is trying to acquire lock:
[ 2.045546] (&p->lock){+.+.+.}, at: [<ffffffff811e12fd>] seq_read+0x3d/0x3e0
[ 2.045781]
[ 2.045781] but task is already holding lock:
[ 2.045781] (&sig->cred_guard_mutex){+.+.+.}, at: [<ffffffff811c0e3d>] prepare_bprm_creds+0x2d/0x90
[ 2.045781]
[ 2.045781] which lock already depends on the new lock.
[ 2.045781]
[ 2.045781]
[ 2.045781] the existing dependency chain (in reverse order) is:
[ 2.045781]
-> #1 (&sig->cred_guard_mutex){+.+.+.}:
[ 2.045781] [<ffffffff810a6e99>] __lock_acquire+0x4d9/0xd40
[ 2.045781] [<ffffffff810a7ff2>] lock_acquire+0xd2/0x2a0
[ 2.045781] [<ffffffff81849da6>] mutex_lock_killable_nested+0x66/0x460
[ 2.045781] [<ffffffff81229de4>] lock_trace+0x24/0x70
[ 2.045781] [<ffffffff81229e8f>] proc_pid_stack+0x5f/0xe0
[ 2.045781] [<ffffffff81227244>] proc_single_show+0x54/0xa0
[ 2.045781] [<ffffffff811e13a0>] seq_read+0xe0/0x3e0
[ 2.045781] [<ffffffff811b9377>] vfs_read+0x97/0x180
[ 2.045781] [<ffffffff811b9f5d>] SyS_read+0x4d/0xc0
[ 2.045781] [<ffffffff8184e492>] system_call_fastpath+0x12/0x17
[ 2.045781]
-> #0 (&p->lock){+.+.+.}:
[ 2.045781] [<ffffffff810a389f>] validate_chain.isra.36+0xfff/0x1400
[ 2.045781] [<ffffffff810a6e99>] __lock_acquire+0x4d9/0xd40
[ 2.045781] [<ffffffff810a7ff2>] lock_acquire+0xd2/0x2a0
[ 2.045781] [<ffffffff81849629>] mutex_lock_nested+0x69/0x3c0
[ 2.045781] [<ffffffff811e12fd>] seq_read+0x3d/0x3e0
[ 2.045781] [<ffffffff81226428>] proc_reg_read+0x48/0x70
[ 2.045781] [<ffffffff811b9377>] vfs_read+0x97/0x180
[ 2.045781] [<ffffffff811bf1a8>] kernel_read+0x48/0x60
[ 2.045781] [<ffffffff811bfb2c>] prepare_binprm+0xdc/0x180
[ 2.045781] [<ffffffff811c139a>] do_execve_common.isra.29+0x4fa/0x960
[ 2.045781] [<ffffffff811c1818>] do_execve+0x18/0x20
[ 2.045781] [<ffffffff811c1b05>] SyS_execve+0x25/0x30
[ 2.045781] [<ffffffff8184ea49>] stub_execve+0x69/0xa0
[ 2.045781]
[ 2.045781] other info that might help us debug this:
[ 2.045781]
[ 2.045781] Possible unsafe locking scenario:
[ 2.045781]
[ 2.045781] CPU0 CPU1
[ 2.045781] ---- ----
[ 2.045781] lock(&sig->cred_guard_mutex);
[ 2.045781] lock(&p->lock);
[ 2.045781] lock(&sig->cred_guard_mutex);
[ 2.045781] lock(&p->lock);
[ 2.045781]
[ 2.045781] *** DEADLOCK ***
[ 2.045781]
[ 2.045781] 1 lock held by sh/94:
[ 2.045781] #0: (&sig->cred_guard_mutex){+.+.+.}, at: [<ffffffff811c0e3d>] prepare_bprm_creds+0x2d/0x90
[ 2.045781]
[ 2.045781] stack backtrace:
[ 2.045781] CPU: 0 PID: 94 Comm: sh Not tainted 3.18.0-rc7-00003-g3a18ca061311-dirty #237
[ 2.045781] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.7.5-0-ge51488c-20140602_164612-nilsson.home.kraxel.org 04/01/2014
[ 2.045781] ffffffff82a48d50 ffff88085427bad8 ffffffff81844a85 0000000000000cac
[ 2.045781] ffffffff82a654a0 ffff88085427bb28 ffffffff810a1b03 0000000000000000
[ 2.045781] ffff88085427bb68 ffff88085427bb28 ffff8808547f1500 ffff8808547f1c40
[ 2.045781] Call Trace:
[ 2.045781] [<ffffffff81844a85>] dump_stack+0x4e/0x68
[ 2.045781] [<ffffffff810a1b03>] print_circular_bug+0x203/0x310
[ 2.045781] [<ffffffff810a389f>] validate_chain.isra.36+0xfff/0x1400
[ 2.045781] [<ffffffff8108fa76>] ? local_clock+0x16/0x30
[ 2.045781] [<ffffffff810a6e99>] __lock_acquire+0x4d9/0xd40
[ 2.045781] [<ffffffff810a7ff2>] lock_acquire+0xd2/0x2a0
[ 2.045781] [<ffffffff811e12fd>] ? seq_read+0x3d/0x3e0
[ 2.045781] [<ffffffff81849629>] mutex_lock_nested+0x69/0x3c0
[ 2.045781] [<ffffffff811e12fd>] ? seq_read+0x3d/0x3e0
[ 2.045781] [<ffffffff8108f9f8>] ? sched_clock_cpu+0x98/0xc0
[ 2.045781] [<ffffffff811e12fd>] ? seq_read+0x3d/0x3e0
[ 2.045781] [<ffffffff814050b9>] ? lockref_put_or_lock+0x29/0x40
[ 2.045781] [<ffffffff811e12fd>] seq_read+0x3d/0x3e0
[ 2.045781] [<ffffffff814050b9>] ? lockref_put_or_lock+0x29/0x40
[ 2.045781] [<ffffffff81226428>] proc_reg_read+0x48/0x70
[ 2.045781] [<ffffffff811b9377>] vfs_read+0x97/0x180
[ 2.045781] [<ffffffff811bf1a8>] kernel_read+0x48/0x60
[ 2.045781] [<ffffffff811bfb2c>] prepare_binprm+0xdc/0x180
[ 2.045781] [<ffffffff811c139a>] do_execve_common.isra.29+0x4fa/0x960
[ 2.092142] tsc: Refined TSC clocksource calibration: 2693.484 MHz
[ 2.045781] [<ffffffff811c0fd3>] ? do_execve_common.isra.29+0x133/0x960
[ 2.045781] [<ffffffff8184f04d>] ? retint_swapgs+0xe/0x13
[ 2.045781] [<ffffffff811c1818>] do_execve+0x18/0x20
[ 2.045781] [<ffffffff811c1b05>] SyS_execve+0x25/0x30
[ 2.045781] [<ffffffff8184ea49>] stub_execve+0x69/0xa0
--
Kirill A. Shutemov
next prev parent reply other threads:[~2014-12-03 14:14 UTC|newest]
Thread overview: 35+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-05 19:46 [PATCH v2 0/7] /proc/PID/*maps* fixes/cleanups Oleg Nesterov
2014-08-05 19:46 ` [PATCH v2 1/7] fs/proc/task_mmu.c: don't use task->mm in m_start() and show_*map() Oleg Nesterov
2014-08-06 9:52 ` Kirill A. Shutemov
2014-08-06 14:10 ` Oleg Nesterov
2014-08-06 19:41 ` Oleg Nesterov
2014-08-05 19:46 ` [PATCH v2 2/7] fs/proc/task_mmu.c: unify/simplify do_maps_open() and numa_maps_open() Oleg Nesterov
2014-08-06 9:55 ` Kirill A. Shutemov
2014-08-05 19:46 ` [PATCH v2 3/7] proc: introduce proc_mem_open() Oleg Nesterov
2014-08-06 9:57 ` Kirill A. Shutemov
2014-08-05 19:46 ` [PATCH v2 4/7] fs/proc/task_mmu.c: shift mm_access() from m_start() to proc_maps_open() Oleg Nesterov
2014-08-06 10:05 ` Kirill A. Shutemov
2014-12-03 14:14 ` Kirill A. Shutemov [this message]
2014-12-03 16:59 ` Eric W. Biederman
2014-12-04 16:17 ` Kirill A. Shutemov
2014-12-03 17:34 ` Oleg Nesterov
2014-08-05 19:46 ` [PATCH v2 5/7] fs/proc/task_mmu.c: simplify the vma_stop() logic Oleg Nesterov
2014-08-06 10:12 ` Kirill A. Shutemov
2014-08-05 19:47 ` [PATCH v2 6/7] fs/proc/task_mmu.c: cleanup the "tail_vma" horror in m_next() Oleg Nesterov
2014-08-06 10:17 ` Kirill A. Shutemov
2014-08-05 19:47 ` [PATCH v2 7/7] fs/proc/task_mmu.c: shift "priv->task = NULL" from m_start() to m_stop() Oleg Nesterov
2014-08-06 10:18 ` Kirill A. Shutemov
2014-08-06 10:19 ` [PATCH v2 0/7] /proc/PID/*maps* fixes/cleanups Kirill A. Shutemov
2014-08-06 18:48 ` Cyrill Gorcunov
2014-08-07 19:17 ` [PATCH 0/5] fs/proc/task_mmu.c: cleanup tail_vma/last_addr mess Oleg Nesterov
2014-08-07 19:17 ` [PATCH 1/5] fs/proc/task_mmu.c: kill the suboptimal and confusing m->version logic Oleg Nesterov
2014-08-07 19:17 ` [PATCH 2/5] fs/proc/task_mmu.c: simplify m_start() to make it readable Oleg Nesterov
2014-08-07 19:17 ` [PATCH 3/5] fs/proc/task_mmu.c: introduce m_next_vma() helper Oleg Nesterov
2014-08-07 19:17 ` [PATCH 4/5] fs/proc/task_mmu.c: reintroduce m->version logic Oleg Nesterov
2014-08-07 19:18 ` [PATCH 5/5] fs/proc/task_mmu.c: update m->version in the main loop in m_start() Oleg Nesterov
2014-08-11 17:00 ` [PATCH 0/3] fs/proc/task_nommu.c: copy-and-paste the changes from task_mmu.c Oleg Nesterov
2014-08-11 17:00 ` [PATCH 1/3] fs/proc/task_nommu.c: change maps_open() to use __seq_open_private() Oleg Nesterov
2014-08-11 17:00 ` [PATCH 2/3] fs/proc/task_nommu.c: shift mm_access() from m_start() to proc_maps_open() Oleg Nesterov
2014-08-11 17:00 ` [PATCH 3/3] fs/proc/task_nommu.c: don't use priv->task->mm Oleg Nesterov
2014-08-12 13:57 ` [PATCH 0/3] fs/proc/task_nommu.c: copy-and-paste the changes from task_mmu.c Greg Ungerer
2014-08-12 17:35 ` Oleg Nesterov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141203141433.GA25683@node.dhcp.inet.fi \
--to=kirill@shutemov.name \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=davem@davemloft.net \
--cc=dhowells@redhat.com \
--cc=ebiederm@xmission.com \
--cc=gorcunov@openvz.org \
--cc=kirill.shutemov@linux.intel.com \
--cc=levinsasha928@gmail.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).