diff options
author | Roland Dreier <roland@digitalvampire.org> | 2009-07-14 16:32:56 -0400 |
---|---|---|
committer | Tyler Hicks <tyhicks@linux.vnet.ibm.com> | 2009-09-23 10:10:30 -0400 |
commit | 05dafedb906425fe935199f4c92700d87285e3e9 (patch) | |
tree | 7f3162455188c322baf9c6ca9a2890394380201d /fs/ecryptfs | |
parent | 94e0fb086fc5663c38bbc0fe86d698be8314f82f (diff) |
ecryptfs: Remove unneeded locking that triggers lockdep false positives
In ecryptfs_destroy_inode(), inode_info->lower_file_mutex is locked,
and just after the mutex is unlocked, the code does:
kmem_cache_free(ecryptfs_inode_info_cache, inode_info);
This means that if another context could possibly try to take the same
mutex as ecryptfs_destroy_inode(), then it could end up getting the
mutex just before the data structure containing the mutex is freed.
So any such use would be an obvious use-after-free bug (catchable with
slab poisoning or mutex debugging), and therefore the locking in
ecryptfs_destroy_inode() is not needed and can be dropped.
Similarly, in ecryptfs_destroy_crypt_stat(), crypt_stat->keysig_list_mutex
is locked, and then the mutex is unlocked just before the code does:
memset(crypt_stat, 0, sizeof(struct ecryptfs_crypt_stat));
Therefore taking this mutex is similarly not necessary.
Removing this locking fixes false-positive lockdep reports such as the
following (and they are false-positives for exactly the same reason
that the locking is not needed):
=================================
[ INFO: inconsistent lock state ]
2.6.31-2-generic #14~rbd3
---------------------------------
inconsistent {RECLAIM_FS-ON-W} -> {IN-RECLAIM_FS-W} usage.
kswapd0/323 [HC0[0]:SC0[0]:HE1:SE1] takes:
(&inode_info->lower_file_mutex){+.+.?.}, at: [<ffffffff81210d34>] ecryptfs_destroy_inode+0x34/0x100
{RECLAIM_FS-ON-W} state was registered at:
[<ffffffff8108c02c>] mark_held_locks+0x6c/0xa0
[<ffffffff8108c10f>] lockdep_trace_alloc+0xaf/0xe0
[<ffffffff81125a51>] kmem_cache_alloc+0x41/0x1a0
[<ffffffff8113117a>] get_empty_filp+0x7a/0x1a0
[<ffffffff8112dd46>] dentry_open+0x36/0xc0
[<ffffffff8121a36c>] ecryptfs_privileged_open+0x5c/0x2e0
[<ffffffff81210283>] ecryptfs_init_persistent_file+0xa3/0xe0
[<ffffffff8120e838>] ecryptfs_lookup_and_interpose_lower+0x278/0x380
[<ffffffff8120f97a>] ecryptfs_lookup+0x12a/0x250
[<ffffffff8113930a>] real_lookup+0xea/0x160
[<ffffffff8113afc8>] do_lookup+0xb8/0xf0
[<ffffffff8113b518>] __link_path_walk+0x518/0x870
[<ffffffff8113bd9c>] path_walk+0x5c/0xc0
[<ffffffff8113be5b>] do_path_lookup+0x5b/0xa0
[<ffffffff8113bfe7>] user_path_at+0x57/0xa0
[<ffffffff811340dc>] vfs_fstatat+0x3c/0x80
[<ffffffff8113424b>] vfs_stat+0x1b/0x20
[<ffffffff81134274>] sys_newstat+0x24/0x50
[<ffffffff81013132>] system_call_fastpath+0x16/0x1b
[<ffffffffffffffff>] 0xffffffffffffffff
irq event stamp: 7811
hardirqs last enabled at (7811): [<ffffffff810c037f>] call_rcu+0x5f/0x90
hardirqs last disabled at (7810): [<ffffffff810c0353>] call_rcu+0x33/0x90
softirqs last enabled at (3764): [<ffffffff810631da>] __do_softirq+0x14a/0x220
softirqs last disabled at (3751): [<ffffffff8101440c>] call_softirq+0x1c/0x30
other info that might help us debug this:
2 locks held by kswapd0/323:
#0: (shrinker_rwsem){++++..}, at: [<ffffffff810f67ed>] shrink_slab+0x3d/0x190
#1: (&type->s_umount_key#35){.+.+..}, at: [<ffffffff811429a1>] prune_dcache+0xd1/0x1b0
stack backtrace:
Pid: 323, comm: kswapd0 Tainted: G C 2.6.31-2-generic #14~rbd3
Call Trace:
[<ffffffff8108ad6c>] print_usage_bug+0x18c/0x1a0
[<ffffffff8108aff0>] ? check_usage_forwards+0x0/0xc0
[<ffffffff8108bac2>] mark_lock_irq+0xf2/0x280
[<ffffffff8108bd87>] mark_lock+0x137/0x1d0
[<ffffffff81164710>] ? fsnotify_clear_marks_by_inode+0x30/0xf0
[<ffffffff8108bee6>] mark_irqflags+0xc6/0x1a0
[<ffffffff8108d337>] __lock_acquire+0x287/0x430
[<ffffffff8108d585>] lock_acquire+0xa5/0x150
[<ffffffff81210d34>] ? ecryptfs_destroy_inode+0x34/0x100
[<ffffffff8108d2e7>] ? __lock_acquire+0x237/0x430
[<ffffffff815526ad>] __mutex_lock_common+0x4d/0x3d0
[<ffffffff81210d34>] ? ecryptfs_destroy_inode+0x34/0x100
[<ffffffff81164710>] ? fsnotify_clear_marks_by_inode+0x30/0xf0
[<ffffffff81210d34>] ? ecryptfs_destroy_inode+0x34/0x100
[<ffffffff8129a91e>] ? _raw_spin_unlock+0x5e/0xb0
[<ffffffff81552b36>] mutex_lock_nested+0x46/0x60
[<ffffffff81210d34>] ecryptfs_destroy_inode+0x34/0x100
[<ffffffff81145d27>] destroy_inode+0x87/0xd0
[<ffffffff81146b4c>] generic_delete_inode+0x12c/0x1a0
[<ffffffff81145832>] iput+0x62/0x70
[<ffffffff811423c8>] dentry_iput+0x98/0x110
[<ffffffff81142550>] d_kill+0x50/0x80
[<ffffffff81142623>] prune_one_dentry+0xa3/0xc0
[<ffffffff811428b1>] __shrink_dcache_sb+0x271/0x290
[<ffffffff811429d9>] prune_dcache+0x109/0x1b0
[<ffffffff81142abf>] shrink_dcache_memory+0x3f/0x50
[<ffffffff810f68dd>] shrink_slab+0x12d/0x190
[<ffffffff810f9377>] balance_pgdat+0x4d7/0x640
[<ffffffff8104c4c0>] ? finish_task_switch+0x40/0x150
[<ffffffff810f63c0>] ? isolate_pages_global+0x0/0x60
[<ffffffff810f95f7>] kswapd+0x117/0x170
[<ffffffff810777a0>] ? autoremove_wake_function+0x0/0x40
[<ffffffff810f94e0>] ? kswapd+0x0/0x170
[<ffffffff810773be>] kthread+0x9e/0xb0
[<ffffffff8101430a>] child_rip+0xa/0x20
[<ffffffff81013c90>] ? restore_args+0x0/0x30
[<ffffffff81077320>] ? kthread+0x0/0xb0
[<ffffffff81014300>] ? child_rip+0x0/0x20
Signed-off-by: Roland Dreier <roland@digitalvampire.org>
Signed-off-by: Tyler Hicks <tyhicks@linux.vnet.ibm.com>
Diffstat (limited to 'fs/ecryptfs')
-rw-r--r-- | fs/ecryptfs/crypto.c | 2 | ||||
-rw-r--r-- | fs/ecryptfs/super.c | 2 |
2 files changed, 0 insertions, 4 deletions
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index b91851f1cda..4610fd6afce 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c | |||
@@ -245,13 +245,11 @@ void ecryptfs_destroy_crypt_stat(struct ecryptfs_crypt_stat *crypt_stat) | |||
245 | crypto_free_blkcipher(crypt_stat->tfm); | 245 | crypto_free_blkcipher(crypt_stat->tfm); |
246 | if (crypt_stat->hash_tfm) | 246 | if (crypt_stat->hash_tfm) |
247 | crypto_free_hash(crypt_stat->hash_tfm); | 247 | crypto_free_hash(crypt_stat->hash_tfm); |
248 | mutex_lock(&crypt_stat->keysig_list_mutex); | ||
249 | list_for_each_entry_safe(key_sig, key_sig_tmp, | 248 | list_for_each_entry_safe(key_sig, key_sig_tmp, |
250 | &crypt_stat->keysig_list, crypt_stat_list) { | 249 | &crypt_stat->keysig_list, crypt_stat_list) { |
251 | list_del(&key_sig->crypt_stat_list); | 250 | list_del(&key_sig->crypt_stat_list); |
252 | kmem_cache_free(ecryptfs_key_sig_cache, key_sig); | 251 | kmem_cache_free(ecryptfs_key_sig_cache, key_sig); |
253 | } | 252 | } |
254 | mutex_unlock(&crypt_stat->keysig_list_mutex); | ||
255 | memset(crypt_stat, 0, sizeof(struct ecryptfs_crypt_stat)); | 253 | memset(crypt_stat, 0, sizeof(struct ecryptfs_crypt_stat)); |
256 | } | 254 | } |
257 | 255 | ||
diff --git a/fs/ecryptfs/super.c b/fs/ecryptfs/super.c index 12d649602d3..b15a43a80ab 100644 --- a/fs/ecryptfs/super.c +++ b/fs/ecryptfs/super.c | |||
@@ -77,7 +77,6 @@ static void ecryptfs_destroy_inode(struct inode *inode) | |||
77 | struct ecryptfs_inode_info *inode_info; | 77 | struct ecryptfs_inode_info *inode_info; |
78 | 78 | ||
79 | inode_info = ecryptfs_inode_to_private(inode); | 79 | inode_info = ecryptfs_inode_to_private(inode); |
80 | mutex_lock(&inode_info->lower_file_mutex); | ||
81 | if (inode_info->lower_file) { | 80 | if (inode_info->lower_file) { |
82 | struct dentry *lower_dentry = | 81 | struct dentry *lower_dentry = |
83 | inode_info->lower_file->f_dentry; | 82 | inode_info->lower_file->f_dentry; |
@@ -89,7 +88,6 @@ static void ecryptfs_destroy_inode(struct inode *inode) | |||
89 | d_drop(lower_dentry); | 88 | d_drop(lower_dentry); |
90 | } | 89 | } |
91 | } | 90 | } |
92 | mutex_unlock(&inode_info->lower_file_mutex); | ||
93 | ecryptfs_destroy_crypt_stat(&inode_info->crypt_stat); | 91 | ecryptfs_destroy_crypt_stat(&inode_info->crypt_stat); |
94 | kmem_cache_free(ecryptfs_inode_info_cache, inode_info); | 92 | kmem_cache_free(ecryptfs_inode_info_cache, inode_info); |
95 | } | 93 | } |