[PATCH] ima: prevent dead lock when a file is opened for direct io
From: Mimi Zohar
Date: Wed Feb 20 2013 - 17:22:22 EST
Hi Al,
Are there any negative repercussions to temporarily removing the
o_direct flag in order to calculate the file hash?
thanks,
Mimi
-----
Files are measured or appraised based on the IMA policy. When a file
in policy is opened for read with the O_DIRECT flag set, a deadlock
occurs due to do_blockdev_direct_IO() taking i_mutex before calling
filemap_write_and_wait_range(). The i_mutex was previously taken in
process_measurement(). This patch temporarily removes the O_DIRECT
flag in order to calculate the hash and restores it once completed.
[ 3.751980]
[ 3.752074] =============================================
[ 3.752074] [ INFO: possible recursive locking detected ]
[ 3.752074] 3.7.5+ #30 Not tainted
[ 3.752074] ---------------------------------------------
[ 3.752074] startpar/1067 is trying to acquire lock:
[ 3.752074] (&sb->s_type->i_mutex_key#10){+.+.+.}, at: [<c1156866>] do_blockdev_direct_IO+0x16a6/0x1900
[ 3.752074]
[ 3.752074] but task is already holding lock:
[ 3.752074] (&sb->s_type->i_mutex_key#10){+.+.+.}, at: [<c125cbd1>] process_measurement+0x71/0x1f0
[ 3.752074]
[ 3.752074] other info that might help us debug this:
[ 3.752074] Possible unsafe locking scenario:
[ 3.752074]
[ 3.752074] CPU0
[ 3.752074] ----
[ 3.752074] lock(&sb->s_type->i_mutex_key#10);
[ 3.752074] lock(&sb->s_type->i_mutex_key#10);
[ 3.752074]
[ 3.752074] *** DEADLOCK ***
[ 3.752074]
[ 3.752074] May be due to missing lock nesting notation
[ 3.752074]
[ 3.752074] 1 lock held by startpar/1067:
[ 3.752074] #0: (&sb->s_type->i_mutex_key#10){+.+.+.}, at: [<c125cbd1>] process_measurement+0x71/0x1f0
[ 3.752074]
[ 3.752074] stack backtrace:
[ 3.752074] Pid: 1067, comm: startpar Not tainted 3.7.5+ #30
[ 3.752074] Call Trace:
[ 3.752074] [<c108b57a>] __lock_acquire+0x5da/0x1490
[ 3.752074] [<c108ad6b>] ? trace_hardirqs_on+0xb/0x10
[ 3.752074] [<c108c4b2>] lock_acquire+0x82/0xf0
[ 3.752074] [<c1156866>] ? do_blockdev_direct_IO+0x16a6/0x1900
[ 3.752074] [<c16e9256>] __mutex_lock_common+0x46/0x350
[ 3.752074] [<c1156866>] ? do_blockdev_direct_IO+0x16a6/0x1900
[ 3.752074] [<c111a99d>] ? kmem_cache_alloc+0xcd/0x120
[ 3.752074] [<c16e9651>] mutex_lock_nested+0x31/0x40
[ 3.752074] [<c1156866>] ? do_blockdev_direct_IO+0x16a6/0x1900
[ 3.752074] [<c1156866>] do_blockdev_direct_IO+0x16a6/0x1900
[ 3.752074] [<c106f7d6>] ? find_busiest_group+0x26/0x440
[ 3.752074] [<c10652b8>] ? finish_task_switch+0x38/0xe0
[ 3.752074] [<c1156b30>] __blockdev_direct_IO+0x70/0x80
[ 3.752074] [<c118ed90>] ? noalloc_get_block_write+0x50/0x50
[ 3.752074] [<c11cb3d0>] ext4_ind_direct_IO+0x120/0x500
[ 3.752074] [<c118ed90>] ? noalloc_get_block_write+0x50/0x50
[ 3.752074] [<c1190426>] ext4_direct_IO+0x276/0x430
[ 3.752074] [<c10e2597>] generic_file_aio_read+0x6f7/0x740
[ 3.752074] [<c108b2b0>] ? __lock_acquire+0x310/0x1490
[ 3.752074] [<c1086920>] ? noop_count+0x10/0x10
[ 3.752074] [<c111f653>] do_sync_read+0x93/0xd0
[ 3.752074] [<c111f997>] vfs_read+0x97/0x160
[ 3.752074] [<c111f5c0>] ? do_sync_write+0xd0/0xd0
[ 3.752074] [<c11255f0>] kernel_read+0x30/0x50
[ 3.752074] [<c125d23b>] ima_calc_hash+0xbb/0x1c0
[ 3.752074] [<c1060476>] ? lg_local_unlock+0x16/0x30
[ 3.752074] [<c113cb27>] ? mntput_no_expire+0x37/0x120
[ 3.752074] [<c125d466>] ima_collect_measurement+0x56/0xa0
[ 3.752074] [<c1133cb2>] ? d_path+0xb2/0xd0
[ 3.752074] [<c125cca6>] process_measurement+0x146/0x1f0
[ 3.752074] [<c125cd93>] ima_file_check+0x43/0x1c0
[ 3.752074] [<c112ae95>] do_last+0x485/0xb80
[ 3.752074] [<c1128e81>] ? inode_permission+0x11/0x50
[ 3.752074] [<c112b5f8>] ? link_path_walk+0x68/0x760
[ 3.752074] [<c112d7fd>] path_openat+0x9d/0x3a0
[ 3.752074] [<c12ff96c>] ? tty_release+0x32c/0x4c0
[ 3.752074] [<c112dbf0>] do_filp_open+0x30/0x80
[ 3.752074] [<c111dc9f>] do_sys_open+0xef/0x1d0
[ 3.752074] [<c111dded>] sys_open+0x2d/0x40
[ 3.752074] [<c16ebe4c>] syscall_call+0x7/0xb
Reported-by: CÃdric BERTHION <cedric.berthion@xxxxxxxxxx>
Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@xxxxxxxxx>
Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx>
---
security/integrity/ima/ima_crypto.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index b691e0f..99aea6a 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -45,6 +45,7 @@ int ima_calc_file_hash(struct file *file, char *digest)
loff_t i_size, offset = 0;
char *rbuf;
int rc, read = 0;
+ unsigned int unset_flags = file->f_flags & O_DIRECT;
struct {
struct shash_desc shash;
char ctx[crypto_shash_descsize(ima_shash_tfm)];
@@ -62,6 +63,10 @@ int ima_calc_file_hash(struct file *file, char *digest)
rc = -ENOMEM;
goto out;
}
+
+ if (unset_flags)
+ file->f_flags &= ~unset_flags;
+
if (!(file->f_mode & FMODE_READ)) {
file->f_mode |= FMODE_READ;
read = 1;
@@ -88,6 +93,8 @@ int ima_calc_file_hash(struct file *file, char *digest)
rc = crypto_shash_final(&desc.shash, digest);
if (read)
file->f_mode &= ~FMODE_READ;
+ if (unset_flags)
+ file->f_flags |= unset_flags;
out:
return rc;
}
--
1.8.1.rc3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/