Skip to content

Commit

Permalink
ima: audit log files opened with O_DIRECT flag
Browse files Browse the repository at this point in the history
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.

The first attempt at resolving this lockdep temporarily removed the
O_DIRECT flag and restored it, after calculating the hash.  The
second attempt introduced the O_DIRECT_HAVELOCK flag. Based on this
flag, do_blockdev_direct_IO() would skip taking the i_mutex a second
time.  The third attempt, by Dmitry Kasatkin, resolves the i_mutex
locking issue, by re-introducing the IMA mutex, but uncovered
another problem.  Reading a file with O_DIRECT flag set, writes
directly to userspace pages.  A second patch allocates a user-space
like memory.  This works for all IMA hooks, except ima_file_free(),
which is called on __fput() to recalculate the file hash.

Until this last issue is addressed, do not 'collect' the
measurement for measuring, appraising, or auditing files opened
with the O_DIRECT flag set.  Based on policy, permit or deny file
access.  This patch defines a new IMA policy rule option named
'permit_directio'.  Policy rules could be defined, based on LSM
or other criteria, to permit specific applications to open files
with the O_DIRECT flag set.

Changelog v1:
- permit or deny file access based IMA policy rules

Signed-off-by: Mimi Zohar <[email protected]>
Acked-by: Dmitry Kasatkin <[email protected]>
Cc: <[email protected]>
  • Loading branch information
Mimi Zohar authored and hallyn committed Jun 3, 2014
1 parent ed1c964 commit f9b2a73
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 4 deletions.
2 changes: 1 addition & 1 deletion Documentation/ABI/testing/ima_policy
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ Description:
[fowner]]
lsm: [[subj_user=] [subj_role=] [subj_type=]
[obj_user=] [obj_role=] [obj_type=]]
option: [[appraise_type=]]
option: [[appraise_type=]] [permit_directio]

base: func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
mask:= [MAY_READ] [MAY_WRITE] [MAY_APPEND] [MAY_EXEC]
Expand Down
10 changes: 9 additions & 1 deletion security/integrity/ima/ima_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
struct evm_ima_xattr_data **xattr_value,
int *xattr_len)
{
const char *audit_cause = "failed";
struct inode *inode = file_inode(file);
const char *filename = file->f_dentry->d_name.name;
int result = 0;
Expand All @@ -213,6 +214,12 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
if (!(iint->flags & IMA_COLLECTED)) {
u64 i_version = file_inode(file)->i_version;

if (file->f_flags & O_DIRECT) {
audit_cause = "failed(directio)";
result = -EACCES;
goto out;
}

/* use default hash algorithm */
hash.hdr.algo = ima_hash_algo;

Expand All @@ -233,9 +240,10 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
result = -ENOMEM;
}
}
out:
if (result)
integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode,
filename, "collect_data", "failed",
filename, "collect_data", audit_cause,
result, 0);
return result;
}
Expand Down
5 changes: 4 additions & 1 deletion security/integrity/ima/ima_main.c
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,11 @@ static int process_measurement(struct file *file, const char *filename,
xattr_ptr = &xattr_value;

rc = ima_collect_measurement(iint, file, xattr_ptr, &xattr_len);
if (rc != 0)
if (rc != 0) {
if (file->f_flags & O_DIRECT)
rc = (iint->flags & IMA_PERMIT_DIRECTIO) ? 0 : -EACCES;
goto out_digsig;
}

pathname = filename ?: ima_d_path(&file->f_path, &pathbuf);

Expand Down
6 changes: 5 additions & 1 deletion security/integrity/ima/ima_policy.c
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ enum {
Opt_obj_user, Opt_obj_role, Opt_obj_type,
Opt_subj_user, Opt_subj_role, Opt_subj_type,
Opt_func, Opt_mask, Opt_fsmagic, Opt_uid, Opt_fowner,
Opt_appraise_type, Opt_fsuuid
Opt_appraise_type, Opt_fsuuid, Opt_permit_directio
};

static match_table_t policy_tokens = {
Expand All @@ -375,6 +375,7 @@ static match_table_t policy_tokens = {
{Opt_uid, "uid=%s"},
{Opt_fowner, "fowner=%s"},
{Opt_appraise_type, "appraise_type=%s"},
{Opt_permit_directio, "permit_directio"},
{Opt_err, NULL}
};

Expand Down Expand Up @@ -622,6 +623,9 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
else
result = -EINVAL;
break;
case Opt_permit_directio:
entry->flags |= IMA_PERMIT_DIRECTIO;
break;
case Opt_err:
ima_log_string(ab, "UNKNOWN", p);
result = -EINVAL;
Expand Down
1 change: 1 addition & 0 deletions security/integrity/integrity.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#define IMA_ACTION_FLAGS 0xff000000
#define IMA_DIGSIG 0x01000000
#define IMA_DIGSIG_REQUIRED 0x02000000
#define IMA_PERMIT_DIRECTIO 0x04000000

#define IMA_DO_MASK (IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
IMA_APPRAISE_SUBMASK)
Expand Down

0 comments on commit f9b2a73

Please sign in to comment.