Files are measured or appraised based on the IMA policy. When a file,
in policy, is opened with the O_DIRECT flag, a deadlock occurs.
ima_process_measurement() takes the i_mutex, before calculating the
hash, appraising the hash against a 'good' known value stored as an
extended attribute, and caching the result, before releasing the
i_mutex. Holding the i_mutex results in a lockdep with
do_blockdev_direct_IO(), which takes the i_mutex and releases it
for each read.
The first attempt at resolving this lockdep, by Dmitry Kasatkin,
temporarily removed the O_DIRECT flag and restored it, after
calculating the hash. This resolved the lockdep, but the benefits
of using O_DIRECT, in terms of page cache, were lost. Al Viro
objected to this patch, because it made the "existing locking rule
more convoluted". (The existing (undocumented) IMA locking
rule prohibits ->read() of regular files from taking the i_mutex.
The change would have prohibited ->read() of regular files from
taking the i_mutex, unless they were opened with O_DIRECT.)
The IMA specific 'iint->mutex' was removed, because it caused dead
locking due to different locking orders in different cases (eg.
regular vs. setxattr).
This patch defines a new flag, O_DIRECT_HAVELOCK for lack of a better
name, to indicate the i_mutex has already been taken. Based on this
flag, the i_mutex is not taken, again, in do_blockdev_direct_IO().
Similar changes for filesystems, that define their own .aio_read
instead of using the generic_file_aio_read(), will probably need
to be made. Before going down this path, I'd appreciate any
comments/suggestions.
thanks,
Mimi
Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx>
---
fs/direct-io.c | 12 +++++++++---
include/uapi/asm-generic/fcntl.h | 1 +
security/integrity/ima/ima_main.c | 5 +++++
3 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/fs/direct-io.c b/fs/direct-io.c
index 0e04142..d6444b9 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -1126,6 +1126,7 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
size_t bytes;
struct buffer_head map_bh = { 0, };
struct blk_plug plug;
+ int skip_lock = 0;
if (rw & WRITE)
rw = WRITE_ODIRECT;
@@ -1179,14 +1180,19 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
if (rw == READ) {
struct address_space *mapping =
iocb->ki_filp->f_mapping;
+ struct file *file = iocb->ki_filp;
/* will be released by direct_io_worker */
- mutex_lock(&inode->i_mutex);
+ if (file->f_flags & O_DIRECT_HAVELOCK)
+ skip_lock = 1;
+ if (!skip_lock)
+ mutex_lock(&inode->i_mutex);
retval = filemap_write_and_wait_range(mapping, offset,
end - 1);
if (retval) {
- mutex_unlock(&inode->i_mutex);
+ if (!skip_lock)
+ mutex_unlock(&inode->i_mutex);
kmem_cache_free(dio_cache, dio);
goto out;
}
@@ -1331,7 +1337,7 @@ do_blockdev_direct_IO(int rw, struct kiocb *iocb, struct inode *inode,
* we can let i_mutex go now that its achieved its purpose
* of protecting us from looking up uninitialized blocks.
*/
- if (rw == READ && (dio->flags & DIO_LOCKING))
+ if (rw == READ && (dio->flags & DIO_LOCKING) && (!skip_lock))
mutex_unlock(&dio->inode->i_mutex);
/*
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 95e46c8..02c6996 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -45,6 +45,7 @@
#endif
#ifndef O_DIRECT
#define O_DIRECT 00040000 /* direct disk access hint */
+#define O_DIRECT_HAVELOCK 04000000 /* direct disk access hint */
#endif
#ifndef O_LARGEFILE
#define O_LARGEFILE 00100000
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 149ee11..0d2c93d 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -191,6 +191,9 @@ static int process_measurement(struct file *file, const char *filename,
mutex_lock(&inode->i_mutex);
+ if (file->f_flags & O_DIRECT)
+ file->f_flags |= O_DIRECT_HAVELOCK;
+
iint = integrity_inode_get(inode);
if (!iint)
goto out;
@@ -237,6 +240,8 @@ out_digsig:
if ((mask & MAY_WRITE) && (iint->flags & IMA_DIGSIG))
rc = -EACCES;
out:
+ if (file->f_flags & O_DIRECT)
+ file->f_flags &= ~O_DIRECT_HAVELOCK;
mutex_unlock(&inode->i_mutex);
kfree(xattr_value);
if ((rc && must_appraise) && (ima_appraise & IMA_APPRAISE_ENFORCE))
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
