| 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
| ||
|
Message-ID: <1357688156-25387-48-git-send-email-paul.gortmaker@windriver.com>
Date: Tue, 8 Jan 2013 18:35:26 -0500
From: Paul Gortmaker <paul.gortmaker@...driver.com>
To: <stable@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC: Robert Richter <robert.richter@....com>,
Carl Love <carll@...ibm.com>,
Paul Gortmaker <paul.gortmaker@...driver.com>
Subject: [v2.6.34-stable 47/77] oprofile: Fix locking dependency in sync_start()
From: Robert Richter <robert.richter@....com>
-------------------
This is a commit scheduled for the next v2.6.34 longterm release.
http://git.kernel.org/?p=linux/kernel/git/paulg/longterm-queue-2.6.34.git
If you see a problem with using this for longterm, please comment.
-------------------
commit 130c5ce716c9bfd1c2a2ec840a746eb7ff9ce1e6 upstream.
This fixes the A->B/B->A locking dependency, see the warning below.
The function task_exit_notify() is called with (task_exit_notifier)
.rwsem set and then calls sync_buffer() which locks buffer_mutex. In
sync_start() the buffer_mutex was set to prevent notifier functions to
be started before sync_start() is finished. But when registering the
notifier, (task_exit_notifier).rwsem is locked too, but now in
different order than in sync_buffer(). In theory this causes a locking
dependency, what does not occur in practice since task_exit_notify()
is always called after the notifier is registered which means the lock
is already released.
However, after checking the notifier functions it turned out the
buffer_mutex in sync_start() is unnecessary. This is because
sync_buffer() may be called from the notifiers even if sync_start()
did not finish yet, the buffers are already allocated but empty. No
need to protect this with the mutex.
So we fix this theoretical locking dependency by removing buffer_mutex
in sync_start(). This is similar to the implementation before commit:
750d857 oprofile: fix crash when accessing freed task structs
which introduced the locking dependency.
Lockdep warning:
oprofiled/4447 is trying to acquire lock:
(buffer_mutex){+.+...}, at: [<ffffffffa0000e55>] sync_buffer+0x31/0x3ec [oprofile]
but task is already holding lock:
((task_exit_notifier).rwsem){++++..}, at: [<ffffffff81058026>] __blocking_notifier_call_chain+0x39/0x67
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 ((task_exit_notifier).rwsem){++++..}:
[<ffffffff8106557f>] lock_acquire+0xf8/0x11e
[<ffffffff81463a2b>] down_write+0x44/0x67
[<ffffffff810581c0>] blocking_notifier_chain_register+0x52/0x8b
[<ffffffff8105a6ac>] profile_event_register+0x2d/0x2f
[<ffffffffa00013c1>] sync_start+0x47/0xc6 [oprofile]
[<ffffffffa00001bb>] oprofile_setup+0x60/0xa5 [oprofile]
[<ffffffffa00014e3>] event_buffer_open+0x59/0x8c [oprofile]
[<ffffffff810cd3b9>] __dentry_open+0x1eb/0x308
[<ffffffff810cd59d>] nameidata_to_filp+0x60/0x67
[<ffffffff810daad6>] do_last+0x5be/0x6b2
[<ffffffff810dbc33>] path_openat+0xc7/0x360
[<ffffffff810dbfc5>] do_filp_open+0x3d/0x8c
[<ffffffff810ccfd2>] do_sys_open+0x110/0x1a9
[<ffffffff810cd09e>] sys_open+0x20/0x22
[<ffffffff8146ad4b>] system_call_fastpath+0x16/0x1b
-> #0 (buffer_mutex){+.+...}:
[<ffffffff81064dfb>] __lock_acquire+0x1085/0x1711
[<ffffffff8106557f>] lock_acquire+0xf8/0x11e
[<ffffffff814634f0>] mutex_lock_nested+0x63/0x309
[<ffffffffa0000e55>] sync_buffer+0x31/0x3ec [oprofile]
[<ffffffffa0001226>] task_exit_notify+0x16/0x1a [oprofile]
[<ffffffff81467b96>] notifier_call_chain+0x37/0x63
[<ffffffff8105803d>] __blocking_notifier_call_chain+0x50/0x67
[<ffffffff81058068>] blocking_notifier_call_chain+0x14/0x16
[<ffffffff8105a718>] profile_task_exit+0x1a/0x1c
[<ffffffff81039e8f>] do_exit+0x2a/0x6fc
[<ffffffff8103a5e4>] do_group_exit+0x83/0xae
[<ffffffff8103a626>] sys_exit_group+0x17/0x1b
[<ffffffff8146ad4b>] system_call_fastpath+0x16/0x1b
other info that might help us debug this:
1 lock held by oprofiled/4447:
#0: ((task_exit_notifier).rwsem){++++..}, at: [<ffffffff81058026>] __blocking_notifier_call_chain+0x39/0x67
stack backtrace:
Pid: 4447, comm: oprofiled Not tainted 2.6.39-00007-gcf4d8d4 #10
Call Trace:
[<ffffffff81063193>] print_circular_bug+0xae/0xbc
[<ffffffff81064dfb>] __lock_acquire+0x1085/0x1711
[<ffffffffa0000e55>] ? sync_buffer+0x31/0x3ec [oprofile]
[<ffffffff8106557f>] lock_acquire+0xf8/0x11e
[<ffffffffa0000e55>] ? sync_buffer+0x31/0x3ec [oprofile]
[<ffffffff81062627>] ? mark_lock+0x42f/0x552
[<ffffffffa0000e55>] ? sync_buffer+0x31/0x3ec [oprofile]
[<ffffffff814634f0>] mutex_lock_nested+0x63/0x309
[<ffffffffa0000e55>] ? sync_buffer+0x31/0x3ec [oprofile]
[<ffffffffa0000e55>] sync_buffer+0x31/0x3ec [oprofile]
[<ffffffff81058026>] ? __blocking_notifier_call_chain+0x39/0x67
[<ffffffff81058026>] ? __blocking_notifier_call_chain+0x39/0x67
[<ffffffffa0001226>] task_exit_notify+0x16/0x1a [oprofile]
[<ffffffff81467b96>] notifier_call_chain+0x37/0x63
[<ffffffff8105803d>] __blocking_notifier_call_chain+0x50/0x67
[<ffffffff81058068>] blocking_notifier_call_chain+0x14/0x16
[<ffffffff8105a718>] profile_task_exit+0x1a/0x1c
[<ffffffff81039e8f>] do_exit+0x2a/0x6fc
[<ffffffff81465031>] ? retint_swapgs+0xe/0x13
[<ffffffff8103a5e4>] do_group_exit+0x83/0xae
[<ffffffff8103a626>] sys_exit_group+0x17/0x1b
[<ffffffff8146ad4b>] system_call_fastpath+0x16/0x1b
Reported-by: Marcin Slusarz <marcin.slusarz@...il.com>
Cc: Carl Love <carll@...ibm.com>
Signed-off-by: Robert Richter <robert.richter@....com>
Signed-off-by: Paul Gortmaker <paul.gortmaker@...driver.com>
---
drivers/oprofile/buffer_sync.c | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/oprofile/buffer_sync.c b/drivers/oprofile/buffer_sync.c
index e353ebf..5830bdc 100644
--- a/drivers/oprofile/buffer_sync.c
+++ b/drivers/oprofile/buffer_sync.c
@@ -155,8 +155,6 @@ int sync_start(void)
if (!zalloc_cpumask_var(&marked_cpus, GFP_KERNEL))
return -ENOMEM;
- mutex_lock(&buffer_mutex);
-
err = task_handoff_register(&task_free_nb);
if (err)
goto out1;
@@ -173,7 +171,6 @@ int sync_start(void)
start_cpu_work();
out:
- mutex_unlock(&buffer_mutex);
return err;
out4:
profile_event_unregister(PROFILE_MUNMAP, &munmap_nb);
@@ -190,14 +187,13 @@ out1:
void sync_stop(void)
{
- /* flush buffers */
- mutex_lock(&buffer_mutex);
end_cpu_work();
unregister_module_notifier(&module_load_nb);
profile_event_unregister(PROFILE_MUNMAP, &munmap_nb);
profile_event_unregister(PROFILE_TASK_EXIT, &task_exit_nb);
task_handoff_unregister(&task_free_nb);
- mutex_unlock(&buffer_mutex);
+ barrier(); /* do all of the above first */
+
flush_scheduled_work();
free_all_tasks();
--
1.7.12.1
--
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/
Powered by blists - more mailing lists